Title: [282175] trunk/Source/WebKit
Revision
282175
Author
peng.l...@apple.com
Date
2021-09-08 15:03:32 -0700 (Wed, 08 Sep 2021)

Log Message

[macOS] -[WKFullScreenWindowController exitFullScreenImmediately] does not exit fullscreen immediately
https://bugs.webkit.org/show_bug.cgi?id=230024

Reviewed by Jer Noble.

A WebContent process may ask the `WKFullScreenWindowController` in
the UI process to exit fullscreen immediately without walking through
the normal exit fullscreen sequence, and the WebContent process won't
expect any IPC message related to fullscreen to come back from the
UI process. If the UI process sends an IPC message to the WebContent
process and expects the response from the WebContent process (e.g., in
`WebPageProxy::forceRepaint()`), the UI process will be stuck.

That will happen when a tab navigates to the previous page while a video
element in the current page is playing in fullscreen. The reason is that
`-[WKFullScreenWindowController exitFullScreen]` is called before
`-[WKFullScreenWindowController exitFullScreenImmediately]`.
`-[WKFullScreenWindowController exitFullScreen]` changes `_fullScreenState`
to `WaitingToExitFullScreen`, so `-[WKFullScreenWindowController exitFullScreenImmediately]`
will return early. This patch fixes this issue by changing the early return condition.

In addition, `-[WKFullScreenWindowController exitFullScreenImmediately]`
does not really exit fullscreen immediately. Instead, it calls
`-[WKFullScreenWindowController finishedExitFullScreenAnimation:]`,
which always calls `WebPageProxy::forceRepaint()` and expects a response
from the WebContet process. This patch fixes this issue as well.

Tested manually.

* UIProcess/mac/WKFullScreenWindowController.mm:
(-[WKFullScreenWindowController exitFullScreenImmediately]):
Update the early return condition to make sure the UI process will proceed
to exit fullscreen unless the current state is `NotInFullScreen`.
(-[WKFullScreenWindowController beganExitFullScreenWithInitialFrame:finalFrame:]):
(-[WKFullScreenWindowController finishedExitFullScreenAnimation:]):
The `completed` parameter was not used. This patch renames it to `immediately` to
indicate whether `WKFullScreenWindowController` wants to exit fullscreen immediately.
(-[WKFullScreenWindowController close]):
Clean up this function after revising `-[WKFullScreenWindowController exitFullScreenImmediately]`.
(-[WKFullScreenWindowController windowDidFailToExitFullScreen:]):
Update the argument according to the change of `-[WKFullScreenWindowController finishedExitFullScreenAnimation:]`.
(-[WKFullScreenWindowController windowDidExitFullScreen:]): Ditto.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (282174 => 282175)


--- trunk/Source/WebKit/ChangeLog	2021-09-08 21:56:20 UTC (rev 282174)
+++ trunk/Source/WebKit/ChangeLog	2021-09-08 22:03:32 UTC (rev 282175)
@@ -1,3 +1,48 @@
+2021-09-08  Peng Liu  <peng.l...@apple.com>
+
+        [macOS] -[WKFullScreenWindowController exitFullScreenImmediately] does not exit fullscreen immediately
+        https://bugs.webkit.org/show_bug.cgi?id=230024
+
+        Reviewed by Jer Noble.
+
+        A WebContent process may ask the `WKFullScreenWindowController` in
+        the UI process to exit fullscreen immediately without walking through
+        the normal exit fullscreen sequence, and the WebContent process won't
+        expect any IPC message related to fullscreen to come back from the
+        UI process. If the UI process sends an IPC message to the WebContent
+        process and expects the response from the WebContent process (e.g., in
+        `WebPageProxy::forceRepaint()`), the UI process will be stuck.
+
+        That will happen when a tab navigates to the previous page while a video
+        element in the current page is playing in fullscreen. The reason is that
+        `-[WKFullScreenWindowController exitFullScreen]` is called before
+        `-[WKFullScreenWindowController exitFullScreenImmediately]`.
+        `-[WKFullScreenWindowController exitFullScreen]` changes `_fullScreenState`
+        to `WaitingToExitFullScreen`, so `-[WKFullScreenWindowController exitFullScreenImmediately]`
+        will return early. This patch fixes this issue by changing the early return condition.
+
+        In addition, `-[WKFullScreenWindowController exitFullScreenImmediately]`
+        does not really exit fullscreen immediately. Instead, it calls
+        `-[WKFullScreenWindowController finishedExitFullScreenAnimation:]`,
+        which always calls `WebPageProxy::forceRepaint()` and expects a response
+        from the WebContet process. This patch fixes this issue as well.
+
+        Tested manually.
+
+        * UIProcess/mac/WKFullScreenWindowController.mm:
+        (-[WKFullScreenWindowController exitFullScreenImmediately]):
+        Update the early return condition to make sure the UI process will proceed
+        to exit fullscreen unless the current state is `NotInFullScreen`.
+        (-[WKFullScreenWindowController beganExitFullScreenWithInitialFrame:finalFrame:]):
+        (-[WKFullScreenWindowController finishedExitFullScreenAnimation:]):
+        The `completed` parameter was not used. This patch renames it to `immediately` to
+        indicate whether `WKFullScreenWindowController` wants to exit fullscreen immediately.
+        (-[WKFullScreenWindowController close]):
+        Clean up this function after revising `-[WKFullScreenWindowController exitFullScreenImmediately]`.
+        (-[WKFullScreenWindowController windowDidFailToExitFullScreen:]):
+        Update the argument according to the change of `-[WKFullScreenWindowController finishedExitFullScreenAnimation:]`.
+        (-[WKFullScreenWindowController windowDidExitFullScreen:]): Ditto.
+
 2021-09-08  Sihui Liu  <sihui_...@apple.com>
 
         Remove responsiveness timer in NetworkProcessProxy::getNetworkProcessConnection

