[Modern Media Controls] Scrubber stops moving while scrubbing on macOS
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2017 01:55:05 +0000 (01:55 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2017 01:55:05 +0000 (01:55 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@212869 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/modern-media-controls/layout-node/children-expected.txt
LayoutTests/media/modern-media-controls/layout-node/children.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/modern-media-controls/controls/layout-node.js

index 2e3669d..4c17813 100644 (file)
@@ -1,5 +1,19 @@
 2017-02-22  Antoine Quint  <graouts@apple.com>
 
+        [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>
+
         [Modern Media Controls] Controls bar may disappear while captions menu is visible
         https://bugs.webkit.org/show_bug.cgi?id=168751
         <rdar://problem/30663411>
index 91a7281..05765c4 100644 (file)
@@ -20,6 +20,11 @@ PASS node.element.firstElementChild === a.element is true
 PASS node.element.firstElementChild.nextElementSibling === b.element is true
 PASS node.element.lastElementChild === c.element is true
 
+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
+
 Set children to [b, a]
 PASS node.children.length === 2 is true
 PASS node.children[0] === b is true
index 38b8db4..24ef088 100644 (file)
@@ -42,6 +42,12 @@ scheduler.frameDidFire = function()
         shouldBeTrue("node.element.firstElementChild.nextElementSibling === b.element");
         shouldBeTrue("node.element.lastElementChild === c.element");
         debug("");
+        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("");
         debug("Set children to [b, a]");
         node.children = [b, a];
         shouldBeTrue("node.children.length === 2");
index e65babe..d3bfa44 100644 (file)
@@ -1,5 +1,28 @@
 2017-02-22  Antoine Quint  <graouts@apple.com>
 
+        [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>
+
         [Modern Media Controls] Controls bar may disappear while captions menu is visible
         https://bugs.webkit.org/show_bug.cgi?id=168751
         <rdar://problem/30663411>
index 2a87f27..7db9219 100644 (file)
@@ -126,6 +126,18 @@ class LayoutNode
 
     set children(children)
     {
+        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;
+        }
+
         while (this._children.length)
             this.removeChild(this._children[0]);