Skip to content

Commit

Permalink
Cherry-pick 3d721c0. rdar://problem/111534888
Browse files Browse the repository at this point in the history
    REGRESSION (UI-side compositing): Jumpy full screen transition animation
    https://bugs.webkit.org/show_bug.cgi?id=259375
    rdar://111534888

    Reviewed by Wenson Hsieh.

    With UI-side compositing, when entering or exiting window fullscreen mode, the transition is
    sometimes jumpy. This happens in cases where the UI process resizes before the web content has a
    chance to resize. The web content and the UI Process should always resize in the same CA commit.

    Fix by adopting an AppKit SPI to defer entering or exiting window fullscreen mode until the web
    content has actually been resized to the new size. This is done by using a "window snapshot readiness handler".

    * Source/WTF/wtf/PlatformHave.h:
    Add a new `HAVE` definition to only enable this on macOS, and only on versions where the SPI exists.

    * Source/WebKit/Platform/spi/mac/AppKitSPI.h:
    * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
    (-[WKWebView dealloc]):
    (-[WKWebView _invalidateWindowSnapshotReadinessHandler]):
    Invokes the readiness handler and then resets it, indicating to AppKit that the full screen should
    be allowed to resume.

    * Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h:
    * Source/WebKit/UIProcess/API/mac/WKView.mm:
    (-[WKView _web_windowWillEnterFullScreen]):
    (-[WKView _web_windowWillExitFullScreen]):
    Stub methods required to compile legacy WebKit.

    * Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:
    (-[WKWebView _doWindowSnapshotReadinessUpdate]):
    (-[WKWebView setFrameSize:]):
    When `setFrameSize` gets called with the new frame size, the UI process waits for the next presentation
    update of the web prcoess. Once in the completion handler, we know that the web content has properly
    resized, and so we should now invalidate the readiness handler.

    (-[WKWebView _web_windowWillEnterFullScreen]):
    (-[WKWebView _web_windowWillExitFullScreen]):
    Upon receiving one of these notifications, we take out a window snapshot readiness handler and store
    it. This tells AppKit to defer the final stage of the enter and exit full screen animations until
    the handler is invoked.

    * Source/WebKit/UIProcess/mac/WKViewLayoutStrategy.h:
    * Source/WebKit/UIProcess/mac/WKViewLayoutStrategy.mm:
    (-[WKViewLayoutStrategy disableFrameSizeUpdates]):
    (-[WKViewLayoutStrategy enableFrameSizeUpdates]):
    (-[WKViewLayoutStrategy frameSizeUpdatesDisabled]):
    (-[WKViewLayoutStrategy didChangeFrameSize]):
    In some cases, `setFrameSize` would early return inside of `didChangeFrameSize` due to these set of
    methods. This caused the bug to still reproduce, since the new drawing area size was no longer being set.

    These methods have long been not actually needed, so their implementations are now removed and there is
    no longer a need for an early return.

    * Source/WebKit/UIProcess/mac/WebViewImpl.h:
    * Source/WebKit/UIProcess/mac/WebViewImpl.mm:
    (-[WKWindowVisibilityObserver startObserving:]):
    (-[WKWindowVisibilityObserver stopObserving:]):
    (-[WKWindowVisibilityObserver _windowWillEnterFullScreen:]):
    (-[WKWindowVisibilityObserver _windowWillExitFullScreen:]):
    (WebKit::WebViewImpl::windowWillEnterFullScreen):
    (WebKit::WebViewImpl::windowWillExitFullScreen):
    When entering or exiting full screen, we now listen to their respective notifications.

    Canonical link: https://commits.webkit.org/266273@main
  • Loading branch information
rr-codes authored and MyahCobbs committed Aug 2, 2023
1 parent 90d1e36 commit 3a36904
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 11 deletions.
4 changes: 4 additions & 0 deletions Source/WTF/wtf/PlatformHave.h
Original file line number Diff line number Diff line change
Expand Up @@ -1476,3 +1476,7 @@
|| (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 100000)
#define HAVE_AVURLASSET_ISPLAYABLEEXTENDEDMIMETYPEWITHOPTIONS 1
#endif

#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 140000)
#define HAVE_NSWINDOW_SNAPSHOT_READINESS_HANDLER 1
#endif
8 changes: 8 additions & 0 deletions Source/WebKit/Platform/spi/mac/AppKitSPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,11 @@ typedef NS_OPTIONS(NSUInteger, NSWindowShadowOptions) {
@interface NSCursor ()
+ (void)hideUntilChanged;
@end

#if HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
// FIXME: Move this above once <rdar://problem/112554759> is in an SDK.
@interface NSWindow (Staging_112554759)
typedef void (^NSWindowSnapshotReadinessHandler) (void);
- (NSWindowSnapshotReadinessHandler)_holdResizeSnapshotWithReason:(NSString *)reason;
@end
#endif
12 changes: 12 additions & 0 deletions Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,10 @@ - (void)dealloc
CFNotificationCenterRemoveObserver(CFNotificationCenterGetDarwinNotifyCenter(), (__bridge const void *)(self), (__bridge CFStringRef)notificationName.get(), nullptr);
#endif

#if PLATFORM(MAC) && HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
[self _invalidateWindowSnapshotReadinessHandler];
#endif

#if HAVE(UIKIT_RESIZABLE_WINDOWS)
[self _invalidateResizeAssertions];
#endif
Expand Down Expand Up @@ -1706,6 +1710,14 @@ - (void)_recalculateViewportSizesWithMinimumViewportInset:(CocoaEdgeInsets)minim
_page->setMaximumUnobscuredSize(maximumUnobscuredSize);
}

