Changeset 233927 in webkit

Timestamp:

Jul 18, 2018 12:51:54 PM (6 years ago)

Author:

jer.noble@apple.com

Message:

-_beginAnimatedResizeWithUpdates: can leave view in bad state if called during an existing animation
​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):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (modified) (6 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -1660 +1660 @@
     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]];
@@ -2891 +2883 @@
 }
 
+- (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
 {
@@ -2901 +2916 @@
         // 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;
     }
@@ -5299 +5314 @@
     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)
@@ -5308 +5323 @@
             [self _dispatchSetDeviceOrientation:newOrientation];
 
-        [self _scheduleVisibleContentRectUpdate];
         return;
     }
@@ -5317 +5331 @@
         && oldOrientation == newOrientation
         && UIEdgeInsetsEqualToEdgeInsets(oldObscuredInsets, newObscuredInsets)) {
-        _dynamicViewportUpdateMode = WebKit::DynamicViewportUpdateMode::NotResizing;
-        [self _scheduleVisibleContentRectUpdate];
+        [self _cancelAnimatedResize];
         return;
     }

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AnimatedResize.mm

@@ -348 +348 @@
 }
 
+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)