Modified: trunk/Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm (282174 => 282175)


--- trunk/Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm	2021-09-08 21:56:20 UTC (rev 282174)
+++ trunk/Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm	2021-09-08 22:03:32 UTC (rev 282175)
@@ -440,7 +440,7 @@
 
 - (void)exitFullScreenImmediately
 {
-    if (![self isFullScreen])
+    if (_fullScreenState == NotInFullScreen)
         return;
 
     [self _manager]->requestExitFullScreen();
@@ -447,7 +447,7 @@
     [_webViewPlaceholder setExitWarningVisible:NO];
     [self _manager]->willExitFullScreen();
     _fullScreenState = ExitingFullScreen;
-    [self finishedExitFullScreenAnimation:YES];
+    [self finishedExitFullScreenAnimationAndExitImmediately:YES];
 }
 
 - (void)requestExitFullScreen
@@ -463,8 +463,8 @@
 
     if (![[self window] isOnActiveSpace]) {
         // If the full screen window is not in the active space, the NSWindow full screen animation delegate methods
-        // will never be called. So call finishedExitFullScreenAnimation explicitly.
-        [self finishedExitFullScreenAnimation:YES];
+        // will never be called. So call finishedExitFullScreenAnimationAndExitImmediately explicitly.
+        [self finishedExitFullScreenAnimationAndExitImmediately:NO];
 
         // Because we are breaking the normal animation pattern, re-enable screen updates
         // as exitFullScreen has disabled them, but _startExitFullScreenAnimationWithDuration:
@@ -497,7 +497,7 @@
     return adoptCF(CGWindowListCreateImage(CGRectNull, kCGWindowListOptionIncludingWindow, windowID, imageOptions));
 }
 
-- (void)finishedExitFullScreenAnimation:(bool)completed
+- (void)finishedExitFullScreenAnimationAndExitImmediately:(bool)immediately
 {
     if (_fullScreenState == InFullScreen) {
         // If we are currently in the InFullScreen state, this notification is unexpected, meaning
@@ -560,15 +560,20 @@
     [self _manager]->restoreScrollPosition();
     _page->setTopContentInset(_savedTopContentInset);
 
+    [CATransaction commit];
+    [CATransaction flush];
+
+    if (immediately) {
+        [self completeFinishExitFullScreenAnimation];
+        return;
+    }
+
     _page->forceRepaint([weakSelf = WeakObjCPtr<WKFullScreenWindowController>(self)] {
-        [weakSelf completeFinishExitFullScreenAnimationAfterRepaint];
+        [weakSelf completeFinishExitFullScreenAnimation];
     });
-
-    [CATransaction commit];
-    [CATransaction flush];
 }
 
-- (void)completeFinishExitFullScreenAnimationAfterRepaint
+- (void)completeFinishExitFullScreenAnimation
 {
     [CATransaction begin];
     [CATransaction setDisableActions:YES];
@@ -603,11 +608,7 @@
     // has closed or the web process has crashed.  Just walk through our
     // normal exit full screen sequence, but don't wait to be called back
     // in response.
-    if ([self isFullScreen])
-        [self exitFullScreenImmediately];
-    
-    if (_fullScreenState == ExitingFullScreen)
-        [self finishedExitFullScreenAnimation:YES];
+    [self exitFullScreenImmediately];
 
     [super close];
 
@@ -692,7 +693,7 @@
 - (void)windowDidFailToExitFullScreen:(NSWindow *)window
 {
     RetainPtr<WKFullScreenWindowController> retain = self;
-    [self finishedExitFullScreenAnimation:NO];
+    [self finishedExitFullScreenAnimationAndExitImmediately:YES];
     [self clearVideoFullscreenManagerObserver];
 }
 
@@ -699,7 +700,7 @@
 - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
     RetainPtr<WKFullScreenWindowController> retain = self;
-    [self finishedExitFullScreenAnimation:YES];
+    [self finishedExitFullScreenAnimationAndExitImmediately:NO];
     [self clearVideoFullscreenManagerObserver];
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to