#if PLATFORM(MAC) && HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
- (void)_invalidateWindowSnapshotReadinessHandler
{
if (auto handler = std::exchange(_windowSnapshotReadinessHandler, nil))
handler();
}
#endif

#if ENABLE(ATTACHMENT_ELEMENT)

- (void)_didInsertAttachment:(API::Attachment&)attachment withSource:(NSString *)source
Expand Down
9 changes: 9 additions & 0 deletions Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#import "_WKAttachmentInternal.h"
#import "_WKWebViewPrintFormatterInternal.h"
#import <variant>
#import <wtf/BlockPtr.h>
#import <wtf/CompletionHandler.h>
#import <wtf/NakedPtr.h>
#import <wtf/RefPtr.h>
Expand Down Expand Up @@ -216,6 +217,10 @@ struct PerWebProcessState {
// Only used with UI-side compositing.
RetainPtr<WKScrollView> _scrollView;
RetainPtr<WKContentView> _contentView;

#if HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
BlockPtr<void()> _windowSnapshotReadinessHandler;
#endif
#endif // PLATFORM(MAC)

#if PLATFORM(IOS_FAMILY)
Expand Down Expand Up @@ -324,6 +329,10 @@ struct PerWebProcessState {
- (BOOL)_isValid;
- (void)_didChangeEditorState;

#if PLATFORM(MAC) && HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
- (void)_invalidateWindowSnapshotReadinessHandler;
#endif

#if ENABLE(ATTACHMENT_ELEMENT)
- (void)_didRemoveAttachment:(API::Attachment&)attachment;
- (void)_didInsertAttachment:(API::Attachment&)attachment withSource:(NSString *)source;
Expand Down
8 changes: 8 additions & 0 deletions Source/WebKit/UIProcess/API/mac/WKView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,14 @@ - (void)_web_gestureEventWasNotHandledByWebCore:(NSEvent *)event
{
}

- (void)_web_windowWillEnterFullScreen
{
}

- (void)_web_windowWillExitFullScreen
{
}

- (void)_didHandleAcceptedCandidate
{
}
Expand Down
38 changes: 38 additions & 0 deletions Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,22 @@ - (void)prepareContentInRect:(NSRect)rect
_impl->prepareContentInRect(NSRectToCGRect(rect));
}

- (void)_doWindowSnapshotReadinessUpdate
{
#if HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
if (!self->_windowSnapshotReadinessHandler)
return;

[self _doAfterNextPresentationUpdate:makeBlockPtr([weakSelf = WeakObjCPtr<WKWebView>(self)] {
auto strongSelf = weakSelf.get();
if (!strongSelf)
return;

[strongSelf _invalidateWindowSnapshotReadinessHandler];
}).get()];
#endif
}

- (void)setFrameSize:(NSSize)size
{
[super setFrameSize:size];
Expand All @@ -142,6 +158,8 @@ - (void)setFrameSize:(NSSize)size
_impl->setFrameSize(NSSizeToCGSize(size));

[self _recalculateViewportSizesWithMinimumViewportInset:_minimumViewportInset maximumViewportInset:_maximumViewportInset throwOnInvalidInput:NO];

[self _doWindowSnapshotReadinessUpdate];
}

- (void)setUserInterfaceLayoutDirection:(NSUserInterfaceLayoutDirection)userInterfaceLayoutDirection
Expand Down Expand Up @@ -1177,6 +1195,26 @@ - (void)_web_didChangeContentSize:(NSSize)newSize
{
}

- (void)_holdWindowResizeSnapshotWithReason:(NSString *)reason
{
#if HAVE(NSWINDOW_SNAPSHOT_READINESS_HANDLER)
if (!self.window || ![self.window respondsToSelector:@selector(_holdResizeSnapshotWithReason:)])
return;

_windowSnapshotReadinessHandler = makeBlockPtr([self.window _holdResizeSnapshotWithReason:reason]);
#endif
}

- (void)_web_windowWillEnterFullScreen
{
[self _holdWindowResizeSnapshotWithReason:@"web view entering full screen"];
}

- (void)_web_windowWillExitFullScreen
{
[self _holdWindowResizeSnapshotWithReason:@"web view exiting full screen"];
}

#if ENABLE(DRAG_SUPPORT)

- (WKDragDestinationAction)_web_dragDestinationActionForDraggingInfo:(id <NSDraggingInfo>)draggingInfo
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/UIProcess/mac/WKViewLayoutStrategy.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class WebViewImpl;
NSView *_view;

WKLayoutMode _layoutMode;
unsigned _frameSizeUpdatesDisabledCount;
}

