doAfterNextPresentationUpdate should not be called while content is hidden due to...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2018 21:29:25 +0000 (21:29 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2018 21:29:25 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187500
<rdar://problem/41294139>

Reviewed by Simon Fraser.

Source/WebKit:

Clients generally expect that after doAfterNextPresentationUpdate, there's
something vaguely sensible on the screen. They use this to remove snapshots,
unhide web views, etc.

During some kinds of resize/rotation, we will hide the WKContentView,
and asynchronously hide it when the resize/rotation is complete. This
can cause clients to prematurely expose a blank WKWebView.

To fix this, avoid calling doAfterNextPresentationUpdate until the
animated resize completes. Add a variant that does not wait for this
(to be used for testing purposes).

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _didCompleteAnimatedResize]):
(-[WKWebView _snapshotRect:intoImageOfWidth:completionHandler:]):
Rename the vector of blocks that we call after animated resize completes
to be generic rather than just about snapshots.

(-[WKWebView _internalDoAfterNextPresentationUpdate:withoutWaitingForPainting:withoutWaitingForAnimatedResize:]):
Add this _internal variant of _doAfterNextPresentationUpdate that takes bits determining
which waits to perform or avoid, to reduce duplication.

(-[WKWebView _doAfterNextPresentationUpdate:]):
(-[WKWebView _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:]):
(-[WKWebView _doAfterNextPresentationUpdateWithoutWaitingForPainting:]):
Call _internalDoAfterNextPresentationUpdate with the appropriate bits set for each situation.

* UIProcess/API/Cocoa/WKWebViewPrivate.h:
Add _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting to WKWebView(WKTesting).

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm:
(-[AnimatedResizeWebView _endAnimatedResize]):
Set didEndAnimatedResize before calling super, because it makes the
new test easier and doesn't affect any of the existing ones.

(AnimatedResizeBlocksViewportFitChanges):
This doAfterNextPresentationUpdate is always called during animated resize,
and then synchronously waited for, so it /must/ use
_doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeWithTesting.

(AnimatedResizeBlocksDoAfterNextPresentationUpdate):
Add a test ensuring that doAfterNextPresentationUpdate is deferred
until endAnimatedResize is called.

* TestWebKitAPI/cocoa/TestNavigationDelegate.mm:
(-[WKWebView _test_waitForDidFinishNavigation]):
_test_waitForDidFinishNavigation is sometimes called during animated resize,
and synchronously waits for a doAfterNextPresentationUpdate, so
it cannot wait for the animated resize to complete.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233698 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm
Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.mm

index 3c19868..8103a83 100644 (file)
@@ -1,5 +1,43 @@
 2018-07-10  Tim Horton  <timothy_horton@apple.com>
 
+        doAfterNextPresentationUpdate should not be called while content is hidden due to animated resize
+        https://bugs.webkit.org/show_bug.cgi?id=187500
+        <rdar://problem/41294139>
+
+        Reviewed by Simon Fraser.
+
+        Clients generally expect that after doAfterNextPresentationUpdate, there's
+        something vaguely sensible on the screen. They use this to remove snapshots,
+        unhide web views, etc.
+
+        During some kinds of resize/rotation, we will hide the WKContentView,
+        and asynchronously hide it when the resize/rotation is complete. This
+        can cause clients to prematurely expose a blank WKWebView.
+
+        To fix this, avoid calling doAfterNextPresentationUpdate until the
+        animated resize completes. Add a variant that does not wait for this
+        (to be used for testing purposes).
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _didCompleteAnimatedResize]):
+        (-[WKWebView _snapshotRect:intoImageOfWidth:completionHandler:]):
+        Rename the vector of blocks that we call after animated resize completes
+        to be generic rather than just about snapshots.
+
+        (-[WKWebView _internalDoAfterNextPresentationUpdate:withoutWaitingForPainting:withoutWaitingForAnimatedResize:]):
+        Add this _internal variant of _doAfterNextPresentationUpdate that takes bits determining
+        which waits to perform or avoid, to reduce duplication.
+
+        (-[WKWebView _doAfterNextPresentationUpdate:]):
+        (-[WKWebView _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:]):
+        (-[WKWebView _doAfterNextPresentationUpdateWithoutWaitingForPainting:]):
+        Call _internalDoAfterNextPresentationUpdate with the appropriate bits set for each situation.
+
+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+        Add _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting to WKWebView(WKTesting).
+
+2018-07-10  Tim Horton  <timothy_horton@apple.com>
+
         REGRESSION (r231510): Dismissing PDFPlugin context menu automatically clicks the first item
         https://bugs.webkit.org/show_bug.cgi?id=187507
         <rdar://problem/42007155>
