From b42eaa63ce43d533c18d3ffc18ef29ef6b8f3ee1 Mon Sep 17 00:00:00 2001 From: Aditya Keerthi Date: Tue, 18 Apr 2023 10:52:57 -0700 Subject: [PATCH] Fullscreen video size/position is incorrect on YouTube when minimumEffectiveDeviceWidth is set https://bugs.webkit.org/show_bug.cgi?id=255556 rdar://101557743 Reviewed by Tim Horton. Entering fullscreen on youtube.com for a `WKWebView` with a non-zero `minimumEffectiveDeviceWidth` currently results in portions of the video becoming clipped, the top of the video being incorrectly offset from the top of the window. In fullscreen, YouTube explicitly sets the width/height of the video element to the `clientWidth`/`clientHeight` of the fullscreen element, inside the `fullscreenchange` event. The fullscreen element is a wrapper `
` containing the video element, with width/height: 100%. Currently, the `WKWebView`'s frame is adjusted prior to dispatching the `fullscreenchange` event, but the `minimumEffectiveDeviceWidth` is not. This results in the viewport state continuing to incorporate the `minimumEffectiveDeviceWidth`, which may be larger than the width of the fullscreen window. The larger size also results in an offset from the top of the window, as the video element has `object-fit: contain`. The video is smaller than its element's size, and is vertically centered relative to the larger size. To fix, ensure `minimumEffectiveDeviceWidth` is unset prior to dispatching the `fullscreenchange` event. However, simply updating the value in the UI process prior to dispatching `willEnterFullscreen` is not enough, as the visible content rect update is asynchronous in both the UI and web processes. This means that there is no guarantee that the viewport state in the web process will be correct at the time of `fullscreenchange` event dispatch. To fix the secondary issue, introduce `-[WKWebView _doAfterNextVisibleContentRectAndPresentationUpdate]` to ensure the correct viewport state is reflected in the web process prior to dispatching the `fullscreenchange` event. * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _doAfterNextVisibleContentRectAndPresentationUpdate:]): * Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h: Introduce `_doAfterNextVisibleContentRectAndPresentationUpdate`, used to ensure ensure that the next visible content rect update is accurately reflected in the web process, prior to performing the update block. * Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm: (-[WKFullScreenWindowController enterFullScreen:]): Unset `minimumEffectiveDeviceWidth` prior to dispatching the `fullscreenchange` event, so that the viewport state accurately reflects the size of the fullscreen window. The original value is already stored `WKWebViewState`, and is restored on fullscreen exit. Canonical link: https://commits.webkit.org/263082@main --- Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm | 7 +++++++ .../UIProcess/API/Cocoa/WKWebViewInternal.h | 2 ++ .../WKFullScreenWindowControllerIOS.mm | 17 +++++++++++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm index ff2a411c7951..826127706cc0 100644 --- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm +++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm @@ -1639,6 +1639,13 @@ - (void)_internalDoAfterNextPresentationUpdate:(void (^)(void))updateBlock witho }); } +- (void)_doAfterNextVisibleContentRectAndPresentationUpdate:(void (^)(void))updateBlock +{ + [self _doAfterNextVisibleContentRectUpdate:makeBlockPtr([strongSelf = retainPtr(self), updateBlock = makeBlockPtr(updateBlock)] { + [strongSelf _doAfterNextPresentationUpdate:updateBlock.get()]; + }).get()]; +} + - (void)_recalculateViewportSizesWithMinimumViewportInset:(CocoaEdgeInsets)minimumViewportInset maximumViewportInset:(CocoaEdgeInsets)maximumViewportInset throwOnInvalidInput:(BOOL)throwOnInvalidInput { auto frame = WebCore::FloatSize(self.frame.size); diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h index 7c0c8722e774..03f82838f84a 100644 --- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h +++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h @@ -336,6 +336,8 @@ struct PerWebProcessState { - (void)_internalDoAfterNextPresentationUpdate:(void (^)(void))updateBlock withoutWaitingForPainting:(BOOL)withoutWaitingForPainting withoutWaitingForAnimatedResize:(BOOL)withoutWaitingForAnimatedResize; +- (void)_doAfterNextVisibleContentRectAndPresentationUpdate:(void (^)(void))updateBlock; + - (void)_recalculateViewportSizesWithMinimumViewportInset:(CocoaEdgeInsets)minimumViewportInset maximumViewportInset:(CocoaEdgeInsets)maximumViewportInset throwOnInvalidInput:(BOOL)throwOnInvalidInput; - (void)_showSafeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler&&)>&&)completionHandler; diff --git a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm index 7a5cc0ed4546..f97a2e534cb7 100644 --- a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm +++ b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm @@ -649,6 +649,7 @@ - (void)enterFullScreen:(CGSize)videoDimensions [webView setAutoresizingMask:(UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight)]; [webView setFrame:[_window bounds]]; + [webView _setMinimumEffectiveDeviceWidth:0]; [webView _overrideLayoutParametersWithMinimumLayoutSize:[_window bounds].size maximumUnobscuredSizeOverride:[_window bounds].size]; [_window insertSubview:webView.get() atIndex:0]; [webView setNeedsLayout]; @@ -671,13 +672,21 @@ - (void)enterFullScreen:(CGSize)videoDimensions return; } - if (auto* manager = [protectedSelf _manager]) { - manager->willEnterFullScreen(); + if (![protectedSelf _manager]) { + ASSERT_NOT_REACHED(); + [self _exitFullscreenImmediately]; return; } - ASSERT_NOT_REACHED(); - [self _exitFullscreenImmediately]; + [self._webView _doAfterNextVisibleContentRectAndPresentationUpdate:makeBlockPtr([protectedSelf] { + if (auto* manager = [protectedSelf _manager]) { + manager->willEnterFullScreen(); + return; + } + + ASSERT_NOT_REACHED(); + [protectedSelf _exitFullscreenImmediately]; + }).get()]; }); [CATransaction commit];