Changeset 228960 in webkit

Timestamp:

Feb 23, 2018 12:30:11 PM (6 years ago)

Author:

jmarcell@apple.com

Message:

Cherry-pick r228947. rdar://problem/37833040

Location:

branches/safari-605-branch

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt (added)
• LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html (added)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/modern-media-controls/media/media-controller.js (modified) (1 diff)
• Source/WebCore/Modules/modern-media-controls/media/start-support.js (modified) (3 diffs)

branches/safari-605-branch/LayoutTests/ChangeLog

@@ +1 @@
+2018-02-23  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r228947. rdar://problem/37833040
+
+    2018-02-22  Antoine Quint  <graouts@apple.com>
+
+            REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
+            https://bugs.webkit.org/show_bug.cgi?id=183074
+            <rdar://problem/37747028>
+
+            Reviewed by Eric Carlson.
+
+            Add a new test that set controls on the video, then immediately removes them, plays the video and turns the controls
+            back on as soon as the video starts to check that the "showsStartButton" property is false on the media controls.
+            Prior to this patch this test would fail.
+
+            * media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt: Added.
+            * media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html: Added.
+            * platform/ios/TestExpectations:
+
 2018-02-21  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605-branch/LayoutTests/platform/ios/TestExpectations

@@ -3222 +3222 @@
 media/modern-media-controls/placard-support/ipad [ Skip ]
 media/modern-media-controls/scrubber-support/ipad/scrubber-support-drag.html [ Skip ]
+media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html [ Skip ]
 
 # There is no focus state for on iOS

branches/safari-605-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-23  Jason Marcell  <jmarcell@apple.com>
+
+        Cherry-pick r228947. rdar://problem/37833040
+
+    2018-02-22  Antoine Quint  <graouts@apple.com>
+
+            REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
+            https://bugs.webkit.org/show_bug.cgi?id=183074
+            <rdar://problem/37747028>
+
+            Reviewed by Eric Carlson.
+
+            Test: media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html
+
+            In the fix for webkit.org/b/182668, we made it so that when the "controls" attribute is absent from a media
+            element we stop listening to the bulk of media events and prevent controls from updating any DOM properties
+            so as to minimize the amount of CPU usage by the Web process.
+
+            An unfortunate side effect was that, if the media controls were disabled at the time the video starts playing,
+            the StartSupport class would thus not catch the "play" event and would not be able to set the "hasPlayed"
+            property to "true" on the MediaController, which would then prevent the _shouldShowStartButton() from returning
+            "false". As a result, if the "controls" attribute was turned back on after the media started playing, they
+            would default to showing the start button, which would be then in the play state, ie. showing the pause icon.
+
+            We now set the "hasPlayed" property in the "play" event handler on MediaController, which is always registered
+            regardless of the "controls" attribute setting. We also ensure we invalidate the "showStartButton" property on
+            the media controls when StartSupport is enabled, which is the case when the "controls" attribute is toggled back
+            to "true" from a previous "false" value.
+
+            * Modules/modern-media-controls/media/media-controller.js:
+            (MediaController.prototype.handleEvent):
+            * Modules/modern-media-controls/media/start-support.js:
+            (StartSupport):
+            (StartSupport.prototype.enable):
+            (StartSupport.prototype.handleEvent):
+            (StartSupport.prototype._updateShowsStartButton):
+
 2018-02-23  Jason Marcell  <jmarcell@apple.com>
 

branches/safari-605-branch/Source/WebCore/Modules/modern-media-controls/media/media-controller.js

@@ -167 +167 @@
             scheduler.flushScheduledLayoutCallbacks();
         } else if (event.currentTarget === this.media) {
+            if (event.type === "play")
+                this.hasPlayed = true;
             this._updateControlsIfNeeded();
             this._updateControlsAvailability();

branches/safari-605-branch/Source/WebCore/Modules/modern-media-controls/media/start-support.js

@@ -31 +31 @@
         super(mediaController);
 
-        this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
+        this._updateShowsStartButton();
     }
 
@@ -41 +41 @@
     }
 
+    enable()
+    {
+        super.enable();
+
+        this._updateShowsStartButton();
+    }
+
     buttonWasPressed(control)
     {
@@ -48 +55 @@
     handleEvent(event)
     {
-        if (event.type === "play")
-            this.mediaController.hasPlayed = true;
+        super.handleEvent(event);
 
-        this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
-
-        super.handleEvent(event);
+        this._updateShowsStartButton();
     }
 
     // Private
+
+    _updateShowsStartButton()
+    {
+        this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
+    }
 
     _shouldShowStartButton()