<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[211501] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/211501">211501</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2017-02-01 11:54:25 -0800 (Wed, 01 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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 <graouts@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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformmacTestExpectations">trunk/LayoutTests/platform/mac/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomDocumentcpp">trunk/Source/WebCore/dom/Document.cpp</a></li>
<li><a href="#trunkSourceWebCorepageanimationAnimationControllercpp">trunk/Source/WebCore/page/animation/AnimationController.cpp</a></li>
<li><a href="#trunkSourceWebCorepageanimationAnimationControllerh">trunk/Source/WebCore/page/animation/AnimationController.h</a></li>
<li><a href="#trunkSourceWebCorepageanimationAnimationControllerPrivateh">trunk/Source/WebCore/page/animation/AnimationControllerPrivate.h</a></li>
<li><a href="#trunkSourceWebCoretestingInternalscpp">trunk/Source/WebCore/testing/Internals.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-02-01 19:44:07 UTC (rev 211500)
+++ trunk/LayoutTests/ChangeLog 2017-02-01 19:54:25 UTC (rev 211501)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2017-02-01 Antoine Quint <graouts@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:
+
</ins><span class="cx"> 2017-02-01 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><span class="cx"> NULL-deref crash in TextTrack::removeCue()
</span></span></pre></div>
<a id="trunkLayoutTestsplatformmacTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/mac/TestExpectations (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1484,8 +1484,6 @@
</span><span class="cx">
</span><span class="cx"> webkit.org/b/167442 [ ElCapitan ] media/modern-media-controls/airplay-support/airplay-support.html [ Pass Timeout ]
</span><span class="cx">
</span><del>-webkit.org/b/165319 media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html [ Pass Timeout ]
-
</del><span class="cx"> webkit.org/b/167461 media/modern-media-controls/layout-node/addChild.html [ Pass Timeout ]
</span><span class="cx">
</span><span class="cx"> webkit.org/b/165874 [ Debug ] streams/pipe-to.html [ Pass Failure ]
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,46 @@
</span><ins>+2017-02-01 Antoine Quint <graouts@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):
+
</ins><span class="cx"> 2017-02-01 Ryan Haddad <ryanhaddad@apple.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed build fix after r211488.
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Document.cpp (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2263,6 +2263,8 @@
</span><span class="cx"> if (m_hasPreparedForDestruction)
</span><span class="cx"> return;
</span><span class="cx">
</span><ins>+ m_frame->animation().detachFromDocument(this);
+
</ins><span class="cx"> #if ENABLE(IOS_TOUCH_EVENTS)
</span><span class="cx"> clearTouchEventHandlersAndListeners();
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceWebCorepageanimationAnimationControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/animation/AnimationController.cpp (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -91,6 +91,9 @@
</span><span class="cx"> renderer.setIsCSSAnimating(true);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ if (animationsAreSuspendedForDocument(&renderer.document()))
+ result.iterator->value->suspendAnimations();
+
</ins><span class="cx"> return *result.iterator->value;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -308,8 +311,23 @@
</span><span class="cx"> m_isSuspended = false;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool AnimationControllerPrivate::animationsAreSuspendedForDocument(Document* document)
+{
+ return isSuspended() || m_suspendedDocuments.contains(document);
+}
+
+void AnimationControllerPrivate::detachFromDocument(Document* document)
+{
+ m_suspendedDocuments.remove(document);
+}
+
</ins><span class="cx"> void AnimationControllerPrivate::suspendAnimationsForDocument(Document* document)
</span><span class="cx"> {
</span><ins>+ if (animationsAreSuspendedForDocument(document))
+ return;
+
+ m_suspendedDocuments.add(document);
+
</ins><span class="cx"> AnimationPrivateUpdateBlock updateBlock(*this);
</span><span class="cx">
</span><span class="cx"> for (auto& animation : m_compositeAnimations) {
</span><span class="lines">@@ -322,6 +340,11 @@
</span><span class="cx">
</span><span class="cx"> void AnimationControllerPrivate::resumeAnimationsForDocument(Document* document)
</span><span class="cx"> {
</span><ins>+ if (!animationsAreSuspendedForDocument(document))
+ return;
+
+ detachFromDocument(document);
+
</ins><span class="cx"> AnimationPrivateUpdateBlock updateBlock(*this);
</span><span class="cx">
</span><span class="cx"> for (auto& animation : m_compositeAnimations) {
</span><span class="lines">@@ -334,7 +357,7 @@
</span><span class="cx">
</span><span class="cx"> void AnimationControllerPrivate::startAnimationsIfNotSuspended(Document* document)
</span><span class="cx"> {
</span><del>- if (!isSuspended() || allowsNewAnimationsWhileSuspended())
</del><ins>+ if (!animationsAreSuspendedForDocument(document) || allowsNewAnimationsWhileSuspended())
</ins><span class="cx"> resumeAnimationsForDocument(document);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -701,6 +724,16 @@
</span><span class="cx"> m_data->animationFrameCallbackFired();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool AnimationController::animationsAreSuspendedForDocument(Document* document)
+{
+ return m_data->animationsAreSuspendedForDocument(document);
+}
+
+void AnimationController::detachFromDocument(Document* document)
+{
+ return m_data->detachFromDocument(document);
+}
+
</ins><span class="cx"> void AnimationController::suspendAnimationsForDocument(Document* document)
</span><span class="cx"> {
</span><span class="cx"> LOG(Animations, "suspending animations for document %p", document);
</span></span></pre></div>
<a id="trunkSourceWebCorepageanimationAnimationControllerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/animation/AnimationController.h (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -72,8 +72,10 @@
</span><span class="cx"> WEBCORE_EXPORT void resumeAnimations();
</span><span class="cx"> void serviceAnimations();
</span><span class="cx">
</span><del>- void suspendAnimationsForDocument(Document*);
- void resumeAnimationsForDocument(Document*);
</del><ins>+ WEBCORE_EXPORT void suspendAnimationsForDocument(Document*);
+ WEBCORE_EXPORT void resumeAnimationsForDocument(Document*);
+ WEBCORE_EXPORT bool animationsAreSuspendedForDocument(Document*);
+ void detachFromDocument(Document*);
</ins><span class="cx"> void startAnimationsIfNotSuspended(Document*);
</span><span class="cx">
</span><span class="cx"> void beginAnimationUpdate();
</span></span></pre></div>
<a id="trunkSourceWebCorepageanimationAnimationControllerPrivateh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/animation/AnimationControllerPrivate.h (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -69,7 +69,9 @@
</span><span class="cx">
</span><span class="cx"> void suspendAnimationsForDocument(Document*);
</span><span class="cx"> void resumeAnimationsForDocument(Document*);
</span><ins>+ bool animationsAreSuspendedForDocument(Document*);
</ins><span class="cx"> void startAnimationsIfNotSuspended(Document*);
</span><ins>+ void detachFromDocument(Document*);
</ins><span class="cx">
</span><span class="cx"> bool isRunningAnimationOnRenderer(RenderElement&, CSSPropertyID, AnimationBase::RunningState) const;
</span><span class="cx"> bool isRunningAcceleratedAnimationOnRenderer(RenderElement&, CSSPropertyID, AnimationBase::RunningState) const;
</span><span class="lines">@@ -131,7 +133,8 @@
</span><span class="cx"> };
</span><span class="cx"> Vector<EventToDispatch> m_eventsToDispatch;
</span><span class="cx"> Vector<Ref<Element>> m_elementChangesToDispatch;
</span><del>-
</del><ins>+ HashSet<Document*> m_suspendedDocuments;
+
</ins><span class="cx"> double m_beginAnimationUpdateTime;
</span><span class="cx">
</span><span class="cx"> using AnimationsSet = HashSet<RefPtr<AnimationBase>>;
</span></span></pre></div>
<a id="trunkSourceWebCoretestingInternalscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/testing/Internals.cpp (211500 => 211501)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -739,7 +739,7 @@
</span><span class="cx"> if (!document || !document->frame())
</span><span class="cx"> return Exception { INVALID_ACCESS_ERR };
</span><span class="cx">
</span><del>- return document->frame()->animation().isSuspended();
</del><ins>+ return document->frame()->animation().animationsAreSuspendedForDocument(document);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ExceptionOr<void> Internals::suspendAnimations() const
</span><span class="lines">@@ -748,7 +748,13 @@
</span><span class="cx"> if (!document || !document->frame())
</span><span class="cx"> return Exception { INVALID_ACCESS_ERR };
</span><span class="cx">
</span><del>- document->frame()->animation().suspendAnimations();
</del><ins>+ document->frame()->animation().suspendAnimationsForDocument(document);
+
+ for (Frame* frame = document->frame(); frame; frame = frame->tree().traverseNext()) {
+ if (Document* document = frame->document())
+ frame->animation().suspendAnimationsForDocument(document);
+ }
+
</ins><span class="cx"> return { };
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -758,7 +764,13 @@
</span><span class="cx"> if (!document || !document->frame())
</span><span class="cx"> return Exception { INVALID_ACCESS_ERR };
</span><span class="cx">
</span><del>- document->frame()->animation().resumeAnimations();
</del><ins>+ document->frame()->animation().resumeAnimationsForDocument(document);
+
+ for (Frame* frame = document->frame(); frame; frame = frame->tree().traverseNext()) {
+ if (Document* document = frame->document())
+ frame->animation().resumeAnimationsForDocument(document);
+ }
+
</ins><span class="cx"> return { };
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>