[Modern Media Controls] PiP button is not visible with a live broadcast video
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2017 20:55:21 +0000 (20:55 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2017 20:55:21 +0000 (20:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167794
<rdar://problem/30348790>

Patch by Antoine Quint <graouts@apple.com> on 2017-02-05
Reviewed by Dean Jackson.

Source/WebCore:

We were only listening to the "loadedmetadata", "error", "webkitpresentationmodechanged"
and "webkitcurrentplaybacktargetiswirelesschanged" events to invalidate the enabled state
for the PiP button. We also need to check availability of video tracks, which we already
did for fullscreen, which is quite similar.

So we now listen to "addtrack", "removetrack" and "change" events on the media.videoTracks
property, which correctly invalidates the PiP button when the first video track becomes
available or the last video track is removed.

Since a couple of other MediaControllerSupport subclasses (FullscreenSupport and TracksSupport)
would also listen to those events on various track types, we add a new "tracksToMonitor"
property on MediaControllerSupport which subclasses can override to provide a list of tracks
that should listen to those events. This removes the need for dedicated construction and
destruction time in MediaControllerSupport subclasses that need to listen to events on
tracks rather than the media itself.

Test: http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html

* Modules/modern-media-controls/media/fullscreen-support.js:
(FullscreenSupport):
(FullscreenSupport.prototype.get tracksToMonitor):
(FullscreenSupport.prototype.destroy): Deleted.
* Modules/modern-media-controls/media/media-controller-support.js:
(MediaControllerSupport):
(MediaControllerSupport.prototype.destroy):
(MediaControllerSupport.prototype.get tracksToMonitor):
* Modules/modern-media-controls/media/pip-support.js:
(PiPSupport.prototype.get tracksToMonitor):
* Modules/modern-media-controls/media/tracks-support.js:
(TracksSupport):
(TracksSupport.prototype.get tracksToMonitor):
(TracksSupport.prototype.destroy): Deleted.

LayoutTests:

Add a new test to check that a live broadcast video shows the picture-in-picture button.

* http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt: Added.
* http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html: Added.
* platform/mac/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html [new file with mode: 0644]
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js
Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js
Source/WebCore/Modules/modern-media-controls/media/pip-support.js
Source/WebCore/Modules/modern-media-controls/media/tracks-support.js

index 83e6f55..a30671e 100644 (file)
@@ -1,5 +1,19 @@
 2017-02-05  Antoine Quint  <graouts@apple.com>
 
+        [Modern Media Controls] PiP button is not visible with a live broadcast video
+        https://bugs.webkit.org/show_bug.cgi?id=167794
+        <rdar://problem/30348790>
+
+        Reviewed by Dean Jackson.
+
+        Add a new test to check that a live broadcast video shows the picture-in-picture button.
+
+        * http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt: Added.
+        * http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html: Added.
+        * platform/mac/TestExpectations:
+
+2017-02-05  Antoine Quint  <graouts@apple.com>
+
         [Modern Media Controls] Time labels may wrap instead of displaying on a single line
         https://bugs.webkit.org/show_bug.cgi?id=167835
         <rdar://problem/30340534>
diff --git a/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt b/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt
new file mode 100644 (file)
index 0000000..66af3a2
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the PiPSupport behavior with live broadcast.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS media.duration is Number.POSITIVE_INFINITY
+PASS mediaController.controls.pipButton.enabled is true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html b/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html
new file mode 100644 (file)
index 0000000..5710453
--- /dev/null
@@ -0,0 +1,31 @@
+<script src="../../../../../resources/js-test-pre.js"></script>
+<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
+<body>
+<video src="../../resources/hls/test-live.php" style="width: 800px;" autoplay></video>
+<div id="shadow"></div>
+<script type="text/javascript">
+
+window.jsTestIsAsync = true;
+
+description("Testing the <code>PiPSupport</code> behavior with live broadcast.");
+
+const shadowRoot = document.querySelector("div#shadow").attachShadow({ mode: "open" });
+const media = document.querySelector("video");
+const mediaController = createControls(shadowRoot, media, null);
+
+if (window.internals)
+    window.internals.settings.setAllowsPictureInPictureMediaPlayback(true);
+
+media.addEventListener("play", event => {
+    shouldBe("media.duration", "Number.POSITIVE_INFINITY");
+    shouldBeTrue("mediaController.controls.pipButton.enabled");
+
+    debug("");
+    shadowRoot.host.remove();
+    media.remove();
+    finishJSTest();
+});
+
+</script>
+<script src="../../../../../resources/js-test-post.js"></script>
+</body>
index f554da4..2943b10 100644 (file)
@@ -1457,6 +1457,7 @@ webkit.org/b/165530 compositing/layer-creation/fixed-position-out-of-view-scaled
 
 # picture-in-picture is not supported prior to Sierra.
 [ ElCapitan ] media/modern-media-controls/pip-support [ Skip ]
+[ ElCapitan ] http/tests/media/modern-media-controls/pip-support [ Skip ]
 [ ElCapitan ] media/modern-media-controls/media-controller/media-controller-inline-to-fullscreen-to-pip-to-inline.html [ Skip ]
 [ ElCapitan ] media/modern-media-controls/placard-support/placard-support-pip.html [ Skip ]
 
index 3e81ca3..456a4c2 100644 (file)
@@ -1,5 +1,46 @@
 2017-02-05  Antoine Quint  <graouts@apple.com>
 
+        [Modern Media Controls] PiP button is not visible with a live broadcast video
+        https://bugs.webkit.org/show_bug.cgi?id=167794
+        <rdar://problem/30348790>
+
+        Reviewed by Dean Jackson.
+
+        We were only listening to the "loadedmetadata", "error", "webkitpresentationmodechanged"
+        and "webkitcurrentplaybacktargetiswirelesschanged" events to invalidate the enabled state
+        for the PiP button. We also need to check availability of video tracks, which we already
+        did for fullscreen, which is quite similar.
+        
+        So we now listen to "addtrack", "removetrack" and "change" events on the media.videoTracks
+        property, which correctly invalidates the PiP button when the first video track becomes
+        available or the last video track is removed.
+
+        Since a couple of other MediaControllerSupport subclasses (FullscreenSupport and TracksSupport)
+        would also listen to those events on various track types, we add a new "tracksToMonitor"
+        property on MediaControllerSupport which subclasses can override to provide a list of tracks
+        that should listen to those events. This removes the need for dedicated construction and
+        destruction time in MediaControllerSupport subclasses that need to listen to events on
+        tracks rather than the media itself.
+
+        Test: http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html
+
+        * Modules/modern-media-controls/media/fullscreen-support.js:
+        (FullscreenSupport):
+        (FullscreenSupport.prototype.get tracksToMonitor):
+        (FullscreenSupport.prototype.destroy): Deleted.
+        * Modules/modern-media-controls/media/media-controller-support.js:
+        (MediaControllerSupport):
+        (MediaControllerSupport.prototype.destroy):
+        (MediaControllerSupport.prototype.get tracksToMonitor):
+        * Modules/modern-media-controls/media/pip-support.js:
+        (PiPSupport.prototype.get tracksToMonitor):
+        * Modules/modern-media-controls/media/tracks-support.js:
+        (TracksSupport):
+        (TracksSupport.prototype.get tracksToMonitor):
+        (TracksSupport.prototype.destroy): Deleted.
+
+2017-02-05  Antoine Quint  <graouts@apple.com>
+
         [Modern Media Controls] Time labels may wrap instead of displaying on a single line
         https://bugs.webkit.org/show_bug.cgi?id=167835
         <rdar://problem/30340534>
index 1c052e6..a18ae7a 100644 (file)
@@ -32,21 +32,6 @@ class FullscreenSupport extends MediaControllerSupport
 
         if (mediaController.controls instanceof IOSInlineMediaControls)
             mediaController.controls.delegate = this;
-
-        const videoTracks = mediaController.media.videoTracks;
-        for (let eventType of ["change", "addtrack", "removetrack"])
-            videoTracks.addEventListener(eventType, this);
-    }
-
-    // Public
-
-    destroy()
-    {
-        super.destroy();
-
-        const videoTracks = this.mediaController.media.videoTracks;
-        for (let eventType of ["change", "addtrack", "removetrack"])
-            videoTracks.removeEventListener(eventType, this);
     }
 
     // Protected
@@ -61,6 +46,11 @@ class FullscreenSupport extends MediaControllerSupport
         return ["loadedmetadata", "error"];
     }
 
