ASSERT(CGSizeEqualToSize(m_resizeScrollOffset, CGSizeZero)) in WebViewImpl::setFrameA...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jan 2018 22:27:27 +0000 (22:27 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jan 2018 22:27:27 +0000 (22:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182082
rdar://problem/13971838

Reviewed by Tim Horton.

Safari could call WebViewImpl::setFrameAndScrollBy() multiple times with different scroll offsets,
triggering this assertion.

Rename to m_resizeScrollOffset to m_scrollOffsetAdjustment to reduce confusion with actual scroll offsets.
This parameter has no effect on macOS, but is used by the -[WKWebView setFrame:andScrollBy:] so at some point
needs to be hooked up to allow synchronous view resize and scroll adjustment (e.g. for the Find bar animation).

Remove DrawingAreaProxy's m_layerPosition which was unused, and remove the parameters from the UpdateGeometry message.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _frameOrBoundsChanged]):
(-[WKWebView _beginAnimatedResizeWithUpdates:]):
* UIProcess/Cocoa/WebViewImpl.h:
* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::WebViewImpl::setFrameAndScrollBy):
(WebKit::WebViewImpl::setDrawingAreaSize):
* UIProcess/DrawingAreaProxy.cpp:
(WebKit::DrawingAreaProxy::setSize):
* UIProcess/DrawingAreaProxy.h:
* UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::sendUpdateGeometry):
* UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:
(WebKit::TiledCoreAnimationDrawingAreaProxy::sendUpdateGeometry):
* WebProcess/WebPage/DrawingArea.h:
(WebKit::DrawingArea::updateGeometry):
* WebProcess/WebPage/DrawingArea.messages.in:
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::updateGeometry):
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::updateGeometry):

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

16 files changed:
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
Source/WebKit/UIProcess/API/wpe/WPEView.cpp
Source/WebKit/UIProcess/Cocoa/WebViewImpl.h
Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
Source/WebKit/UIProcess/DrawingAreaProxy.cpp
Source/WebKit/UIProcess/DrawingAreaProxy.h
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
Source/WebKit/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm
Source/WebKit/WebProcess/WebPage/DrawingArea.h
Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

index c4d18fb..ff22f4c 100644 (file)
@@ -1,3 +1,44 @@
+2018-01-25  Simon Fraser  <simon.fraser@apple.com>
+
+        ASSERT(CGSizeEqualToSize(m_resizeScrollOffset, CGSizeZero)) in WebViewImpl::setFrameAndScrollBy()
+        https://bugs.webkit.org/show_bug.cgi?id=182082
+        rdar://problem/13971838
+
+        Reviewed by Tim Horton.
+
+        Safari could call WebViewImpl::setFrameAndScrollBy() multiple times with different scroll offsets,
+        triggering this assertion.
+
+        Rename to m_resizeScrollOffset to m_scrollOffsetAdjustment to reduce confusion with actual scroll offsets.
+        This parameter has no effect on macOS, but is used by the -[WKWebView setFrame:andScrollBy:] so at some point
+        needs to be hooked up to allow synchronous view resize and scroll adjustment (e.g. for the Find bar animation).
+        
+        Remove DrawingAreaProxy's m_layerPosition which was unused, and remove the parameters from the UpdateGeometry message.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _frameOrBoundsChanged]):
+        (-[WKWebView _beginAnimatedResizeWithUpdates:]):
+        * UIProcess/Cocoa/WebViewImpl.h:
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (WebKit::WebViewImpl::setFrameAndScrollBy):
+        (WebKit::WebViewImpl::setDrawingAreaSize):
+        * UIProcess/DrawingAreaProxy.cpp:
+        (WebKit::DrawingAreaProxy::setSize):
+        * UIProcess/DrawingAreaProxy.h:
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
+        (WebKit::RemoteLayerTreeDrawingAreaProxy::sendUpdateGeometry):
+        * UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:
+        (WebKit::TiledCoreAnimationDrawingAreaProxy::sendUpdateGeometry):
+        * WebProcess/WebPage/DrawingArea.h:
+        (WebKit::DrawingArea::updateGeometry):
+        * WebProcess/WebPage/DrawingArea.messages.in:
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
+        (WebKit::RemoteLayerTreeDrawingArea::updateGeometry):
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::updateGeometry):
+
 2018-01-25  Keith Rollin  <krollin@apple.com>
 
         Add logging to facilitate binding of WebContent and Network processes to UI process
