Attachment 340577 Details for Bug 185658 – Patch

[patch] Patch

bug-185658-20180517152803.patch (text/plain), 8.21 KB, created by Antoine Quint on 2018-05-17 06:28:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-05-17 06:28:05 PDT

Size: 8.21 KB

patch

obsolete

>Subversion Revision: 231891
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ae073322d0fbd8ef35315365458b069da8e51084..cfdbff8b296b8ddd327aeb31714b37dea24cb458 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-05-17  Antoine Quint  <graouts@apple.com>
>+
>+        [modern-media-controls] AirPlaySupport should be disabled by default
>+        https://bugs.webkit.org/show_bug.cgi?id=185658
>+        <rdar://problem/40272213>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We now only enable AirplaySupport if the controls are visible to the user _and_ media has played.
>+
>+        Test: media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html
>+
>+        * Modules/modern-media-controls/media/airplay-support.js:
>+        (AirplaySupport.prototype.enable):
>+        (AirplaySupport.prototype.controlsUserVisibilityDidChange):
>+        (AirplaySupport.prototype._shouldBeEnabled):
>+        (AirplaySupport):
>+
> 2018-05-17  Thibault Saunier  <tsaunier@igalia.com>
> 
>         [GStreamer]: Consider GstStream(Collection) as if if was not a GInitiallyUnowned
>diff --git a/Source/WebCore/Modules/modern-media-controls/media/airplay-support.js b/Source/WebCore/Modules/modern-media-controls/media/airplay-support.js
>index 248e576ce2e5e8e9d87a3e9d791fbc310c825cb5..473e73d97565c21d9a9d6533e93951bc7d495e90 100644
>--- a/Source/WebCore/Modules/modern-media-controls/media/airplay-support.js
>+++ b/Source/WebCore/Modules/modern-media-controls/media/airplay-support.js
>@@ -38,6 +38,12 @@ class AirplaySupport extends MediaControllerSupport
>         return ["webkitplaybacktargetavailabilitychanged", "webkitcurrentplaybacktargetiswirelesschanged"];
>     }
> 
>+    enable()
>+    {
>+        if (this._shouldBeEnabled())
>+            super.enable();
>+    }
>+
>     buttonWasPressed(control)
>     {
>         this.mediaController.media.webkitShowPlaybackTargetPicker();
>@@ -45,8 +51,7 @@ class AirplaySupport extends MediaControllerSupport
> 
>     controlsUserVisibilityDidChange()
>     {
>-        const controls = this.mediaController.controls;
>-        if (controls.visible && !controls.faded)
>+        if (this._shouldBeEnabled())
>             this.enable();
>         else
>             this.disable();
>@@ -67,4 +72,15 @@ class AirplaySupport extends MediaControllerSupport
>         this.mediaController.controls.muteButton.enabled = !this.control.on;
>     }
> 
>+    // Private
>+
>+    _shouldBeEnabled()
>+    {
>+        if (!this.mediaController.hasPlayed)
>+            return false;
>+
>+        const controls = this.mediaController.controls;
>+        return controls.visible && !controls.faded;
>+    }
>+
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 8cb8808d9a88f195f9921dbc707f76c65777905e..dc99e2d3c4e4c1b4ca0e68644a07739b236146a9 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-05-17  Antoine Quint  <graouts@apple.com>
>+
>+        [modern-media-controls] AirPlaySupport should be disabled by default
>+        https://bugs.webkit.org/show_bug.cgi?id=185658
>+        <rdar://problem/40272213>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new test that checks that we create an AirplaySupport object only after media has started playing.
>+
>+        * media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play-expected.txt: Added.
>+        * media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html: Added.
>+
> 2018-05-16  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         [WinCairo] Unreviewed gardening after WinCairoRequirements update.
>diff --git a/LayoutTests/media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play-expected.txt b/LayoutTests/media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..1fc8c57418a943738f637c12e78902e14491b70b
>--- /dev/null
>+++ b/LayoutTests/media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play-expected.txt
>@@ -0,0 +1,23 @@
>+Testing AirPlaySupport is only enabled once media has started playing.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+Starting test, the media can be played but hasn't started playing.
>+
>+Making AirPlay routes available.
>+
>+Airplay routes are available and controls are visible on the screen, but since the media hasn't played yet, AirplaySupport should be disabled.
>+PASS controls.airplayButton.uiDelegate is undefined.
>+PASS controls.visible is true
>+PASS controls.faded is false
>+
>+We play the media so that the Airplay button can be enabled.
>+
>+Media played, AirplaySupport should become enabled now.
>+PASS !!controls.airplayButton.uiDelegate became true
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html b/LayoutTests/media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..58b2c3a66bf4e76082401887ec16e37b9168a7d9
>--- /dev/null
>+++ b/LayoutTests/media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html
>@@ -0,0 +1,83 @@
>+<script src="../../../resources/js-test-pre.js"></script>
>+<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
>+<body>
>+<video src="../../content/test.mp4" style="width: 320px; height: 240px;" preload controls></video>
>+<div id="host"></div>
>+<script type="text/javascript">
>+
>+window.jsTestIsAsync = true;
>+
>+description("Testing <code>AirPlaySupport</code> is only enabled once media has started playing.");
>+
>+const container = document.querySelector("div#host");
>+const media = document.querySelector("video");
>+const mediaController = createControls(container, media, null);
>+const controls = mediaController.controls;
>+
>+controls.autoHideController.autoHideDelay = 100;
>+
>+media.addEventListener("canplay", startTest);
>+
>+function startTest()
>+{
>+    debug("Starting test, the media can be played but hasn't started playing.");
>+
>+    makeAirPlayAvailable();
>+}
>+
>+function makeAirPlayAvailable()
>+{
>+    debug("");
>+    debug("Making AirPlay routes available.");
>+    media.addEventListener("webkitplaybacktargetavailabilitychanged", playbackTargetAvailabilityChangedOnce, true);
>+    window.internals.setMockMediaPlaybackTargetPickerEnabled(true);
>+}
>+
>+function playbackTargetAvailabilityChangedOnce(event)
>+{
>+    media.removeEventListener('webkitplaybacktargetavailabilitychanged', playbackTargetAvailabilityChangedOnce, true);
>+    media.addEventListener('webkitplaybacktargetavailabilitychanged', playbackTargetAvailabilityChangedAgain, true);
>+
>+    window.internals.setMockMediaPlaybackTargetPickerState('Sleepy TV', 'DeviceAvailable');
>+}
>+
>+function playbackTargetAvailabilityChangedAgain(event)
>+{
>+    // setMockMediaPlaybackTargetPickerState happens asynchronously in WK2 and a
>+    // "webkitplaybacktargetavailabilitychanged" is always sent when an event listener
>+    // is added, so we may get a "not available" event first.
>+    if (event.availability == 'not-available')
>+        return;
>+
>+    media.removeEventListener('webkitplaybacktargetavailabilitychanged', playbackTargetAvailabilityChangedAgain, true);
>+
>+    debug("");
>+    debug("Airplay routes are available and controls are visible on the screen, but since the media hasn't played yet, AirplaySupport should be disabled.");
>+    shouldBeUndefined("controls.airplayButton.uiDelegate");
>+    shouldBeTrue("controls.visible");
>+    shouldBeFalse("controls.faded");
>+
>+    debug("");
>+    debug("We play the media so that the Airplay button can be enabled.");
>+    media.addEventListener("play", mediaDidPlay);
>+    media.play();
>+}
>+
>+function mediaDidPlay()
>+{
>+    debug("");
>+    debug("Media played, AirplaySupport should become enabled now.");
>+    shouldBecomeEqual("!!controls.airplayButton.uiDelegate", "true", finishTest);
>+}
>+
>+function finishTest()
>+{
>+    debug("");
>+    container.remove();
>+    media.remove();
>+    finishJSTest();
>+}
>+
>+</script>
>+<script src="../../../resources/js-test-post.js"></script>
>+</body>

Actions: View | Formatted Diff | Diff

Attachments on bug 185658: 340577