- Revision
- 211501
- Author
- commit-qu...@webkit.org
- Date
- 2017-02-01 11:54:25 -0800 (Wed, 01 Feb 2017)
Log Message
[mac-wk1] LayoutTest media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html is a flaky timeout
https://bugs.webkit.org/show_bug.cgi?id=165319
<rdar://problem/30284104>
Patch by Antoine Quint <grao...@apple.com> on 2017-02-01
Reviewed by Dean Jackson.
Source/WebCore:
Running media/controls/track-menu.html before media/modern-media-controls/tracks-support/tracks-
support-click-track-in-panel.html makes that test time out in all test runs. The root of the issue
is that animations are suspended by media/controls/track-menu.html with a call to
internals.suspendAnimations(), and that state isn't reset with a call to internals.resumeAnimations().
Then, media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html fails because
the selection animation for the tracks panel menu item that is clicked never completes and the delegate
to notify that an item in the tracks panel was selected is never fired, which leads to the test failure.
We change Internals::suspendAnimations() and Internals::resumeAnimations() to only affect the current
document, rather than calling into AnimationController::suspendAnimations() which would do just that,
but also set a Frame-wide flag that would prevent further animations from running, even in a subsequent
document load.
* dom/Document.cpp:
(WebCore::Document::prepareForDestruction): Ensure the document that is about to be destroyed is no longer
associated with an AnimationController.
* page/animation/AnimationController.cpp:
(WebCore::AnimationControllerPrivate::ensureCompositeAnimation): Update the animation's suspend state in case
the document its renderer is associated with is suspended. This is required since previously CompositeAnimations
would set their suspend state in their constructor, based on the Frame-wide suspended state, but there is no
document to use as a basis to query its suspended state in that constructor.
(WebCore::AnimationControllerPrivate::animationsAreSuspendedForDocument):
(WebCore::AnimationControllerPrivate::detachFromDocument):
(WebCore::AnimationControllerPrivate::suspendAnimationsForDocument):
(WebCore::AnimationControllerPrivate::resumeAnimationsForDocument):
(WebCore::AnimationControllerPrivate::startAnimationsIfNotSuspended):
(WebCore::AnimationController::animationsAreSuspendedForDocument):
(WebCore::AnimationController::detachFromDocument):
* page/animation/AnimationController.h:
* page/animation/AnimationControllerPrivate.h:
* testing/Internals.cpp:
(WebCore::Internals::animationsAreSuspended):
(WebCore::Internals::suspendAnimations):
(WebCore::Internals::resumeAnimations):
LayoutTests:
Since we've fixed the root cause of this test's flakiness, we no longer need to mark it as flaky.
* platform/mac/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (211500 => 211501)
--- trunk/LayoutTests/ChangeLog 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/LayoutTests/ChangeLog 2017-02-01 19:54:25 UTC (rev 211501)
@@ -1,3 +1,15 @@
+2017-02-01 Antoine Quint <grao...@apple.com>
+
+ [mac-wk1] LayoutTest media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html is a flaky timeout
+ https://bugs.webkit.org/show_bug.cgi?id=165319
+ <rdar://problem/30284104>
+
+ Reviewed by Dean Jackson.
+
+ Since we've fixed the root cause of this test's flakiness, we no longer need to mark it as flaky.
+
+ * platform/mac/TestExpectations:
+
2017-02-01 Jer Noble <jer.no...@apple.com>
NULL-deref crash in TextTrack::removeCue()
Modified: trunk/LayoutTests/platform/mac/TestExpectations (211500 => 211501)
--- trunk/LayoutTests/platform/mac/TestExpectations 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2017-02-01 19:54:25 UTC (rev 211501)
@@ -1484,8 +1484,6 @@
webkit.org/b/167442 [ ElCapitan ] media/modern-media-controls/airplay-support/airplay-support.html [ Pass Timeout ]
-webkit.org/b/165319 media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html [ Pass Timeout ]
-
webkit.org/b/167461 media/modern-media-controls/layout-node/addChild.html [ Pass Timeout ]
webkit.org/b/165874 [ Debug ] streams/pipe-to.html [ Pass Failure ]
Modified: trunk/Source/WebCore/ChangeLog (211500 => 211501)
--- trunk/Source/WebCore/ChangeLog 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/Source/WebCore/ChangeLog 2017-02-01 19:54:25 UTC (rev 211501)
@@ -1,3 +1,46 @@
+2017-02-01 Antoine Quint <grao...@apple.com>
+
+ [mac-wk1] LayoutTest media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html is a flaky timeout
+ https://bugs.webkit.org/show_bug.cgi?id=165319
+ <rdar://problem/30284104>
+
+ Reviewed by Dean Jackson.
+
+ Running media/controls/track-menu.html before media/modern-media-controls/tracks-support/tracks-
+ support-click-track-in-panel.html makes that test time out in all test runs. The root of the issue
+ is that animations are suspended by media/controls/track-menu.html with a call to
+ internals.suspendAnimations(), and that state isn't reset with a call to internals.resumeAnimations().
+ Then, media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html fails because
+ the selection animation for the tracks panel menu item that is clicked never completes and the delegate
+ to notify that an item in the tracks panel was selected is never fired, which leads to the test failure.
+
+ We change Internals::suspendAnimations() and Internals::resumeAnimations() to only affect the current
+ document, rather than calling into AnimationController::suspendAnimations() which would do just that,
+ but also set a Frame-wide flag that would prevent further animations from running, even in a subsequent
+ document load.
+
+ * dom/Document.cpp:
+ (WebCore::Document::prepareForDestruction): Ensure the document that is about to be destroyed is no longer
+ associated with an AnimationController.
+ * page/animation/AnimationController.cpp:
+ (WebCore::AnimationControllerPrivate::ensureCompositeAnimation): Update the animation's suspend state in case
+ the document its renderer is associated with is suspended. This is required since previously CompositeAnimations
+ would set their suspend state in their constructor, based on the Frame-wide suspended state, but there is no
+ document to use as a basis to query its suspended state in that constructor.
+ (WebCore::AnimationControllerPrivate::animationsAreSuspendedForDocument):
+ (WebCore::AnimationControllerPrivate::detachFromDocument):
+ (WebCore::AnimationControllerPrivate::suspendAnimationsForDocument):
+ (WebCore::AnimationControllerPrivate::resumeAnimationsForDocument):
+ (WebCore::AnimationControllerPrivate::startAnimationsIfNotSuspended):
+ (WebCore::AnimationController::animationsAreSuspendedForDocument):
+ (WebCore::AnimationController::detachFromDocument):
+ * page/animation/AnimationController.h:
+ * page/animation/AnimationControllerPrivate.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::animationsAreSuspended):
+ (WebCore::Internals::suspendAnimations):
+ (WebCore::Internals::resumeAnimations):
+
2017-02-01 Ryan Haddad <ryanhad...@apple.com>
Unreviewed build fix after r211488.
Modified: trunk/Source/WebCore/dom/Document.cpp (211500 => 211501)
--- trunk/Source/WebCore/dom/Document.cpp 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-02-01 19:54:25 UTC (rev 211501)
@@ -2263,6 +2263,8 @@
if (m_hasPreparedForDestruction)
return;
+ m_frame->animation().detachFromDocument(this);
+
#if ENABLE(IOS_TOUCH_EVENTS)
clearTouchEventHandlersAndListeners();
#endif
Modified: trunk/Source/WebCore/page/animation/AnimationController.cpp (211500 => 211501)
--- trunk/Source/WebCore/page/animation/AnimationController.cpp 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/Source/WebCore/page/animation/AnimationController.cpp 2017-02-01 19:54:25 UTC (rev 211501)
@@ -91,6 +91,9 @@
renderer.setIsCSSAnimating(true);
}
+ if (animationsAreSuspendedForDocument(&renderer.document()))
+ result.iterator->value->suspendAnimations();
+
return *result.iterator->value;
}
@@ -308,8 +311,23 @@
m_isSuspended = false;
}
+bool AnimationControllerPrivate::animationsAreSuspendedForDocument(Document* document)
+{
+ return isSuspended() || m_suspendedDocuments.contains(document);
+}
+
+void AnimationControllerPrivate::detachFromDocument(Document* document)
+{
+ m_suspendedDocuments.remove(document);
+}
+
void AnimationControllerPrivate::suspendAnimationsForDocument(Document* document)
{
+ if (animationsAreSuspendedForDocument(document))
+ return;
+
+ m_suspendedDocuments.add(document);
+
AnimationPrivateUpdateBlock updateBlock(*this);
for (auto& animation : m_compositeAnimations) {
@@ -322,6 +340,11 @@
void AnimationControllerPrivate::resumeAnimationsForDocument(Document* document)
{
+ if (!animationsAreSuspendedForDocument(document))
+ return;
+
+ detachFromDocument(document);
+
AnimationPrivateUpdateBlock updateBlock(*this);
for (auto& animation : m_compositeAnimations) {
@@ -334,7 +357,7 @@
void AnimationControllerPrivate::startAnimationsIfNotSuspended(Document* document)
{
- if (!isSuspended() || allowsNewAnimationsWhileSuspended())
+ if (!animationsAreSuspendedForDocument(document) || allowsNewAnimationsWhileSuspended())
resumeAnimationsForDocument(document);
}
@@ -701,6 +724,16 @@
m_data->animationFrameCallbackFired();
}
+bool AnimationController::animationsAreSuspendedForDocument(Document* document)
+{
+ return m_data->animationsAreSuspendedForDocument(document);
+}
+
+void AnimationController::detachFromDocument(Document* document)
+{
+ return m_data->detachFromDocument(document);
+}
+
void AnimationController::suspendAnimationsForDocument(Document* document)
{
LOG(Animations, "suspending animations for document %p", document);
Modified: trunk/Source/WebCore/page/animation/AnimationController.h (211500 => 211501)
--- trunk/Source/WebCore/page/animation/AnimationController.h 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/Source/WebCore/page/animation/AnimationController.h 2017-02-01 19:54:25 UTC (rev 211501)
@@ -72,8 +72,10 @@
WEBCORE_EXPORT void resumeAnimations();
void serviceAnimations();
- void suspendAnimationsForDocument(Document*);
- void resumeAnimationsForDocument(Document*);
+ WEBCORE_EXPORT void suspendAnimationsForDocument(Document*);
+ WEBCORE_EXPORT void resumeAnimationsForDocument(Document*);
+ WEBCORE_EXPORT bool animationsAreSuspendedForDocument(Document*);
+ void detachFromDocument(Document*);
void startAnimationsIfNotSuspended(Document*);
void beginAnimationUpdate();
Modified: trunk/Source/WebCore/page/animation/AnimationControllerPrivate.h (211500 => 211501)
--- trunk/Source/WebCore/page/animation/AnimationControllerPrivate.h 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/Source/WebCore/page/animation/AnimationControllerPrivate.h 2017-02-01 19:54:25 UTC (rev 211501)
@@ -69,7 +69,9 @@
void suspendAnimationsForDocument(Document*);
void resumeAnimationsForDocument(Document*);
+ bool animationsAreSuspendedForDocument(Document*);
void startAnimationsIfNotSuspended(Document*);
+ void detachFromDocument(Document*);
bool isRunningAnimationOnRenderer(RenderElement&, CSSPropertyID, AnimationBase::RunningState) const;
bool isRunningAcceleratedAnimationOnRenderer(RenderElement&, CSSPropertyID, AnimationBase::RunningState) const;
@@ -131,7 +133,8 @@
};
Vector<EventToDispatch> m_eventsToDispatch;
Vector<Ref<Element>> m_elementChangesToDispatch;
-
+ HashSet<Document*> m_suspendedDocuments;
+
double m_beginAnimationUpdateTime;
using AnimationsSet = HashSet<RefPtr<AnimationBase>>;
Modified: trunk/Source/WebCore/testing/Internals.cpp (211500 => 211501)
--- trunk/Source/WebCore/testing/Internals.cpp 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/Source/WebCore/testing/Internals.cpp 2017-02-01 19:54:25 UTC (rev 211501)
@@ -739,7 +739,7 @@
if (!document || !document->frame())
return Exception { INVALID_ACCESS_ERR };
- return document->frame()->animation().isSuspended();
+ return document->frame()->animation().animationsAreSuspendedForDocument(document);
}
ExceptionOr<void> Internals::suspendAnimations() const
@@ -748,7 +748,13 @@
if (!document || !document->frame())
return Exception { INVALID_ACCESS_ERR };
- document->frame()->animation().suspendAnimations();
+ document->frame()->animation().suspendAnimationsForDocument(document);
+
+ for (Frame* frame = document->frame(); frame; frame = frame->tree().traverseNext()) {
+ if (Document* document = frame->document())
+ frame->animation().suspendAnimationsForDocument(document);
+ }
+
return { };
}
@@ -758,7 +764,13 @@
if (!document || !document->frame())
return Exception { INVALID_ACCESS_ERR };
- document->frame()->animation().resumeAnimations();
+ document->frame()->animation().resumeAnimationsForDocument(document);
+
+ for (Frame* frame = document->frame(); frame; frame = frame->tree().traverseNext()) {
+ if (Document* document = frame->document())
+ frame->animation().resumeAnimationsForDocument(document);
+ }
+
return { };
}
