Changeset 220286 in webkit

Timestamp:

Aug 4, 2017, 12:50:23 PM (9 years ago)

Author:

timothy_horton@apple.com

Message:

viewport-fit changes during animated resize can cause layout size to get stuck
​https://bugs.webkit.org/show_bug.cgi?id=175169
<rdar://problem/33684697>

Reviewed by Simon Fraser.

If we get a commit that changes viewport-fit state while an animated
resize is underway, and that change causes the client to push a new
minimumLayoutSizeOverride, the new size will be lost forever, and we
will get stuck laying out at the wrong size.

This is because we unconditionally apply avoidsUnsafeArea for every commit,
while most other changes that come in from a commit are ignored if we're
inside animated resize. To fix, also ignore avoidsUnsafeArea changes during
animated resize, by moving the code that keeps track of it into WKWebView
like all of the rest, and read it out of the commit in didCommitLayerTree
*after* the animated-resize early-return.

Also, fix the associated layout tests by adding a missing assignment
in _computedContentInset, which was broken in r220138.

• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView _initializeWithConfiguration:]):
(-[WKWebView _setHasCustomContentView:loadedMIMEType:]):
(-[WKWebView _processDidExit]):
(-[WKWebView _didCommitLayerTree:]):
(-[WKWebView _setAvoidsUnsafeArea:]):
(-[WKWebView _safeAreaShouldAffectObscuredInsets]):
(-[WKWebView _didChangeAvoidsUnsafeArea:]): Deleted.

• UIProcess/API/Cocoa/WKWebViewInternal.h:
• UIProcess/PageClient.h:
• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::resetState):
(WebKit::WebPageProxy::setAvoidsUnsafeArea): Deleted.

• UIProcess/WebPageProxy.h:

(WebKit::WebPageProxy::avoidsUnsafeArea const): Deleted.

• UIProcess/ios/PageClientImplIOS.h:
• UIProcess/ios/PageClientImplIOS.mm:

(WebKit::PageClientImpl::didChangeAvoidsUnsafeArea): Deleted.

• UIProcess/ios/WebPageProxyIOS.mm:

(WebKit::WebPageProxy::didCommitLayerTree):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/Cocoa/WKWebView.mm (modified) (9 diffs)
• UIProcess/API/Cocoa/WKWebViewInternal.h (modified) (1 diff)
• UIProcess/API/gtk/PageClientImpl.h (modified) (1 diff)
• UIProcess/API/wpe/PageClientImpl.h (modified) (1 diff)
• UIProcess/PageClient.h (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (2 diffs)
• UIProcess/WebPageProxy.h (modified) (2 diffs)
• UIProcess/ios/PageClientImplIOS.h (modified) (1 diff)
• UIProcess/ios/PageClientImplIOS.mm (modified) (1 diff)
• UIProcess/ios/WebPageProxyIOS.mm (modified) (1 diff)
• UIProcess/mac/PageClientImpl.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-08-04  Tim Horton  <timothy_horton@apple.com>
+
+        viewport-fit changes during animated resize can cause layout size to get stuck
+        https://bugs.webkit.org/show_bug.cgi?id=175169
+        <rdar://problem/33684697>
+
+        Reviewed by Simon Fraser.
+
+        If we get a commit that changes viewport-fit state while an animated
+        resize is underway, and that change causes the client to push a new
+        minimumLayoutSizeOverride, the new size will be lost forever, and we
+        will get stuck laying out at the wrong size.
+
+        This is because we unconditionally apply avoidsUnsafeArea for every commit,
+        while most other changes that come in from a commit are ignored if we're
+        inside animated resize. To fix, also ignore avoidsUnsafeArea changes during
+        animated resize, by moving the code that keeps track of it into WKWebView
+        like all of the rest, and read it out of the commit in didCommitLayerTree
+        *after* the animated-resize early-return.
+
+        Also, fix the associated layout tests by adding a missing assignment
+        in _computedContentInset, which was broken in r220138.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _initializeWithConfiguration:]):
+        (-[WKWebView _setHasCustomContentView:loadedMIMEType:]):
+        (-[WKWebView _processDidExit]):
+        (-[WKWebView _didCommitLayerTree:]):
+        (-[WKWebView _setAvoidsUnsafeArea:]):
+        (-[WKWebView _safeAreaShouldAffectObscuredInsets]):
+        (-[WKWebView _didChangeAvoidsUnsafeArea:]): Deleted.
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::resetState):
+        (WebKit::WebPageProxy::setAvoidsUnsafeArea): Deleted.
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::avoidsUnsafeArea const): Deleted.
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::didChangeAvoidsUnsafeArea): Deleted.
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::didCommitLayerTree):
+
 2017-08-04  Matt Lewis  <jlewis3@apple.com>
 

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

@@ -247 +247 @@
     UIEdgeInsets _unobscuredSafeAreaInsets;
     BOOL _haveSetUnobscuredSafeAreaInsets;
+    BOOL _avoidsUnsafeArea;
     UIRectEdge _obscuredInsetEdgesAffectedBySafeArea;
 
@@ -524 +525 @@
     [_scrollView setBouncesZoom:YES];
 
+    _avoidsUnsafeArea = YES;
     [self _updateScrollViewInsetAdjustmentBehavior];
 
@@ -1269 +1271 @@
         [_scrollView setContentOffset:[self _adjustedContentOffset:CGPointZero]];
 
-        [self _didChangeAvoidsUnsafeArea:NO];
+        [self _setAvoidsUnsafeArea:NO];
     } else if (_customContentView) {
         [_customContentView removeFromSuperview];
@@ -1283 +1285 @@
         [_customContentFixedOverlayView setFrame:self.bounds];
         [self addSubview:_customContentFixedOverlayView.get()];
-
-        [self _didChangeAvoidsUnsafeArea:_page->avoidsUnsafeArea()];
     }
 