+    get tracksToMonitor()
+    {
+        return [this.mediaController.media.videoTracks];
+    }
+
     buttonWasPressed(control)
     {
         const media = this.mediaController.media;
index 9b9ae41..55c42b1 100644 (file)
@@ -33,6 +33,11 @@ class MediaControllerSupport
         for (let eventType of this.mediaEvents)
             mediaController.media.addEventListener(eventType, this);
 
+        for (let tracks of this.tracksToMonitor) {
+            for (let eventType of ["change", "addtrack", "removetrack"])
+                tracks.addEventListener(eventType, this);
+        }
+
         if (!this.control)
             return;
 
@@ -49,6 +54,11 @@ class MediaControllerSupport
         for (let eventType of this.mediaEvents)
             media.removeEventListener(eventType, this);
 
+        for (let tracks of this.tracksToMonitor) {
+            for (let eventType of ["change", "addtrack", "removetrack"])
+                tracks.removeEventListener(eventType, this);
+        }
+
         if (this.control)
             this.control.uiDelegate = null;
     }
@@ -66,6 +76,12 @@ class MediaControllerSupport
         return [];
     }
 
+    get tracksToMonitor()
+    {
+        // Implemented by subclasses.
+        return [];
+    }
+
     buttonWasPressed(control)
     {
         // Implemented by subclasses.
index b839b3f..2a73a31 100644 (file)
@@ -41,6 +41,11 @@ class PiPSupport extends MediaControllerSupport
         return ["loadedmetadata", "error", "webkitpresentationmodechanged", "webkitcurrentplaybacktargetiswirelesschanged"];
     }
 