index feef2bb..9a19d52 100644 (file)
@@ -2517,7 +2517,7 @@ static WebCore::FloatSize activeMinimumLayoutSize(WKWebView *webView, const CGRe
 
         BOOL sizeChanged = NO;
         if (auto drawingArea = _page->drawingArea())
-            sizeChanged = drawingArea->setSize(WebCore::IntSize(bounds.size), WebCore::IntSize(), WebCore::IntSize());
+            sizeChanged = drawingArea->setSize(WebCore::IntSize(bounds.size));
 
         if (sizeChanged & [self usesStandardContentView])
             [_contentView setSizeChangedSinceLastVisibleContentRectUpdate:YES];
@@ -5136,7 +5136,7 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
 
     _page->dynamicViewportSizeUpdate(newMinimumLayoutSize, newMaximumUnobscuredSize, visibleRectInContentCoordinates, unobscuredRectInContentCoordinates, futureUnobscuredRectInSelfCoordinates, unobscuredSafeAreaInsetsExtent, targetScale, newOrientation);
     if (WebKit::DrawingAreaProxy* drawingArea = _page->drawingArea())
-        drawingArea->setSize(WebCore::IntSize(newBounds.size), WebCore::IntSize(), WebCore::IntSize());
+        drawingArea->setSize(WebCore::IntSize(newBounds.size));
 }
 
 - (void)_endAnimatedResize
index caf75b7..563049d 100644 (file)
@@ -602,7 +602,7 @@ static void webkitWebViewBaseSizeAllocate(GtkWidget* widget, GtkAllocation* allo
     }
 
     if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea()))
-        drawingArea->setSize(viewRect.size(), IntSize(), IntSize());
+        drawingArea->setSize(viewRect.size());
 }
 
 static void webkitWebViewBaseGetPreferredWidth(GtkWidget* widget, gint* minimumSize, gint* naturalSize)
index e79977b..49b6424 100644 (file)
@@ -165,7 +165,7 @@ void View::setSize(const WebCore::IntSize& size)
 {
     m_size = size;
     if (m_pageProxy->drawingArea())
-        m_pageProxy->drawingArea()->setSize(size, WebCore::IntSize(), WebCore::IntSize());
+        m_pageProxy->drawingArea()->setSize(size);
 }
 
 void View::setViewState(WebCore::ActivityState::Flags flags)
index 1977941..7429260 100644 (file)
@@ -622,8 +622,8 @@ private:
     bool m_automaticallyAdjustsContentInsets { false };
     CGFloat m_pendingTopContentInset { 0 };
     bool m_didScheduleSetTopContentInset { false };
-
-    CGSize m_resizeScrollOffset { 0, 0 };
+    
+    CGSize m_scrollOffsetAdjustment { 0, 0 };
 
     CGSize m_intrinsicContentSize { 0, 0 };
     CGFloat m_overrideDeviceScaleFactor { 0 };
index 9a666fa..b9af3dd 100644 (file)
@@ -1527,11 +1527,11 @@ bool WebViewImpl::frameSizeUpdatesDisabled() const
     return [m_layoutStrategy frameSizeUpdatesDisabled];
 }
 
-void WebViewImpl::setFrameAndScrollBy(CGRect frame, CGSize offset)
+void WebViewImpl::setFrameAndScrollBy(CGRect frame, CGSize scrollDelta)
 {
-    ASSERT(CGSizeEqualToSize(m_resizeScrollOffset, CGSizeZero));
+    if (!CGSizeEqualToSize(scrollDelta, CGSizeZero))
+        m_scrollOffsetAdjustment = scrollDelta;
 
-    m_resizeScrollOffset = offset;
     [m_view frame] = NSRectFromCGRect(frame);
 }
 
@@ -1600,8 +1600,8 @@ void WebViewImpl::setDrawingAreaSize(CGSize size)
     if (!m_page->drawingArea())
         return;
 
-    m_page->drawingArea()->setSize(WebCore::IntSize(size), WebCore::IntSize(), WebCore::IntSize(m_resizeScrollOffset));
-    m_resizeScrollOffset = CGSizeZero;
+    m_page->drawingArea()->setSize(WebCore::IntSize(size), WebCore::IntSize(m_scrollOffsetAdjustment));
+    m_scrollOffsetAdjustment = CGSizeZero;
 }
 
 void WebViewImpl::updateLayer()
index c55c3f5..fb2eb42 100644 (file)
@@ -55,14 +55,13 @@ DrawingAreaProxy::~DrawingAreaProxy()
     m_webPageProxy.process().removeMessageReceiver(Messages::DrawingAreaProxy::messageReceiverName(), m_webPageProxy.pageID());
 }
 