@@ -1408 +1408 @@
 #if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
     if (self._safeAreaShouldAffectObscuredInsets)
-        UIEdgeInsetsAdd(insets, [_scrollView _systemContentInset], self._effectiveObscuredInsetEdgesAffectedBySafeArea);
+        insets = UIEdgeInsetsAdd(insets, [_scrollView _systemContentInset], self._effectiveObscuredInsetEdgesAffectedBySafeArea);
 #endif
 
@@ -1462 +1462 @@
     _firstTransactionIDAfterPageRestore = 0;
     _resizeAnimationTransformTransactionID = std::nullopt;
+
+    _avoidsUnsafeArea = YES;
 }
 
@@ -1560 +1562 @@
 
     [self _updateScrollViewBackground];
+    [self _setAvoidsUnsafeArea:layerTreeTransaction.avoidsUnsafeArea()];
 
     if (_gestureController)
@@ -2718 +2721 @@
 }
 
-- (void)_didChangeAvoidsUnsafeArea:(BOOL)avoidsUnsafeArea
-{
+- (void)_setAvoidsUnsafeArea:(BOOL)avoidsUnsafeArea
+{
+    if (_avoidsUnsafeArea == avoidsUnsafeArea)
+        return;
+
+    _avoidsUnsafeArea = avoidsUnsafeArea;
+
     [self _updateScrollViewInsetAdjustmentBehavior];
     [self _scheduleVisibleContentRectUpdate];
@@ -4673 +4681 @@
     if (![self usesStandardContentView])
         return NO;
-    if (!_page)
-        return YES;
-    return _page->avoidsUnsafeArea();
+    return _avoidsUnsafeArea;
 }
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h

@@ -120 +120 @@
 - (void)_hidePasswordView;
 
-- (void)_didChangeAvoidsUnsafeArea:(BOOL)avoidsUnsafeArea;
-
 - (void)_addShortcut:(id)sender;
 - (void)_arrowKey:(id)sender;

trunk/Source/WebKit/UIProcess/API/gtk/PageClientImpl.h

@@ -147 +147 @@
     WebCore::UserInterfaceLayoutDirection userInterfaceLayoutDirection() override { return WebCore::UserInterfaceLayoutDirection::LTR; }
 
-    void didChangeAvoidsUnsafeArea(bool) override { }
-
     JSGlobalContextRef javascriptGlobalContext() override;
 

trunk/Source/WebKit/UIProcess/API/wpe/PageClientImpl.h

@@ -118 +118 @@
     WebCore::UserInterfaceLayoutDirection userInterfaceLayoutDirection() override;
 
-    void didChangeAvoidsUnsafeArea(bool) override { }
-
     JSGlobalContextRef javascriptGlobalContext() override;
 

trunk/Source/WebKit/UIProcess/PageClient.h

@@ -385 +385 @@
 #endif
 
-    virtual void didChangeAvoidsUnsafeArea(bool avoidsUnsafeArea) = 0;
-
 #if PLATFORM(GTK) || PLATFORM(WPE)
     virtual JSGlobalContextRef javascriptGlobalContext() { return nullptr; }

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -5515 +5515 @@
     requestPointerUnlock();
 #endif
-
-    m_avoidsUnsafeArea = true;
 }
 
@@ -6951 +6949 @@
 }
 
-void WebPageProxy::setAvoidsUnsafeArea(bool avoidsUnsafeArea)
-{
-    if (m_avoidsUnsafeArea == avoidsUnsafeArea)
-        return;
-
-    m_avoidsUnsafeArea = avoidsUnsafeArea;
-
-    m_pageClient.didChangeAvoidsUnsafeArea(avoidsUnsafeArea);
-}
-
 } // namespace WebKit

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1202 +1202 @@
 #endif
 
-    void setAvoidsUnsafeArea(bool);
-    bool avoidsUnsafeArea() const { return m_avoidsUnsafeArea; }
-
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, uint64_t pageID, Ref<API::PageConfiguration>&&);
@@ -2001 +1998 @@
     bool m_isUsingHighPerformanceWebGL { false };
 
-    bool m_avoidsUnsafeArea { true };
-
     WeakPtrFactory<WebPageProxy> m_weakPtrFactory;
 

trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.h

@@ -200 +200 @@
     void handleActiveNowPlayingSessionInfoResponse(bool hasActiveSession, const String& title, double duration, double elapsedTime) override;
 
-    void didChangeAvoidsUnsafeArea(bool avoidsUnsafeArea) override;
-
 #if USE(QUICK_LOOK)
     void requestPasswordForQuickLookDocument(const String& fileName, WTF::Function<void(const String&)>&&) override;

trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm

@@ -827 +827 @@
 #endif
 
-void PageClientImpl::didChangeAvoidsUnsafeArea(bool avoidsUnsafeArea)
-{
-    [m_webView _didChangeAvoidsUnsafeArea:avoidsUnsafeArea];
-}
-
 } // namespace WebKit
 

trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

@@ -373 +373 @@
 {
     m_pageExtendedBackgroundColor = layerTreeTransaction.pageExtendedBackgroundColor();
-    setAvoidsUnsafeArea(layerTreeTransaction.avoidsUnsafeArea());
 
     if (!m_hasReceivedLayerTreeTransactionAfterDidCommitLoad) {

trunk/Source/WebKit/UIProcess/mac/PageClientImpl.h

@@ -254 +254 @@
     void didRestoreScrollPosition() override;
     bool windowIsFrontWindowUnderMouse(const NativeWebMouseEvent&) override;
-
-    void didChangeAvoidsUnsafeArea(bool avoidsUnsafeArea) override { }
 };