-_beginAnimatedResizeWithUpdates: can leave view in bad state if called during an...
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2018 19:51:54 +0000 (19:51 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2018 19:51:54 +0000 (19:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187739
Source/WebKit:

Reviewed by Tim Horton.

It's not enough to reset _dynamicViewportUpdateMode to NotResizing; other parts of the code
check whether _resizeAnimationView is non-nil, the contentView may be hidden, etc. Add a new
internal method _cancelAnimatedResize that cleans up state when a call to
_beginAnimatedResizeWithUpdates: fails.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _processDidExit]):
(-[WKWebView _cancelAnimatedResize]):
(-[WKWebView _didCompleteAnimatedResize]):
(-[WKWebView _beginAnimatedResizeWithUpdates:]):

Tools:

<rdar://problem/42304518>

Reviewed by Tim Horton.

* TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm:
(TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm

index 1cce107..4c498f7 100644 (file)
@@ -1,5 +1,23 @@
 2018-07-18  Jer Noble  <jer.noble@apple.com>
 
+        -_beginAnimatedResizeWithUpdates: can leave view in bad state if called during an existing animation
+        https://bugs.webkit.org/show_bug.cgi?id=187739
+
+        Reviewed by Tim Horton.
+
+        It's not enough to reset _dynamicViewportUpdateMode to NotResizing; other parts of the code
+        check whether _resizeAnimationView is non-nil, the contentView may be hidden, etc. Add a new
+        internal method _cancelAnimatedResize that cleans up state when a call to
+        _beginAnimatedResizeWithUpdates: fails.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _processDidExit]):
+        (-[WKWebView _cancelAnimatedResize]):
+        (-[WKWebView _didCompleteAnimatedResize]):
+        (-[WKWebView _beginAnimatedResizeWithUpdates:]):
+
+2018-07-18  Jer Noble  <jer.noble@apple.com>
+
         PiP from Element Fullscreen should match AVKit's behavior
         https://bugs.webkit.org/show_bug.cgi?id=187623
 
index 9258b07..c3e4be9 100644 (file)
@@ -1659,15 +1659,7 @@ static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
 {
     RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _processDidExit]", self);
     [self _hidePasswordView];
-    if (!_customContentView && _dynamicViewportUpdateMode != WebKit::DynamicViewportUpdateMode::NotResizing) {
-        NSUInteger indexOfResizeAnimationView = [[_scrollView subviews] indexOfObject:_resizeAnimationView.get()];
-        [_scrollView insertSubview:_contentView.get() atIndex:indexOfResizeAnimationView];
-        [_scrollView insertSubview:[_contentView unscaledView] atIndex:indexOfResizeAnimationView + 1];
-        [_resizeAnimationView removeFromSuperview];
-        _resizeAnimationView = nil;
-
-        _resizeAnimationTransformAdjustments = CATransform3DIdentity;
-    }
+    [self _cancelAnimatedResize];
     [_contentView setFrame:self.bounds];
     [_scrollView setBackgroundColor:[UIColor whiteColor]];
     [_scrollView setContentOffset:[self _initialContentOffsetForScrollView]];
@@ -2890,6 +2882,29 @@ static int32_t activeOrientation(WKWebView *webView)
     return webView->_overridesInterfaceOrientation ? deviceOrientationForUIInterfaceOrientation(webView->_interfaceOrientationOverride) : webView->_page->deviceOrientation();
 }
 
+- (void)_cancelAnimatedResize
+{
+    LOG_WITH_STREAM(VisibleRects, stream << "-[WKWebView " << _page->pageID() << " _cancelAnimatedResize:] " << " _dynamicViewportUpdateMode " << (int)_dynamicViewportUpdateMode);
+    if (_dynamicViewportUpdateMode == WebKit::DynamicViewportUpdateMode::NotResizing)
+        return;
+
+    if (!_customContentView) {
+        if (_resizeAnimationView) {
+            NSUInteger indexOfResizeAnimationView = [[_scrollView subviews] indexOfObject:_resizeAnimationView.get()];
+            [_scrollView insertSubview:_contentView.get() atIndex:indexOfResizeAnimationView];
+            [_scrollView insertSubview:[_contentView unscaledView] atIndex:indexOfResizeAnimationView + 1];
+            [_resizeAnimationView removeFromSuperview];
+            _resizeAnimationView = nil;
+        }
+
+        [_contentView setHidden:NO];
+        _resizeAnimationTransformAdjustments = CATransform3DIdentity;
+    }
+
+    _dynamicViewportUpdateMode = WebKit::DynamicViewportUpdateMode::NotResizing;
+    [self _scheduleVisibleContentRectUpdate];
+}
+
 - (void)_didCompleteAnimatedResize
 {
     if (_dynamicViewportUpdateMode == WebKit::DynamicViewportUpdateMode::NotResizing)
@@ -2900,7 +2915,7 @@ static int32_t activeOrientation(WKWebView *webView)
     if (!_resizeAnimationView) {
         // Paranoia. If _resizeAnimationView is null we'll end up setting a zero scale on the content view.
         RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _didCompleteAnimatedResize:] - _resizeAnimationView is nil", self);
-        _dynamicViewportUpdateMode = WebKit::DynamicViewportUpdateMode::NotResizing;
+        [self _cancelAnimatedResize];
         return;
     }
 