index 024d34c..cb92948 100644 (file)
@@ -347,7 +347,7 @@ static std::optional<WebCore::ScrollbarOverlayStyle> toCoreScrollbarStyle(_WKOve
     BOOL _waitingForEndAnimatedResize;
     BOOL _waitingForCommitAfterAnimatedResize;
 
-    Vector<WTF::Function<void ()>> _snapshotsDeferredDuringResize;
+    Vector<WTF::Function<void ()>> _callbacksDeferredDuringResize;
     RetainPtr<NSMutableArray> _stableStatePresentationUpdateCallbacks;
 
     RetainPtr<WKPasswordView> _passwordView;
@@ -2953,8 +2953,8 @@ static int32_t activeOrientation(WKWebView *webView)
     if (!_lastSentDeviceOrientation || newOrientation != _lastSentDeviceOrientation.value())
         [self _dispatchSetDeviceOrientation:newOrientation];
 
-    while (!_snapshotsDeferredDuringResize.isEmpty())
-        _snapshotsDeferredDuringResize.takeLast()();
+    while (!_callbacksDeferredDuringResize.isEmpty())
+        _callbacksDeferredDuringResize.takeLast()();
 }
 
 - (void)_didFinishLoadForMainFrame
@@ -5422,7 +5422,7 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
         // Defer snapshotting until after the current resize completes.
         void (^copiedCompletionHandler)(CGImageRef) = [completionHandler copy];
         RetainPtr<WKWebView> retainedSelf = self;
