<!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>[212869] 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/212869">212869</a></dd>
<dt>Author</dt> <dd>graouts@webkit.org</dd>
<dt>Date</dt> <dd>2017-02-22 17:55:05 -0800 (Wed, 22 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>[Modern Media Controls] Scrubber stops moving while scrubbing on macOS
https://bugs.webkit.org/show_bug.cgi?id=168518
<rdar://problem/30577637>
Reviewed by Dean Jackson.
Source/WebCore:
As we start to scrub, controlValueWillStartChanging() is called on
ScrubberSupport and pauses the media if it's not already paused. This
causes the play/pause button to change icon and for layout() to be
called on MacOSInlineMediaControls. This in turns sets the .children
property on the .controlsBar and eventually yields a DOM manipulation
which re-inserts the scrubber's DOM hierarchy, causing stutters during
user interaction.
Our solution is to make the .children setter smarter about identifying
that the children list hasn't changed and that no DOM invalidation is
necessary.
* Modules/modern-media-controls/controls/layout-node.js:
(LayoutNode.prototype.set children):
LayoutTests:
Add assertions to check that setting children to a copy of itself doesn't
mark nodes as needing layout.
* media/modern-media-controls/layout-node/children-expected.txt:
* media/modern-media-controls/layout-node/children.html:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsmediamodernmediacontrolslayoutnodechildrenexpectedtxt">trunk/LayoutTests/media/modern-media-controls/layout-node/children-expected.txt</a></li>
<li><a href="#trunkLayoutTestsmediamodernmediacontrolslayoutnodechildrenhtml">trunk/LayoutTests/media/modern-media-controls/layout-node/children.html</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolscontrolslayoutnodejs">trunk/Source/WebCore/Modules/modern-media-controls/controls/layout-node.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (212868 => 212869)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-02-23 01:48:40 UTC (rev 212868)
+++ trunk/LayoutTests/ChangeLog 2017-02-23 01:55:05 UTC (rev 212869)
</span><span class="lines">@@ -1,5 +1,19 @@
</span><span class="cx"> 2017-02-22 Antoine Quint <graouts@apple.com>
</span><span class="cx">
</span><ins>+ [Modern Media Controls] Scrubber stops moving while scrubbing on macOS
+ https://bugs.webkit.org/show_bug.cgi?id=168518
+ <rdar://problem/30577637>
+
+ Reviewed by Dean Jackson.
+
+ Add assertions to check that setting children to a copy of itself doesn't
+ mark nodes as needing layout.
+
+ * media/modern-media-controls/layout-node/children-expected.txt:
+ * media/modern-media-controls/layout-node/children.html:
+
+2017-02-22 Antoine Quint <graouts@apple.com>
+
</ins><span class="cx"> [Modern Media Controls] Controls bar may disappear while captions menu is visible
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=168751
</span><span class="cx"> <rdar://problem/30663411>
</span></span></pre></div>
<a id="trunkLayoutTestsmediamodernmediacontrolslayoutnodechildrenexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/media/modern-media-controls/layout-node/children-expected.txt (212868 => 212869)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/media/modern-media-controls/layout-node/children-expected.txt 2017-02-23 01:48:40 UTC (rev 212868)
+++ trunk/LayoutTests/media/modern-media-controls/layout-node/children-expected.txt 2017-02-23 01:55:05 UTC (rev 212869)
</span><span class="lines">@@ -20,6 +20,11 @@
</span><span class="cx"> PASS node.element.firstElementChild.nextElementSibling === b.element is true
</span><span class="cx"> PASS node.element.lastElementChild === c.element is true
</span><span class="cx">
</span><ins>+Set children to be a copy of itself
+PASS node.children[0].needsLayout is false
+PASS node.children[1].needsLayout is false
+PASS node.children[2].needsLayout is false
+
</ins><span class="cx"> Set children to [b, a]
</span><span class="cx"> PASS node.children.length === 2 is true
</span><span class="cx"> PASS node.children[0] === b is true
</span></span></pre></div>
<a id="trunkLayoutTestsmediamodernmediacontrolslayoutnodechildrenhtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/media/modern-media-controls/layout-node/children.html (212868 => 212869)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/media/modern-media-controls/layout-node/children.html 2017-02-23 01:48:40 UTC (rev 212868)
+++ trunk/LayoutTests/media/modern-media-controls/layout-node/children.html 2017-02-23 01:55:05 UTC (rev 212869)
</span><span class="lines">@@ -42,6 +42,12 @@
</span><span class="cx"> shouldBeTrue("node.element.firstElementChild.nextElementSibling === b.element");
</span><span class="cx"> shouldBeTrue("node.element.lastElementChild === c.element");
</span><span class="cx"> debug("");
</span><ins>+ debug("Set children to be a copy of itself");
+ node.children = Array.from(node.children);
+ shouldBeFalse("node.children[0].needsLayout");
+ shouldBeFalse("node.children[1].needsLayout");
+ shouldBeFalse("node.children[2].needsLayout");
+ debug("");
</ins><span class="cx"> debug("Set children to [b, a]");
</span><span class="cx"> node.children = [b, a];
</span><span class="cx"> shouldBeTrue("node.children.length === 2");
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (212868 => 212869)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-02-23 01:48:40 UTC (rev 212868)
+++ trunk/Source/WebCore/ChangeLog 2017-02-23 01:55:05 UTC (rev 212869)
</span><span class="lines">@@ -1,5 +1,28 @@
</span><span class="cx"> 2017-02-22 Antoine Quint <graouts@apple.com>
</span><span class="cx">
</span><ins>+ [Modern Media Controls] Scrubber stops moving while scrubbing on macOS
+ https://bugs.webkit.org/show_bug.cgi?id=168518
+ <rdar://problem/30577637>
+
+ Reviewed by Dean Jackson.
+
+ As we start to scrub, controlValueWillStartChanging() is called on
+ ScrubberSupport and pauses the media if it's not already paused. This
+ causes the play/pause button to change icon and for layout() to be
+ called on MacOSInlineMediaControls. This in turns sets the .children
+ property on the .controlsBar and eventually yields a DOM manipulation
+ which re-inserts the scrubber's DOM hierarchy, causing stutters during
+ user interaction.
+
+ Our solution is to make the .children setter smarter about identifying
+ that the children list hasn't changed and that no DOM invalidation is
+ necessary.
+
+ * Modules/modern-media-controls/controls/layout-node.js:
+ (LayoutNode.prototype.set children):
+
+2017-02-22 Antoine Quint <graouts@apple.com>
+
</ins><span class="cx"> [Modern Media Controls] Controls bar may disappear while captions menu is visible
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=168751
</span><span class="cx"> <rdar://problem/30663411>
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolscontrolslayoutnodejs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/layout-node.js (212868 => 212869)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/controls/layout-node.js 2017-02-23 01:48:40 UTC (rev 212868)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/layout-node.js 2017-02-23 01:55:05 UTC (rev 212869)
</span><span class="lines">@@ -126,6 +126,18 @@
</span><span class="cx">
</span><span class="cx"> set children(children)
</span><span class="cx"> {
</span><ins>+ if (children.length === this._children.length) {
+ let arraysDiffer = false;
+ for (let i = children.length - 1; i >= 0; --i) {
+ if (children[i] !== this._children[i]) {
+ arraysDiffer = true;
+ break;
+ }
+ }
+ if (!arraysDiffer)
+ return;
+ }
+
</ins><span class="cx"> while (this._children.length)
</span><span class="cx"> this.removeChild(this._children[0]);
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>