@@ -5298,7 +5313,7 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
 
     ASSERT_WITH_MESSAGE(!(_overridesViewLayoutSize && newViewLayoutSize.isEmpty()), "Clients controlling the layout size should maintain a valid layout size to minimize layouts.");
     if (CGRectIsEmpty(newBounds) || newViewLayoutSize.isEmpty() || CGRectIsEmpty(futureUnobscuredRectInSelfCoordinates) || CGRectIsEmpty(contentViewBounds)) {
-        _dynamicViewportUpdateMode = WebKit::DynamicViewportUpdateMode::NotResizing;
+        [self _cancelAnimatedResize];
         [self _frameOrBoundsChanged];
         if (_overridesViewLayoutSize)
             [self _dispatchSetViewLayoutSize:newViewLayoutSize];
@@ -5307,7 +5322,6 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
         if (_overridesInterfaceOrientation)
             [self _dispatchSetDeviceOrientation:newOrientation];
 
-        [self _scheduleVisibleContentRectUpdate];
         return;
     }
 
@@ -5316,8 +5330,7 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
         && oldMaximumUnobscuredSize == newMaximumUnobscuredSize
         && oldOrientation == newOrientation
         && UIEdgeInsetsEqualToEdgeInsets(oldObscuredInsets, newObscuredInsets)) {
-        _dynamicViewportUpdateMode = WebKit::DynamicViewportUpdateMode::NotResizing;
-        [self _scheduleVisibleContentRectUpdate];
+        [self _cancelAnimatedResize];
         return;
     }
 
index db9fa0a..ea05e7e 100644 (file)
@@ -1,5 +1,16 @@
 2018-07-18  Jer Noble  <jer.noble@apple.com>
 
+        -_beginAnimatedResizeWithUpdates: can leave view in bad state if called during an existing animation
+        https://bugs.webkit.org/show_bug.cgi?id=187739
+        <rdar://problem/42304518>
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm:
+        (TEST):
+
+2018-07-18  Jer Noble  <jer.noble@apple.com>
+
         PiP from Element Fullscreen should match AVKit's behavior
         https://bugs.webkit.org/show_bug.cgi?id=187623
         <rdar://problem/41212379>
index 7ccffb9..45c43cb 100644 (file)
@@ -347,6 +347,50 @@ TEST(WebKit, ResizeWithContentHiddenCompletes)
     EXPECT_FALSE(contentView.hidden);
 }
 
+TEST(WebKit, ResizeWithContentHiddenWithSubsequentNoOpResizeCompletes)
+{
+    auto webView = createAnimatedResizeWebView();
+    [webView setUIDelegate:webView.get()];
+
+    [webView loadHTMLString:@"<head><meta name='viewport' content='initial-scale=1'></head>" baseURL:nil];
+    auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
+    webView.get().navigationDelegate = navigationDelegate.get();
+    [navigationDelegate waitForDidFinishNavigation];
+
+    auto window = adoptNS([[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 800, 600)]);
+    [window addSubview:webView.get()];
+    [window setHidden:NO];
+
+    [webView _resizeWhileHidingContentWithUpdates:^{
+        [webView setFrame:CGRectMake(0, 0, 100, 200)];
+    }];
+
+    [webView _beginAnimatedResizeWithUpdates:^{
+        [webView setFrame:CGRectMake(0, 0, 100, 200)];
+    }];
+
+    __block bool didReadLayoutSize = false;
+    [webView _doAfterNextPresentationUpdate:^{
+        [webView evaluateJavaScript:@"[window.innerWidth, window.innerHeight]" completionHandler:^(id value, NSError *error) {
+            CGFloat innerWidth = [[value objectAtIndex:0] floatValue];
+            CGFloat innerHeight = [[value objectAtIndex:1] floatValue];
+
+            EXPECT_EQ(innerWidth, 100);
+            EXPECT_EQ(innerHeight, 200);
+
+            didReadLayoutSize = true;
+        }];
+    }];
+    TestWebKitAPI::Util::run(&didReadLayoutSize);
+
+    UIView *scrollView = immediateSubviewOfClass(webView.get(), NSClassFromString(@"WKScrollView"));
+    UIView *contentView = immediateSubviewOfClass(scrollView, NSClassFromString(@"WKContentView"));
+
+    // Make sure that we've put the view hierarchy back together after the resize completed.
+    EXPECT_NOT_NULL(scrollView);
+    EXPECT_NOT_NULL(contentView);
+    EXPECT_FALSE(contentView.hidden);
+}
 
 TEST(WebKit, AnimatedResizeBlocksDoAfterNextPresentationUpdate)
 {