Attachment 341523 Details for Bug 186063 – Patch

[patch] Patch

bug-186063-20180529151114.patch (text/plain), 15.61 KB, created by Jer Noble on 2018-05-29 15:11:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2018-05-29 15:11:15 PDT

Size: 15.61 KB

patch

obsolete

>Subversion Revision: 232115
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c463c68fd64a88817748e575dbaa14130127060d..26d2f15732b9b82266cc7a8125479c1694893357 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-05-29  Jer Noble  <jer.noble@apple.com>
>+
>+        Media elements outside fullscreen should not be considered main content.
>+        https://bugs.webkit.org/show_bug.cgi?id=186063
>+        <rdar://problem/40630437>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: platform/mac/media/video-best-element-for-playback-controls-purpose.html
>+
>+        Media elements outside the current fullscreen element are not visible, and thus should not be considered
>+        main content.
>+
>+        Drive-by fix: set the m_hasEverNotifiedAboutPlaying before dispatching the 'playing' event, so that
>+        tests can check bestMediaElementForShowingPlaybackControlsManager() in the 'playing' handler.
>+
>+        * html/HTMLMediaElement.cpp:
>+        (WebCore::HTMLMediaElement::notifyAboutPlaying):
>+        * html/HTMLMediaElement.h:
>+        * html/MediaElementSession.cpp:
>+        (WebCore::MediaElementSession::canShowControlsManager const):
>+        * testing/Internals.cpp:
>+        (WebCore::Internals::bestMediaElementForShowingPlaybackControlsManager):
>+        * testing/Internals.h:
>+        * testing/Internals.idl:
>+
> 2018-05-23  Jer Noble  <jer.noble@apple.com>
> 
>         [El Capitan] FAIL http/tests/security/webaudio-render-remote-audio-allowed-crossorigin.html
>diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
>index 25c7df1a4b4309c0449b48d8690163e707e52c41..fedbe0f061096eaeb5e55fd2e6c9dad0b43f27d3 100644
>--- a/Source/WebCore/html/HTMLMediaElement.cpp
>+++ b/Source/WebCore/html/HTMLMediaElement.cpp
>@@ -1120,10 +1120,10 @@ void HTMLMediaElement::notifyAboutPlaying(PlayPromiseVector&& pendingPlayPromise
> {
>     Ref<HTMLMediaElement> protectedThis(*this); // The 'playing' event can make arbitrary DOM mutations.
>     m_playbackStartedTime = currentMediaTime().toDouble();
>+    m_hasEverNotifiedAboutPlaying = true;
>     dispatchEvent(Event::create(eventNames().playingEvent, false, true));
>     resolvePendingPlayPromises(WTFMove(pendingPlayPromises));
> 
>-    m_hasEverNotifiedAboutPlaying = true;
>     scheduleUpdatePlaybackControlsManager();
> }
> 
>diff --git a/Source/WebCore/html/HTMLMediaElement.h b/Source/WebCore/html/HTMLMediaElement.h
>index 42109a3430b0953f21b143818872be598917fb89..e2f8ef424eceeb2f413b665f63dfd1aa12c4429e 100644
>--- a/Source/WebCore/html/HTMLMediaElement.h
>+++ b/Source/WebCore/html/HTMLMediaElement.h
>@@ -155,7 +155,7 @@ public:
> 
>     static HashSet<HTMLMediaElement*>& allMediaElements();
> 
>-    static HTMLMediaElement* bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose);
>+    WEBCORE_EXPORT static HTMLMediaElement* bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose);
> 
>     static bool isRunningDestructor();
> 
>diff --git a/Source/WebCore/html/MediaElementSession.cpp b/Source/WebCore/html/MediaElementSession.cpp
>index 03b106045c6dc055c5a30aa9bd5934f369838396..e8dd9592121cf87e6e40504a8075618cd1066e4b 100644
>--- a/Source/WebCore/html/MediaElementSession.cpp
>+++ b/Source/WebCore/html/MediaElementSession.cpp
>@@ -473,6 +473,13 @@ bool MediaElementSession::canShowControlsManager(PlaybackControlsPurpose purpose
>         return false;
>     }
> 
>+    // Elements which are not descendents of the current fullscreen element cannot be main content.
>+    auto* fullscreenElement = m_element.document().webkitCurrentFullScreenElement();
>+    if (fullscreenElement && !m_element.isDescendantOf(*fullscreenElement)) {
>+        INFO_LOG(LOGIDENTIFIER, "returning FALSE: outside of full screen");
>+        return false;
>+    }
>+
>     // Only allow the main content heuristic to forbid videos from showing up if our purpose is the controls manager.
>     if (purpose == PlaybackControlsPurpose::ControlsManager && m_element.isVideo()) {
>         if (!m_element.renderer()) {
>diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
>index f0b8c55aaf25c6f0b5a27ca1732e71fe1ba3568b..18da49eb8e220156a8b9e08c485d09c25bbe9e43 100644
>--- a/Source/WebCore/testing/Internals.cpp
>+++ b/Source/WebCore/testing/Internals.cpp
>@@ -3707,6 +3707,13 @@ ExceptionOr<Internals::NowPlayingState> Internals::nowPlayingState() const
> #endif
> }
> 
>+#if ENABLE(VIDEO)
>+HTMLMediaElement* Internals::bestMediaElementForShowingPlaybackControlsManager(Internals::PlaybackControlsPurpose purpose)
>+{
>+    return HTMLMediaElement::bestMediaElementForShowingPlaybackControlsManager(purpose);
>+}
>+#endif
>+
> #if ENABLE(WIRELESS_PLAYBACK_TARGET)
> 
> void Internals::setMockMediaPlaybackTargetPickerEnabled(bool enabled)
>diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
>index 8ac4dd7b15609f6940bd38f6858d3dd66db560b3..3880420ca548649259448badf6db3f8ad6fc7b08 100644
>--- a/Source/WebCore/testing/Internals.h
>+++ b/Source/WebCore/testing/Internals.h
>@@ -40,6 +40,10 @@
> #include "MediaSessionInterruptionProvider.h"
> #endif
> 
>+#if ENABLE(VIDEO)
>+#include "MediaElementSession.h"
>+#endif
>+
> namespace WebCore {
> 
> class AnimationTimeline;
>@@ -680,6 +684,9 @@ public:
>     };
>     ExceptionOr<NowPlayingState> nowPlayingState() const;
> 
>+    using PlaybackControlsPurpose = MediaElementSession::PlaybackControlsPurpose;
>+    HTMLMediaElement* bestMediaElementForShowingPlaybackControlsManager(PlaybackControlsPurpose);
>+
> private:
>     explicit Internals(Document&);
>     Document* contextDocument() const;
>diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
>index 35b2f2822daea30b419943d0151a2631656bd180..e908f740438fa2b035a9e4acd29edcf736cc62ab 100644
>--- a/Source/WebCore/testing/Internals.idl
>+++ b/Source/WebCore/testing/Internals.idl
>@@ -82,6 +82,11 @@ enum EventThrottlingBehavior {
>     "unresponsive"
> };
> 
>+[Conditional=VIDEO] enum PlaybackControlsPurpose {
>+    "ControlsManager",
>+    "NowPlaying"
>+};
>+
> [
>     ExportMacro=WEBCORE_TESTSUPPORT_EXPORT,
>     Conditional=VIDEO,
>@@ -616,4 +621,6 @@ enum EventThrottlingBehavior {
>     boolean usingAppleInternalSDK();
> 
>     [Conditional=VIDEO, MayThrowException] readonly attribute NowPlayingState nowPlayingState;
>+
>+    [Conditional=VIDEO] HTMLMediaElement bestMediaElementForShowingPlaybackControlsManager(PlaybackControlsPurpose purpose);
> };
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6f8d10cf2fdcd5845b7cf99adb87e4caaaa9f1cd..25f662d8afb421de4daa82f7de1659a946d1f5bf 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-05-29  Jer Noble  <jer.noble@apple.com>
>+
>+        Media elements outside fullscreen should not be considered main content.
>+        https://bugs.webkit.org/show_bug.cgi?id=186063
>+        <rdar://problem/40630437>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/mac/media/video-best-element-for-playback-controls-purpose-expected.txt: Added.
>+        * platform/mac/media/video-best-element-for-playback-controls-purpose.html: Added.
>+
> 2018-05-23  Jer Noble  <jer.noble@apple.com>
> 
>         [El Capitan] FAIL http/tests/security/webaudio-render-remote-audio-allowed-crossorigin.html
>diff --git a/LayoutTests/platform/mac/media/video-best-element-for-playback-controls-purpose-expected.txt b/LayoutTests/platform/mac/media/video-best-element-for-playback-controls-purpose-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..9ac0126dc60169122c04036d26b20e513269fb1e
>--- /dev/null
>+++ b/LayoutTests/platform/mac/media/video-best-element-for-playback-controls-purpose-expected.txt
>@@ -0,0 +1,49 @@
>+
>+Unloaded video elements should not be considered main content.
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == 'null') OK
>+
>+Large, autoplay videos with video and audio should be considered main content.
>+RUN(video = createVideo({autoplay: true, type: "audio+video", size: "large"}))
>+EVENT(playing)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == '[object HTMLVideoElement]') OK
>+
>+Small, autoplay videos with video and audio should be considered main content.
>+RUN(video = createVideo({autoplay: true, type: "audio+video", size: "small"}))
>+EVENT(playing)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == 'null') OK
>+
>+Muted autoplay videos should not be considered main content.
>+RUN(video = createVideo({autoplay: true, muted: true, type: "audio+video", size: "large"}))
>+EVENT(playing)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == 'null') OK
>+
>+Video-only autoplay videos should not be considered main content.
>+RUN(video = createVideo({autoplay: true, type: "video", size: "large"}))
>+EVENT(playing)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == 'null') OK
>+
>+Non-playing videos should not be considered main content.
>+RUN(video = createVideo({type: "audio+video", size: "large"}))
>+EVENT(canplaythrough)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == 'null') OK
>+
>+Large, autoplay videos outside fullscreen element should not be considered main content
>+RUN(video = createVideo({autoplay: true, muted: true, type: "audio+video", size: "large"}))
>+EVENT(playing)
>+RUN(document.querySelector("#fullscreen").webkitRequestFullscreen())
>+EVENT(webkitfullscreenchange)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == 'null') OK
>+RUN(document.webkitExitFullscreen())
>+EVENT(webkitfullscreenchange)
>+
>+Large, autoplay videos inside fullscreen element should be considered main content
>+RUN(video = createVideo({autoplay: true, type: "audio+video", size: "large"}))
>+EVENT(playing)
>+RUN(document.querySelector("#target").webkitRequestFullscreen())
>+EVENT(webkitfullscreenchange)
>+EXPECTED (internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager") == '[object HTMLVideoElement]') OK
>+RUN(document.webkitExitFullscreen())
>+EVENT(webkitfullscreenchange)
>+
>+END OF TEST
>+
>diff --git a/LayoutTests/platform/mac/media/video-best-element-for-playback-controls-purpose.html b/LayoutTests/platform/mac/media/video-best-element-for-playback-controls-purpose.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2f67b120fc79aa433ecf7e3027cda2b2dd06448d
>--- /dev/null
>+++ b/LayoutTests/platform/mac/media/video-best-element-for-playback-controls-purpose.html
>@@ -0,0 +1,129 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <title>audio-session-category-track-change</title>
>+    <script src="../../../media/video-test.js"></script>
>+    <script src="../../../media/media-file.js"></script>
>+    <script>
>+    async function go() {
>+        for (test of tests) {
>+            await test();
>+            consoleWrite('');
>+        }
>+        endTest();
>+    }
>+
>+    function createVideo(parameters) {
>+        let target = document.querySelector('#target');
>+        target.innerHTML = '';
>+
>+        let video = document.createElement('video');
>+        target.appendChild(video);
>+
>+        if (!parameters)
>+            return video;
>+
>+        if (parameters.autoplay)
>+            video.autoplay = parameters.autoplay;
>+
>+        if (parameters.muted)
>+            video.muted = parameters.muted;
>+
>+        if (parameters.size) {
>+            if (parameters.size === 'large') {
>+                video.width = 640;
>+                video.height = 480;
>+            } else if (parameters.size === 'small') {
>+                video.width = 320;
>+                video.height = 240;
>+            }
>+        }
>+
>+        if (parameters.type) {
>+            if (parameters.type === 'audio+video')
>+                video.src = findMediaFile("video", "../../../media/content/test");
>+            else if (parameters.type === 'video')
>+                video.src = findMediaFile("video", "../../../media/content/long-test");
>+        }
>+
>+        return video;
>+    }
>+
>+    function enterFullscreen(target) {
>+        runWithKeyDown(`document.querySelector("${target}").webkitRequestFullscreen()`);
>+        return waitFor(document, 'webkitfullscreenchange');
>+    }
>+
>+    function exitFullscreen() {
>+        runWithKeyDown('document.webkitExitFullscreen()');
>+        return waitFor(document, 'webkitfullscreenchange');
>+    }
>+
>+    let tests = [
>+        function() {
>+            consoleWrite('Unloaded video elements should not be considered main content.');
>+            video = createVideo();
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', null)
>+        },
>+
>+        async function() {
>+            consoleWrite('Large, autoplay videos with video and audio should be considered main content.')
>+            run('video = createVideo({autoplay: true, type: "audio+video", size: "large"})');
>+            await waitFor(video, 'playing');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', video)
>+        },
>+
>+        async function() {
>+            consoleWrite('Small, autoplay videos with video and audio should be considered main content.')
>+            run('video = createVideo({autoplay: true, type: "audio+video", size: "small"})');
>+            await waitFor(video, 'playing');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', null)
>+        },
>+
>+        async function() {
>+            consoleWrite('Muted autoplay videos should not be considered main content.')
>+            run('video = createVideo({autoplay: true, muted: true, type: "audio+video", size: "large"})');
>+            await waitFor(video, 'playing');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', null)
>+        },
>+
>+        async function() {
>+            consoleWrite('Video-only autoplay videos should not be considered main content.')
>+            run('video = createVideo({autoplay: true, type: "video", size: "large"})');
>+            await waitFor(video, 'playing');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', null)
>+        },
>+
>+        async function() {
>+            consoleWrite('Non-playing videos should not be considered main content.')
>+            run('video = createVideo({type: "audio+video", size: "large"})');
>+            await waitFor(video, 'canplaythrough');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', null)
>+        },
>+
>+        async function() {
>+            consoleWrite('Large, autoplay videos outside fullscreen element should not be considered main content');
>+            run('video = createVideo({autoplay: true, muted: true, type: "audio+video", size: "large"})');
>+            await waitFor(video, 'playing');
>+            await enterFullscreen('#fullscreen');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', null);
>+            await exitFullscreen();
>+        },
>+
>+        async function() {
>+            consoleWrite('Large, autoplay videos inside fullscreen element should be considered main content');
>+            run('video = createVideo({autoplay: true, type: "audio+video", size: "large"})');
>+            await waitFor(video, 'playing');
>+            await enterFullscreen('#target');
>+            testExpected('internals.bestMediaElementForShowingPlaybackControlsManager("ControlsManager")', video);
>+            await exitFullscreen();
>+        },
>+    ];
>+
>+    </script>
>+</head>
>+<body onload="go()">
>+    <div id="target"></div>
>+    <div id="fullscreen"></div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 186063: 341523 | 341528