-bool DrawingAreaProxy::setSize(const IntSize& size, const IntSize& layerPosition, const IntSize& scrollOffset)
+bool DrawingAreaProxy::setSize(const IntSize& size, const IntSize& scrollDelta)
 { 
-    if (m_size == size && m_layerPosition == layerPosition && scrollOffset.isZero())
+    if (m_size == size && scrollDelta.isZero())
         return false;
 
     m_size = size;
-    m_layerPosition = layerPosition;
-    m_scrollOffset += scrollOffset;
+    m_scrollOffset += scrollDelta;
     sizeDidChange();
     return true;
 }
index ad8356c..04c52af 100644 (file)
@@ -24,8 +24,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef DrawingAreaProxy_h
-#define DrawingAreaProxy_h
+#pragma once
 
 #include "DrawingAreaInfo.h"
 #include "GenericCallback.h"
@@ -66,7 +65,7 @@ public:
     virtual void waitForBackingStoreUpdateOnNextPaint() { }
 
     const WebCore::IntSize& size() const { return m_size; }
-    bool setSize(const WebCore::IntSize&, const WebCore::IntSize&, const WebCore::IntSize& scrollOffset);
+    bool setSize(const WebCore::IntSize&, const WebCore::IntSize& scrollOffset = { });
 
     // The timeout we use when waiting for a DidUpdateGeometry message.
     static constexpr Seconds didUpdateBackingStoreStateTimeout() { return Seconds::fromMilliseconds(500); }
@@ -114,7 +113,6 @@ protected:
     WebPageProxy& m_webPageProxy;
 
     WebCore::IntSize m_size;
-    WebCore::IntSize m_layerPosition;
     WebCore::IntSize m_scrollOffset;
 
     // IPC::MessageReceiver
@@ -149,4 +147,3 @@ SPECIALIZE_TYPE_TRAITS_BEGIN(WebKit::ToValueTypeName) \
     static bool isType(const WebKit::DrawingAreaProxy& proxy) { return proxy.type() == WebKit::ProxyType; } \
 SPECIALIZE_TYPE_TRAITS_END()
 
-#endif // DrawingAreaProxy_h
index 1c2518e..e23c9ee 100644 (file)
@@ -172,7 +172,7 @@ void RemoteLayerTreeDrawingAreaProxy::didUpdateGeometry()
 void RemoteLayerTreeDrawingAreaProxy::sendUpdateGeometry()
 {
     m_lastSentSize = m_size;
-    m_webPageProxy.process().send(Messages::DrawingArea::UpdateGeometry(m_size, IntSize(), false, MachSendRight()), m_webPageProxy.pageID());
+    m_webPageProxy.process().send(Messages::DrawingArea::UpdateGeometry(m_size, false /* flushSynchronously */, MachSendRight()), m_webPageProxy.pageID());
     m_isWaitingForDidUpdateGeometry = true;
 }
 
index b9e0073..6c72796 100644 (file)
@@ -183,7 +183,7 @@ void TiledCoreAnimationDrawingAreaProxy::sendUpdateGeometry()
     ASSERT(!m_isWaitingForDidUpdateGeometry);
 
     willSendUpdateGeometry();
-    m_webPageProxy.process().send(Messages::DrawingArea::UpdateGeometry(m_size, m_layerPosition, true, createFence()), m_webPageProxy.pageID());
+    m_webPageProxy.process().send(Messages::DrawingArea::UpdateGeometry(m_size, true /* flushSynchronously */, createFence()), m_webPageProxy.pageID());
 }
 
 void TiledCoreAnimationDrawingAreaProxy::adjustTransientZoom(double scale, FloatPoint origin)
index 88e90e8..4284dec 100644 (file)
@@ -131,7 +131,7 @@ public:
 
 #if PLATFORM(COCOA)
     // Used by TiledCoreAnimationDrawingArea.
-    virtual void updateGeometry(const WebCore::IntSize& viewSize, const WebCore::IntSize& layerPosition, bool flushSynchronously, const WebCore::MachSendRight& fencePort) { }
+    virtual void updateGeometry(const WebCore::IntSize& viewSize, bool flushSynchronously, const WebCore::MachSendRight& fencePort) { }
 #endif
 
     virtual void layerHostDidFlushLayers() { };
