Attachment 343945 Details for Bug 186226 – Patch

[patch] Patch

bug-186226-20180629143733.patch (text/plain), 17.67 KB, created by Jeremy Jones on 2018-06-29 14:37:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Jones

Created: 2018-06-29 14:37:33 PDT

Size: 17.67 KB

patch

obsolete

>Subversion Revision: 233214
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f049638b707a069c4acc01e0bc84d470d5043c0c..9b13c34fa36366fe93e7aad19d7795ad7745cc0c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-06-29  Jeremy Jones  <jeremyj@apple.com>
>+
>+        Fullscreen requires active document.
>+        https://bugs.webkit.org/show_bug.cgi?id=186226
>+        rdar://problem/36187413
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: media/no-fullscreen-when-hidden.html
>+
>+        This change guarantees the document to be visible for both element fullscreen and video fullscreen.
>+
>+        User gesture is not enough to guarantee that the document is visible when fullscreen is initiated
>+        because JavaScript can spin wait before initiating fullscreen. During that spin the page or window might
>+        be hidden.
>+
>+        Document::hidden() can't be relied upon because it won't update while JavaScript spins.
>+
>+        This change adds a sync call to the UI process to get the current UI visibility state.
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::requestFullScreenForElement):
>+        * html/HTMLMediaElement.cpp:
>+        (WebCore::HTMLMediaElement::enterFullscreen):
>+        * page/ChromeClient.h:
>+
> 2018-06-26  Eric Carlson  <eric.carlson@apple.com>
> 
>         [Mac] AirPlay picker uses incorrect theme in Dark mode
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 766381d2e378d311a118d0724b7016db11b8eabc..13e8fa86c60dd15bc38b07c824fc366365daa0d8 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-06-29  Jeremy Jones  <jeremyj@apple.com>
>+
>+        Fullscreen requires active document.
>+        https://bugs.webkit.org/show_bug.cgi?id=186226
>+        rdar://problem/36187413
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This change guarantees the document to be visible for both element fullscreen and video fullscreen.
>+
>+        User gesture is not enough to guarantee that the document is visible when fullscreen is initiated
>+        because JavaScript can spin wait before initiating fullscreen. During that spin the page or window might
>+        be hidden.
>+
>+        Document::hidden() can't be relied upon because it won't update while JavaScript spins.
>+
>+        This change adds a sync call to the UI process to get the current UI visibility state.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::getIsViewVisible):
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/WebPageProxy.messages.in:
>+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
>+        (WebKit::WebChromeClient::isViewVisible):
>+        * WebProcess/WebCoreSupport/WebChromeClient.h:
>+
> 2018-06-26  Eric Carlson  <eric.carlson@apple.com>
> 
>         [Mac] AirPlay picker uses incorrect theme in Dark mode
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 22d495b837f8ebb82fa157e9170f80248603efc3..18c1e6f4126cac1568975af1897267527686c24d 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -6015,6 +6015,10 @@ void Document::requestFullScreenForElement(Element* element, FullScreenCheckType
>         // an event named fullscreenerror with its bubbles attribute set to true on the context object's 
>         // node document:
> 
>+        // Don't allow fullscreen if document is hidden.
>+        if (!page() || !page()->chrome().client().isViewVisible())
>+            break;
>+
>         // The context object is not in a document.
>         if (!element->isConnected())
>             break;
>diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
>index 628b0a0a1a760cf2e31deda3b70ae74307ad5861..d573f7d26f99b5befd72ffba317a6c51f544f012 100644
>--- a/Source/WebCore/html/HTMLMediaElement.cpp
>+++ b/Source/WebCore/html/HTMLMediaElement.cpp
>@@ -5880,6 +5880,11 @@ void HTMLMediaElement::enterFullscreen(VideoFullscreenMode mode)
>     if (m_videoFullscreenMode == mode)
>         return;
> 
>+    if (!document().page() || !document().page()->chrome().client().isViewVisible()) {
>+        ALWAYS_LOG(LOGIDENTIFIER, "  returning because document is hidden");
>+        return;
>+    }
>+
>     m_temporarilyAllowingInlinePlaybackAfterFullscreen = false;
>     m_waitingToEnterFullscreen = true;
> 
>@@ -5906,7 +5911,7 @@ void HTMLMediaElement::enterFullscreen(VideoFullscreenMode mode)
>     configureMediaControls();
>     if (hasMediaControls())
>         mediaControls()->enteredFullscreen();
>-    if (document().page() && is<HTMLVideoElement>(*this)) {
>+    if (is<HTMLVideoElement>(*this)) {
>         HTMLVideoElement& asVideo = downcast<HTMLVideoElement>(*this);
>         if (document().page()->chrome().client().supportsVideoFullscreen(m_videoFullscreenMode)) {
>             document().page()->chrome().client().enterVideoFullscreenForVideoElement(asVideo, m_videoFullscreenMode, m_videoFullscreenStandby);
>diff --git a/Source/WebCore/page/ChromeClient.h b/Source/WebCore/page/ChromeClient.h
>index bea0d8f4cb8ad34b1e996ebf318cffb0fb968be7..472b6183fc7bc2dbffbea0a59735e7cab0f50ce9 100644
>--- a/Source/WebCore/page/ChromeClient.h
>+++ b/Source/WebCore/page/ChromeClient.h
>@@ -487,6 +487,8 @@ public:
> 
>     virtual String signedPublicKeyAndChallengeString(unsigned, const String&, const URL&) const { return emptyString(); }
> 
>+    virtual bool isViewVisible() { return true; }
>+
> protected:
>     virtual ~ChromeClient() = default;
> };
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 99aeede341f440959b44853de693b4eeddde6fd2..f19f93ab1900d5f9c57d14e99113ac335df9ab9f 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -7802,4 +7802,9 @@ void WebPageProxy::setDefersLoadingForTesting(bool defersLoading)
>     m_process->send(Messages::WebPage::SetDefersLoading(defersLoading), m_pageID);
> }
> 
>+void WebPageProxy::getIsViewVisible(bool& result)
>+{
>+    result = isViewVisible();
>+}
>+
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 2b820f2d5af58134b1b547fd1d859c032e0b5b69..b86a2ba3e005383dd711114612c57139958429f6 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1439,6 +1439,7 @@ private:
>     void getMenuBarIsVisible(Messages::WebPageProxy::GetMenuBarIsVisible::DelayedReply&&);
>     void setStatusBarIsVisible(bool statusBarIsVisible);
>     void getStatusBarIsVisible(Messages::WebPageProxy::GetStatusBarIsVisible::DelayedReply&&);
>+    void getIsViewVisible(bool&);
>     void setIsResizable(bool isResizable);
>     void screenToRootView(const WebCore::IntPoint& screenPoint, Messages::WebPageProxy::ScreenToRootView::DelayedReply&&);
>     void rootViewToScreen(const WebCore::IntRect& viewRect, Messages::WebPageProxy::RootViewToScreen::DelayedReply&&);
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>index 4e1de5ca22ae00cc6f6f0216ddda60986345279d..c7e8756262d13a07e630629fcbd4477092b1e224 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -61,6 +61,7 @@ messages -> WebPageProxy {
>     GetWindowFrame() -> (WebCore::FloatRect windowFrame) Delayed
>     ScreenToRootView(WebCore::IntPoint screenPoint) -> (WebCore::IntPoint windowPoint) Delayed
>     RootViewToScreen(WebCore::IntRect rect) -> (WebCore::IntRect screenFrame) Delayed
>+    GetIsViewVisible() -> (bool result)
> 
> #if PLATFORM(COCOA)
>     ShowValidationMessage(WebCore::IntRect anchorRect, String message)
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>index 0c955f6771beccbe568f0242479f16301f5d1f4c..c0652f16dcd87f91641d56a47cf92fee3d53a6f5 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>@@ -1311,4 +1311,11 @@ void WebChromeClient::requestStorageAccess(String&& subFrameHost, String&& topFr
> }
> #endif
> 
>+bool WebChromeClient::isViewVisible()
>+{
>+    bool isVisible = false;
>+    WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::GetIsViewVisible(), Messages::WebPageProxy::GetIsViewVisible::Reply(isVisible), m_page.pageID());
>+    return isVisible;
>+}
>+
> } // namespace WebKit
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
>index 811a822c94534366a75031115e9a0faa93f427d8..e5d8e433e4107032466963eaa8ea45a34987776f 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
>@@ -367,6 +367,8 @@ private:
>     void requestStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void (bool)>&&) final;
> #endif
> 
>+    bool isViewVisible() final;
>+
>     String m_cachedToolTip;
>     mutable RefPtr<WebFrame> m_cachedFrameSetLargestFrame;
>     mutable bool m_cachedMainFrameHasHorizontalScrollbar { false };
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index ed0c1a82c6cea88434e7290251e733eacfa7c0fc..4dae0effae8dd25f7d6f10d577f36ac183f9af47 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-06-29  Jeremy Jones  <jeremyj@apple.com>
>+
>+        Fullscreen requires active document.
>+        https://bugs.webkit.org/show_bug.cgi?id=186226
>+        rdar://problem/36187413
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This change guarantees the document to be visible for both element fullscreen and video fullscreen.
>+
>+        User gesture is not enough to guarantee that the document is visible when fullscreen is initiated
>+        because JavaScript can spin wait before initiating fullscreen. During that spin the page or window might
>+        be hidden.
>+
>+        Document::hidden() can't be relied upon because it won't update while JavaScript spins.
>+
>+        This change adds a sync call to the UI process to get the current UI visibility state.
>+
>+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
>+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
>+        (WTR::TestRunner::setViewVisible):
>+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
>+        * WebKitTestRunner/PlatformWebView.h:
>+        * WebKitTestRunner/TestInvocation.cpp:
>+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
>+        * WebKitTestRunner/mac/PlatformWebViewMac.mm:
>+        (WTR::PlatformWebView::setViewVisible):
>+
> 2018-06-26  Daniel Bates  <dabates@apple.com>
> 
>         contributors.json fails to parse after r233209
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>index 1791668127a6660772ca191c05839d6cc08f3964..93c70ee78fe3c5d322d213c52879aac110651f98 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>+++ b/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
>@@ -343,4 +343,5 @@ interface TestRunner {
>     void clearMockMediaDevices();
>     void removeMockMediaDevice(DOMString persistentId);
>     void resetMockMediaDevices();
>+    void setViewVisible(boolean value);
> };
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>index 95aaef4cda0f88c28a1df8e3efa2c551ea7b73d8..77d6afe5332aa81b34ce2a4726bd6c0c44b78640 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>@@ -2036,6 +2036,14 @@ void TestRunner::resetMockMediaDevices()
>     WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), nullptr, nullptr);
> }
> 
>+void TestRunner::setViewVisible(bool value)
>+{
>+    WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("SetViewVisible"));
>+    WKRetainPtr<WKBooleanRef> messageBody(AdoptWK, WKBooleanCreate(value));
>+
>+    WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), nullptr);
>+}
>+
> #if PLATFORM(MAC)
> void TestRunner::connectMockGamepad(unsigned index)
> {
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>index 789ba2302368309ca099a1efb7457e17a25326f2..2ee7fed5f22fc7a1b6d3c0a036f106ee10090afc 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
>@@ -452,6 +452,7 @@ public:
>     void clearMockMediaDevices();
>     void removeMockMediaDevice(JSStringRef persistentId);
>     void resetMockMediaDevices();
>+    void setViewVisible(bool value);
> 
> private:
>     TestRunner();
>diff --git a/Tools/WebKitTestRunner/PlatformWebView.h b/Tools/WebKitTestRunner/PlatformWebView.h
>index 6d0e9b0c7485fb5a6381e47692064c69262a0965..33ddd797ab2795f603f40abb01cca2bea45592f5 100644
>--- a/Tools/WebKitTestRunner/PlatformWebView.h
>+++ b/Tools/WebKitTestRunner/PlatformWebView.h
>@@ -93,6 +93,7 @@ public:
>     void makeWebViewFirstResponder();
>     void setWindowIsKey(bool);
>     bool windowIsKey() const { return m_windowIsKey; }
>+    void setViewVisible(bool);
>     
>     void removeFromWindow();
>     void addToWindow();
>diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp
>index 05c3ce61a8bef3e89569fc56bc3c97b2d7888760..82cc5f2ad05000a25f88f4c8e843f8821c692e7b 100644
>--- a/Tools/WebKitTestRunner/TestInvocation.cpp
>+++ b/Tools/WebKitTestRunner/TestInvocation.cpp
>@@ -949,6 +949,13 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
>         return nullptr;
>     }
> 
>+    if (WKStringIsEqualToUTF8CString(messageName, "SetViewVisible")) {
>+
>+        WKBooleanRef value = static_cast<WKBooleanRef>(messageBody);
>+        TestController::singleton().mainWebView()->setViewVisible(WKBooleanGetValue(value));
>+        return nullptr;
>+    }
>+
> #if PLATFORM(MAC)
>     if (WKStringIsEqualToUTF8CString(messageName, "ConnectMockGamepad")) {
>         ASSERT(WKGetTypeID(messageBody) == WKUInt64GetTypeID());
>diff --git a/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm b/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm
>index 115cf0395a335559a10f98a32966dbeeb182db79..e19ea915cd642e581c672e7857b20b754879643d 100644
>--- a/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm
>+++ b/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm
>@@ -168,6 +168,14 @@ void PlatformWebView::setWindowIsKey(bool isKey)
>         [m_window resignKeyWindow];
> }
> 
>+void PlatformWebView::setViewVisible(bool value)
>+{
>+    if (value)
>+        [m_window orderFront:nil];
>+    else
>+        [m_window orderOut:nil];
>+}
>+
> void PlatformWebView::resizeTo(unsigned width, unsigned height, WebViewSizingMode sizingMode)
> {
>     WKRect frame = windowFrame();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6defdd6d9ac1fe04fed4c7a9b7b0efa7eb9001b7..30950ad8545c865373a09310b7e7256778415eae 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-06-29  Jeremy Jones  <jeremyj@apple.com>
>+
>+        Fullscreen requires active document.
>+        https://bugs.webkit.org/show_bug.cgi?id=186226
>+        rdar://problem/36187413
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This change guarantees the document to be visible for both element fullscreen and video fullscreen.
>+
>+        User gesture is not enough to guarantee that the document is visible when fullscreen is initiated
>+        because JavaScript can spin wait before initiating fullscreen. During that spin the page or window might
>+        be hidden.
>+
>+        Document::hidden() can't be relied upon because it won't update while JavaScript spins.
>+
>+        This change adds a sync call to the UI process to get the current UI visibility state.
>+
>+        * media/no-fullscreen-when-hidden-expected.txt: Added.
>+        * media/no-fullscreen-when-hidden.html: Added.
>+
> 2018-06-26  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r233184.
>diff --git a/LayoutTests/media/no-fullscreen-when-hidden-expected.txt b/LayoutTests/media/no-fullscreen-when-hidden-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..2b293a9038d7106f64816200ba992dbc2e0bf9cb
>--- /dev/null
>+++ b/LayoutTests/media/no-fullscreen-when-hidden-expected.txt
>@@ -0,0 +1,6 @@
>+Fullscreen should be prevented when page is not visible.
>+
>+RUN(testRunner.setViewVisible(false))
>+Attempting to go into Full Screen
>+END OF TEST
>+
>diff --git a/LayoutTests/media/no-fullscreen-when-hidden.html b/LayoutTests/media/no-fullscreen-when-hidden.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2133fac77fb3798be1cad804c364039ce88ecccb
>--- /dev/null
>+++ b/LayoutTests/media/no-fullscreen-when-hidden.html
>@@ -0,0 +1,32 @@
>+<html>
>+<head>
>+    <title>Test fullscreen video going into pip should exit fullscreen</title>
>+    <script src="media-file.js"></script>
>+    <script src="video-test.js"></script>
>+    <script>
>+        function init()
>+        {
>+            findMediaElement();
>+            run('testRunner.setViewVisible(false)');
>+            video.addEventListener('canplaythrough', oncanplaythrough);
>+            video.src = findMediaFile('video', 'content/test');
>+        }
>+
>+        function oncanplaythrough()
>+        {
>+            consoleWrite("Attempting to go into Full Screen");
>+            waitForEventAndFail('webkitpresentationmodechanged');
>+            waitForEventAndFail('webkitbeginfullscreen');
>+            runWithKeyDown(function(){ 
>+                video.webkitRequestFullscreen(); 
>+                endTestLater()
>+            });
>+        }
>+    </script>
>+</head>
>+<body onload="init()">
>+    Fullscreen should be prevented when page is not visible.<br>
>+    <video controls></video>
>+</body>
>+</html>
>+

Flags:

jer.noble: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 186226: 341832 | 341834 | 341835 | 341836 | 341840 | 341868 | 343833 | 343859 | 343873 | 343945 | 343956 | 343957 | 343965 | 343972 | 343985 | 343995 | 344478 | 344494 | 344500 | 344727 | 344733 | 344735 | 344737 | 344739 | 344908 | 344910 | 344916 | 344919 | 344921 | 344923 | 344925 | 345028 | 345030 | 345031 | 345111