+ (instancetype)layoutStrategyWithPage:(NakedRef<WebKit::WebPageProxy>)page view:(NSView *)view viewImpl:(NakedRef<WebKit::WebViewImpl>)webViewImpl mode:(WKLayoutMode)mode;
Expand Down
11 changes: 1 addition & 10 deletions Source/WebKit/UIProcess/mac/WKViewLayoutStrategy.mm
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,15 @@ - (void)updateLayout

- (void)disableFrameSizeUpdates
{
_frameSizeUpdatesDisabledCount++;
}

- (void)enableFrameSizeUpdates
{
if (!_frameSizeUpdatesDisabledCount)
return;

if (!(--_frameSizeUpdatesDisabledCount))
[self didChangeFrameSize];
}

- (BOOL)frameSizeUpdatesDisabled
{
return _frameSizeUpdatesDisabledCount > 0;
return NO;
}

- (void)didChangeViewScale
Expand All @@ -136,9 +130,6 @@ - (void)didEndLiveResize

- (void)didChangeFrameSize
{
if ([self frameSizeUpdatesDisabled])
return;

if (_webViewImpl->clipsToVisibleRect())
_webViewImpl->updateViewExposedRect();
_webViewImpl->setDrawingAreaSize(NSSizeToCGSize(_view.frame.size));
Expand Down
6 changes: 6 additions & 0 deletions Source/WebKit/UIProcess/mac/WebViewImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ struct TranslationContextMenuInfo;

- (void)_web_didChangeContentSize:(NSSize)newSize;

- (void)_web_windowWillEnterFullScreen;
- (void)_web_windowWillExitFullScreen;

#if ENABLE(DRAG_SUPPORT)
- (WKDragDestinationAction)_web_dragDestinationActionForDraggingInfo:(id <NSDraggingInfo>)draggingInfo;
- (void)_web_didPerformDragOperation:(BOOL)handled;
Expand Down Expand Up @@ -229,6 +232,9 @@ class WebViewImpl : public CanMakeWeakPtr<WebViewImpl> {
void setFrameAndScrollBy(CGRect, CGSize);
void updateWindowAndViewFrames();

void windowWillEnterFullScreen();
void windowWillExitFullScreen();

void setFixedLayoutSize(CGSize);
CGSize fixedLayoutSize() const;

Expand Down
25 changes: 25 additions & 0 deletions Source/WebKit/UIProcess/mac/WebViewImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@ - (void)startObserving:(NSWindow *)window
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidChangeLayerHosting:) name:_NSWindowDidChangeContentsHostedInLayerSurfaceNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowDidChangeOcclusionState:) name:NSWindowDidChangeOcclusionStateNotification object:window];

[defaultNotificationCenter addObserver:self selector:@selector(_windowWillEnterFullScreen:) name:NSWindowWillEnterFullScreenNotification object:window];
[defaultNotificationCenter addObserver:self selector:@selector(_windowWillExitFullScreen:) name:NSWindowWillExitFullScreenNotification object:window];

[defaultNotificationCenter addObserver:self selector:@selector(_screenDidChangeColorSpace:) name:NSScreenColorSpaceDidChangeNotification object:nil];

if (_shouldObserveFontPanel)
Expand Down Expand Up @@ -356,6 +359,8 @@ - (void)stopObserving:(NSWindow *)window
[defaultNotificationCenter removeObserver:self name:NSWindowDidChangeScreenNotification object:window];
[defaultNotificationCenter removeObserver:self name:_NSWindowDidChangeContentsHostedInLayerSurfaceNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowDidChangeOcclusionStateNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowWillEnterFullScreenNotification object:window];
[defaultNotificationCenter removeObserver:self name:NSWindowWillExitFullScreenNotification object:window];

[defaultNotificationCenter removeObserver:self name:NSScreenColorSpaceDidChangeNotification object:nil];

Expand Down Expand Up @@ -449,6 +454,16 @@ - (void)_windowDidChangeOcclusionState:(NSNotification *)notification
_impl->windowDidChangeOcclusionState();
}

- (void)_windowWillEnterFullScreen:(NSNotification *)notification
{
_impl->windowWillEnterFullScreen();
}

- (void)_windowWillExitFullScreen:(NSNotification *)notification
{
_impl->windowWillExitFullScreen();
}

- (void)_screenDidChangeColorSpace:(NSNotification *)notification
{
_impl->screenDidChangeColorSpace();
Expand Down Expand Up @@ -2060,6 +2075,16 @@ static bool isInRecoveryOS()
m_page->activityStateDidChange(WebCore::ActivityState::IsVisible);
}

void WebViewImpl::windowWillEnterFullScreen()
{
[m_view _web_windowWillEnterFullScreen];
}

void WebViewImpl::windowWillExitFullScreen()
{
[m_view _web_windowWillExitFullScreen];
}

void WebViewImpl::screenDidChangeColorSpace()
{
m_page->process().processPool().screenPropertiesChanged();
Expand Down

0 comments on commit 3a36904

Please sign in to comment.