index 6f58fe8..993b1c4 100644 (file)
@@ -26,7 +26,7 @@ messages -> DrawingArea {
 
 #if PLATFORM(COCOA)
     // Used by TiledCoreAnimationDrawingArea.
-    UpdateGeometry(WebCore::IntSize viewSize, WebCore::IntSize layerPosition, bool flushSynchronously, WebCore::MachSendRight fencePort)
+    UpdateGeometry(WebCore::IntSize viewSize, bool flushSynchronously, WebCore::MachSendRight fencePort)
     SetDeviceScaleFactor(float deviceScaleFactor)
     SetColorSpace(struct WebKit::ColorSpaceData colorSpace)
     SetViewExposedRect(std::optional<WebCore::FloatRect> viewExposedRect)
index e58dd61..555c584 100644 (file)
@@ -61,7 +61,7 @@ private:
     void setNeedsDisplay() override;
     void setNeedsDisplayInRect(const WebCore::IntRect&) override;
     void scroll(const WebCore::IntRect& scrollRect, const WebCore::IntSize& scrollDelta) override;
-    void updateGeometry(const WebCore::IntSize& viewSize, const WebCore::IntSize& layerPosition, bool flushSynchronously, const WebCore::MachSendRight& fencePort) override;
+    void updateGeometry(const WebCore::IntSize& viewSize, bool flushSynchronously, const WebCore::MachSendRight& fencePort) override;
 
     WebCore::GraphicsLayerFactory* graphicsLayerFactory() override;
     void setRootCompositingLayer(WebCore::GraphicsLayer*) override;
index 9eba9a9..243c7fa 100644 (file)
@@ -143,7 +143,7 @@ void RemoteLayerTreeDrawingArea::setRootCompositingLayer(GraphicsLayer* rootLaye
     scheduleCompositingLayerFlush();
 }
 
-void RemoteLayerTreeDrawingArea::updateGeometry(const IntSize& viewSize, const IntSize& layerPosition, bool flushSynchronously, const WebCore::MachSendRight&)
+void RemoteLayerTreeDrawingArea::updateGeometry(const IntSize& viewSize, bool flushSynchronously, const WebCore::MachSendRight&)
 {
     m_viewSize = viewSize;
     m_webPage.setSize(viewSize);
index 50c4126..b9cc751 100644 (file)
@@ -23,8 +23,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef TiledCoreAnimationDrawingArea_h
-#define TiledCoreAnimationDrawingArea_h
+#pragma once
 
 #if !PLATFORM(IOS)
 
@@ -93,7 +92,7 @@ private:
     bool flushLayers() override;
 
     // Message handlers.
-    void updateGeometry(const WebCore::IntSize& viewSize, const WebCore::IntSize& layerPosition, bool flushSynchronously, const WebCore::MachSendRight& fencePort) override;
+    void updateGeometry(const WebCore::IntSize& viewSize, bool flushSynchronously, const WebCore::MachSendRight& fencePort) override;
     void setDeviceScaleFactor(float) override;
     void suspendPainting();
     void resumePainting();
@@ -171,4 +170,3 @@ SPECIALIZE_TYPE_TRAITS_DRAWING_AREA(TiledCoreAnimationDrawingArea, DrawingAreaTy
 
 #endif // !PLATFORM(IOS)
 
-#endif // TiledCoreAnimationDrawingArea_h
index 0ddf3c9..a77221c 100644 (file)
@@ -540,7 +540,7 @@ void TiledCoreAnimationDrawingArea::updateScrolledExposedRect()
     frameView->setViewExposedRect(m_scrolledViewExposedRect);
 }
 
-void TiledCoreAnimationDrawingArea::updateGeometry(const IntSize& viewSize, const IntSize& layerPosition, bool flushSynchronously, const WebCore::MachSendRight& fencePort)
+void TiledCoreAnimationDrawingArea::updateGeometry(const IntSize& viewSize, bool flushSynchronously, const WebCore::MachSendRight& fencePort)
 {
     m_inUpdateGeometry = true;
 
@@ -568,7 +568,7 @@ void TiledCoreAnimationDrawingArea::updateGeometry(const IntSize& viewSize, cons
     [CATransaction begin];
     [CATransaction setDisableActions:YES];
 
-    [m_hostingLayer setFrame:CGRectMake(layerPosition.width(), layerPosition.height(), viewSize.width(), viewSize.height())];
+    [m_hostingLayer setFrame:CGRectMake(0, 0, viewSize.width(), viewSize.height())];
 
     [CATransaction commit];