+    get tracksToMonitor()
+    {
+        return [this.mediaController.media.videoTracks];
+    }
+
     buttonWasPressed(control)
     {
         const media = this.mediaController.media;
index 0773d0f..2455d8b 100644 (file)
@@ -35,25 +35,6 @@ class TracksSupport extends MediaControllerSupport
 
         this.mediaController.controls.tracksPanel.dataSource = this;
         this.mediaController.controls.tracksPanel.uiDelegate = this;
-
-        const media = mediaController.media;
-        for (let tracks of [media.audioTracks, media.textTracks]) {
-            for (let eventType of ["addtrack", "change", "removetrack"])
-                tracks.addEventListener(eventType, this);
-        }
-    }
-
-    // Public
-
-    destroy()
-    {
-        super.destroy();
-
-        const media = this.mediaController.media;
-        for (let tracks of [media.audioTracks, media.textTracks]) {
-            for (let eventType of ["addtrack", "change", "removetrack"])
-                tracks.removeEventListener(eventType, this);
-        }
     }
 
     // Protected
@@ -68,6 +49,11 @@ class TracksSupport extends MediaControllerSupport
         return ["loadedmetadata"];
     }
 
+    get tracksToMonitor()
+    {
+        return [this.mediaController.media.audioTracks, this.mediaController.media.textTracks];
+    }
+
     buttonWasPressed(control)
     {
         this.mediaController.controls.showTracksPanel();