-        _snapshotsDeferredDuringResize.append([retainedSelf, rectInViewCoordinates, imageWidth, copiedCompletionHandler] {
+        _callbacksDeferredDuringResize.append([retainedSelf, rectInViewCoordinates, imageWidth, copiedCompletionHandler] {
             [retainedSelf _snapshotRect:rectInViewCoordinates intoImageOfWidth:imageWidth completionHandler:copiedCompletionHandler];
             [copiedCompletionHandler release];
         });
@@ -6348,8 +6348,7 @@ static WebCore::UserInterfaceLayoutDirection toUserInterfaceLayoutDirection(UISe
     return _page->pageScaleFactor();
 }
 
-// Execute the supplied block after the next transaction from the WebProcess.
-- (void)_doAfterNextPresentationUpdate:(void (^)(void))updateBlock
+- (void)_internalDoAfterNextPresentationUpdate:(void (^)(void))updateBlock withoutWaitingForPainting:(BOOL)withoutWaitingForPainting withoutWaitingForAnimatedResize:(BOOL)withoutWaitingForAnimatedResize
 {
 #if PLATFORM(IOS)
     if (![self usesStandardContentView]) {
@@ -6358,25 +6357,44 @@ static WebCore::UserInterfaceLayoutDirection toUserInterfaceLayoutDirection(UISe
     }
 #endif
 
+    if (withoutWaitingForPainting)
+        _page->setShouldSkipWaitingForPaintAfterNextViewDidMoveToWindow(true);
+
     auto updateBlockCopy = makeBlockPtr(updateBlock);
 
-    _page->callAfterNextPresentationUpdate([updateBlockCopy](WebKit::CallbackBase::Error error) {
-        if (updateBlockCopy)
-            updateBlockCopy();
+    RetainPtr<WKWebView> strongSelf = self;
+    _page->callAfterNextPresentationUpdate([updateBlockCopy, withoutWaitingForAnimatedResize, strongSelf](WebKit::CallbackBase::Error error) {
+        if (!updateBlockCopy)
+            return;
+
+#if PLATFORM(IOS)
+        if (!withoutWaitingForAnimatedResize && strongSelf->_dynamicViewportUpdateMode != WebKit::DynamicViewportUpdateMode::NotResizing) {
+            strongSelf->_callbacksDeferredDuringResize.append([updateBlockCopy] {
+                updateBlockCopy();
+            });
+            
+            return;
+        }
+#endif
+
+        updateBlockCopy();
     });
 }
 
-- (void)_doAfterNextPresentationUpdateWithoutWaitingForPainting:(void (^)(void))updateBlock
+// Execute the supplied block after the next transaction from the WebProcess.
+- (void)_doAfterNextPresentationUpdate:(void (^)(void))updateBlock
 {
-#if PLATFORM(IOS)
-    if (![self usesStandardContentView]) {
-        dispatch_async(dispatch_get_main_queue(), updateBlock);
-        return;
-    }
-#endif
+    [self _internalDoAfterNextPresentationUpdate:updateBlock withoutWaitingForPainting:NO withoutWaitingForAnimatedResize:NO];
+}
+
+- (void)_doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:(void (^)(void))updateBlock
+{
+    [self _internalDoAfterNextPresentationUpdate:updateBlock withoutWaitingForPainting:NO withoutWaitingForAnimatedResize:YES];
+}
 
-    _page->setShouldSkipWaitingForPaintAfterNextViewDidMoveToWindow(true);
-    [self _doAfterNextPresentationUpdate:updateBlock];
+- (void)_doAfterNextPresentationUpdateWithoutWaitingForPainting:(void (^)(void))updateBlock
+{
+    [self _internalDoAfterNextPresentationUpdate:updateBlock withoutWaitingForPainting:YES withoutWaitingForAnimatedResize:NO];
 }
 
 - (void)_doAfterNextVisibleContentRectUpdate:(void (^)(void))updateBlock
index e95a673..652812b 100644 (file)
@@ -453,6 +453,7 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) {
 - (CGFloat)_pageScale WK_API_AVAILABLE(ios(10.3));
 
 - (void)_doAfterNextPresentationUpdate:(void (^)(void))updateBlock WK_API_AVAILABLE(macosx(10.12), ios(10.0));
+- (void)_doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:(void (^)(void))updateBlock WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 - (void)_doAfterNextPresentationUpdateWithoutWaitingForPainting:(void (^)(void))updateBlock WK_API_AVAILABLE(macosx(10.12.3), ios(10.3));
 - (void)_doAfterNextVisibleContentRectUpdate:(void (^)(void))updateBlock WK_API_AVAILABLE(macosx(10.13), ios(11.0));
 
index 6627894..2060f6f 100644 (file)
@@ -1,3 +1,31 @@
+2018-07-10  Tim Horton  <timothy_horton@apple.com>
+
+        doAfterNextPresentationUpdate should not be called while content is hidden due to animated resize
+        https://bugs.webkit.org/show_bug.cgi?id=187500
+        <rdar://problem/41294139>
+
+        Reviewed by Simon Fraser.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm:
+        (-[AnimatedResizeWebView _endAnimatedResize]):
+        Set didEndAnimatedResize before calling super, because it makes the
+        new test easier and doesn't affect any of the existing ones.
+
+        (AnimatedResizeBlocksViewportFitChanges):
+        This doAfterNextPresentationUpdate is always called during animated resize,
+        and then synchronously waited for, so it /must/ use
+        _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeWithTesting.
+        
+        (AnimatedResizeBlocksDoAfterNextPresentationUpdate):
+        Add a test ensuring that doAfterNextPresentationUpdate is deferred
+        until endAnimatedResize is called.
+
+        * TestWebKitAPI/cocoa/TestNavigationDelegate.mm:
+        (-[WKWebView _test_waitForDidFinishNavigation]):
+        _test_waitForDidFinishNavigation is sometimes called during animated resize,
+        and synchronously waits for a doAfterNextPresentationUpdate, so
+        it cannot wait for the animated resize to complete.
+
 2018-07-10  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [GTK][WPE] TestWTF and TestJSC fail to compile due to missing forwarding headers
index 2213bea..7ccffb9 100644 (file)
@@ -50,9 +50,9 @@ static bool didChangeSafeAreaShouldAffectObscuredInsets;
 
 - (void)_endAnimatedResize
 {
-    [super _endAnimatedResize];
-
     didEndAnimatedResize = true;
+
+    [super _endAnimatedResize];
 }
 
 - (void)_webView:(WKWebView *)webView didChangeSafeAreaShouldAffectObscuredInsets:(BOOL)safeAreaShouldAffectObscuredInsets
@@ -152,7 +152,7 @@ TEST(WebKit, AnimatedResizeBlocksViewportFitChanges)
     [window addSubview:webView.get()];
     [window setHidden:NO];
 
-    [webView _beginAnimatedResizeWithUpdates:^ {
+    [webView _beginAnimatedResizeWithUpdates:^{
         [webView setFrame:CGRectMake(0, 0, [webView frame].size.width + 100, 400)];
     }];
 
@@ -166,7 +166,7 @@ TEST(WebKit, AnimatedResizeBlocksViewportFitChanges)
     // Wait for a commit to come in /after/ loading the viewport-fit=cover
     // page, and ensure that we didn't call the UIDelegate callback,
     // because we're still in the resize. Then, end the resize.
-    [webView _doAfterNextPresentationUpdate:^ {
+    [webView _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:^{
         EXPECT_FALSE(didChangeSafeAreaShouldAffectObscuredInsets);
         [webView _endAnimatedResize];
     }];
@@ -177,7 +177,7 @@ TEST(WebKit, AnimatedResizeBlocksViewportFitChanges)
     // Wait for one more commit so that we see the viewport-fit state
     // change actually take place (post-resize), and ensure that it does.
     __block bool didGetCommitAfterEndAnimatedResize = false;
-    [webView _doAfterNextPresentationUpdate:^ {
+    [webView _doAfterNextPresentationUpdate:^{
         didGetCommitAfterEndAnimatedResize = true;
     }];
     TestWebKitAPI::Util::run(&didGetCommitAfterEndAnimatedResize);
@@ -201,7 +201,7 @@ TEST(WebKit, OverrideLayoutSizeChangesDuringAnimatedResizeSucceed)
     [window addSubview:webView.get()];
     [window setHidden:NO];
 
-    [webView _beginAnimatedResizeWithUpdates:^ {
+    [webView _beginAnimatedResizeWithUpdates:^{
         [webView setFrame:CGRectMake(0, 0, [webView frame].size.width + 100, 400)];
     }];
 
@@ -347,4 +347,41 @@ TEST(WebKit, ResizeWithContentHiddenCompletes)
     EXPECT_FALSE(contentView.hidden);
 }
 
+
+TEST(WebKit, AnimatedResizeBlocksDoAfterNextPresentationUpdate)
+{
+    auto webView = createAnimatedResizeWebView();
+    [webView setUIDelegate:webView.get()];
+
+    // We need to have something loaded before beginning the animated
+    // resize, or it will bail.
+    [webView loadHTMLString:@"<head></head>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    auto window = adoptNS([[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 800, 600)]);
+    [window addSubview:webView.get()];
+    [window setHidden:NO];
+
+    [webView _beginAnimatedResizeWithUpdates:^{
+        [webView setFrame:CGRectMake(0, 0, [webView frame].size.width + 100, 400)];
+    }];
+
+    __block bool didGetCommitAfterEndAnimatedResize = false;
+
+    // Despite being invoked first, this doAfterNextPresentationUpdate
+    // should be deferred until after we call endAnimatedResize, inside
+    // the below _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResize.
+    [webView _doAfterNextPresentationUpdate:^{
+        EXPECT_TRUE(didEndAnimatedResize);
+        didGetCommitAfterEndAnimatedResize = true;
+    }];
+
+    [webView _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:^{
+        [webView _endAnimatedResize];
+    }];
+
+    TestWebKitAPI::Util::run(&didEndAnimatedResize);
+    TestWebKitAPI::Util::run(&didGetCommitAfterEndAnimatedResize);
+}
+
 #endif
index 4d5bc8b..0ebdb6c 100644 (file)
 
 #if PLATFORM(IOS)
     __block bool presentationUpdateHappened = false;
-    [self _doAfterNextPresentationUpdate:^{
+    [self _doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:^{
         presentationUpdateHappened = true;
     }];
     TestWebKitAPI::Util::run(&presentationUpdateHappened);