[iOS][WK2] When a page does not finish rotation before the end of the animation,...
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 31 May 2014 00:56:51 +0000 (00:56 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 31 May 2014 00:56:51 +0000 (00:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133364
<rdar://problem/17026333>

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-05-30
Reviewed by Sam Weinig.

When a page that does layout on rotation does not respond before the end of the animation, we end up with
a completely inconsistent state on the UIProcess (because it is unware of the new states).

The perfect way to fix this would be to make animated resize transactional and have the WebProcess resolve
conflicts. That is very complicated and the issue is uncommon, so this patch does not do that.

This patch force the synchronization whenever we finish the animation before we heard back
from the WebProcess.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _endAnimatedResize]):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::resetState):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::dynamicViewportSizeUpdate):
(WebKit::WebPageProxy::synchronizeDynamicViewportUpdate):
(WebKit::WebPageProxy::dynamicViewportUpdateChangedTarget):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::dynamicViewportSizeUpdate):
(WebKit::WebPage::synchronizeDynamicViewportUpdate):

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm
Source/WebKit2/WebProcess/WebPage/WebPage.h
Source/WebKit2/WebProcess/WebPage/WebPage.messages.in
Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm

index 7d439e2..3c20637 100644 (file)
@@ -1,3 +1,36 @@
+2014-05-30  Benjamin Poulain  <bpoulain@apple.com>
+
+        [iOS][WK2] When a page does not finish rotation before the end of the animation, synchronize explicitely
+        https://bugs.webkit.org/show_bug.cgi?id=133364
+        <rdar://problem/17026333>
+
+        Reviewed by Sam Weinig.
+
+        When a page that does layout on rotation does not respond before the end of the animation, we end up with
+        a completely inconsistent state on the UIProcess (because it is unware of the new states).
+
+        The perfect way to fix this would be to make animated resize transactional and have the WebProcess resolve
+        conflicts. That is very complicated and the issue is uncommon, so this patch does not do that.
+
+        This patch force the synchronization whenever we finish the animation before we heard back
+        from the WebProcess.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _endAnimatedResize]):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::WebPageProxy):
+        (WebKit::WebPageProxy::resetState):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::dynamicViewportSizeUpdate):
+        (WebKit::WebPageProxy::synchronizeDynamicViewportUpdate):
+        (WebKit::WebPageProxy::dynamicViewportUpdateChangedTarget):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::dynamicViewportSizeUpdate):
+        (WebKit::WebPage::synchronizeDynamicViewportUpdate):
+
 2014-05-30  Enrica Casucci  <enrica@apple.com>
 
         REGRESSION (WebKit2): space space to insert period doesn't work in web forms.
index cb528a0..bbb2557 100644 (file)
@@ -1780,6 +1780,8 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
 
 - (void)_endAnimatedResize
 {
+    _page->synchronizeDynamicViewportUpdate();
+
     if (!_customContentView && _isAnimatingResize) {
         NSUInteger indexOfResizeAnimationView = [[_scrollView subviews] indexOfObject:_resizeAnimationView.get()];
         [_scrollView insertSubview:_contentView.get() atIndex:indexOfResizeAnimationView];
index 2254eaf..c55599a 100644 (file)
@@ -272,6 +272,9 @@ WebPageProxy::WebPageProxy(PageClient& pageClient, WebProcessProxy& process, uin
     , m_visitedLinkProvider(*configuration.visitedLinkProvider)
     , m_mainFrame(nullptr)
     , m_userAgent(standardUserAgent())
+#if PLATFORM(IOS)
+    , m_dynamicViewportSizeUpdateInProgress(false)
+#endif
     , m_geolocationPermissionRequestManager(*this)
     , m_notificationPermissionRequestManager(*this)
     , m_viewState(ViewState::NoFlags)
@@ -4204,6 +4207,7 @@ void WebPageProxy::resetState()
     }
 
     m_lastVisibleContentRectUpdate = VisibleContentRectUpdateInfo();
+    m_dynamicViewportSizeUpdateInProgress = false;
 #endif
 
     invalidateCallbackMap(m_voidCallbacks);
index cc3946c..c755b11 100644 (file)
@@ -596,7 +596,8 @@ public:
     WebCore::FloatRect computeCustomFixedPositionRect(const WebCore::FloatRect& unobscuredContentRect, double displayedContentScale, UnobscuredRectConstraint = UnobscuredRectConstraint::Unconstrained) const;
 
     void dynamicViewportSizeUpdate(const WebCore::FloatSize& minimumLayoutSize, const WebCore::FloatSize& minimumLayoutSizeForMinimalUI, const WebCore::FloatSize& maximumUnobscuredSize, const WebCore::FloatRect& targetExposedContentRect, const WebCore::FloatRect& targetUnobscuredRect, const WebCore::FloatRect& targetUnobscuredRectInScrollViewCoordinates, double targetScale);
-    
+    void synchronizeDynamicViewportUpdate();
+
     void setViewportConfigurationMinimumLayoutSize(const WebCore::FloatSize&);
     void setViewportConfigurationMinimumLayoutSizeForMinimalUI(const WebCore::FloatSize&);
     void setMaximumUnobscuredSize(const WebCore::FloatSize&);
@@ -1489,6 +1490,7 @@ private:
 #if PLATFORM(IOS)
     RefPtr<WebVideoFullscreenManagerProxy> m_videoFullscreenManager;
     VisibleContentRectUpdateInfo m_lastVisibleContentRectUpdate;
+    bool m_dynamicViewportSizeUpdateInProgress;
 #endif
 
 #if ENABLE(VIBRATION)
index 5200aa8..396c4c1 100644 (file)
@@ -33,6 +33,7 @@
 #import "EditingRange.h"
 #import "NativeWebKeyboardEvent.h"
 #import "PageClient.h"
+#import "RemoteLayerTreeDrawingAreaProxyMessages.h"
 #import "RemoteLayerTreeTransaction.h"
 #import "ViewUpdateDispatcherMessages.h"
 #import "WKBrowsingContextControllerInternal.h"
@@ -237,9 +238,35 @@ WebCore::FloatRect WebPageProxy::computeCustomFixedPositionRect(const FloatRect&
 
 void WebPageProxy::dynamicViewportSizeUpdate(const FloatSize& minimumLayoutSize, const WebCore::FloatSize& minimumLayoutSizeForMinimalUI, const WebCore::FloatSize& maximumUnobscuredSize, const FloatRect& targetExposedContentRect, const FloatRect& targetUnobscuredRect, const FloatRect& targetUnobscuredRectInScrollViewCoordinates,  double targetScale)
 {
+    m_dynamicViewportSizeUpdateInProgress = true;
     m_process->send(Messages::WebPage::DynamicViewportSizeUpdate(minimumLayoutSize, minimumLayoutSizeForMinimalUI, maximumUnobscuredSize, targetExposedContentRect, targetUnobscuredRect, targetUnobscuredRectInScrollViewCoordinates, targetScale), m_pageID);
 }
 
+void WebPageProxy::synchronizeDynamicViewportUpdate()
+{
+    if (m_dynamicViewportSizeUpdateInProgress) {
+        // We do not want the UIProcess to finish animated resize with the old content size, scale, etc.
+        // If that happens, the UIProcess would start pushing new VisibleContentRectUpdateInfo to the WebProcess with
+        // invalid informations.
+        //
+        // Ideally, the animated resize should just be transactional, and the UIProcess would remain in the "resize" state
+        // until both DynamicViewportUpdateChangedTarget and the associated commitLayerTree are finished.
+        // The tricky part with such implementation is if a second animated resize starts before the end of the previous one.
+        // In that case, the values used for the target state needs to be computed from the output of the previous animated resize.
+        //
+        // The following is a workaround to have the UIProcess in a consistent state.
+        // Instead of handling nested resize, we block the UIProcess until the animated resize finishes.
+        m_dynamicViewportSizeUpdateInProgress = false;
+        double newScale;
+        FloatPoint newScrollPosition;
+        if (m_process->sendSync(Messages::WebPage::SynchronizeDynamicViewportUpdate(), Messages::WebPage::SynchronizeDynamicViewportUpdate::Reply(newScale, newScrollPosition), m_pageID, std::chrono::seconds(2))) {
+            m_pageClient.dynamicViewportUpdateChangedTarget(newScale, newScrollPosition);
+
+            m_process->connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_pageID, std::chrono::seconds(1));
+        }
+    }
+}
+
 void WebPageProxy::setViewportConfigurationMinimumLayoutSize(const WebCore::FloatSize& size)
 {
     m_process->send(Messages::WebPage::SetViewportConfigurationMinimumLayoutSize(size), m_pageID);
@@ -546,10 +573,13 @@ float WebPageProxy::textAutosizingWidth()
 {
     return WKGetScreenSize().width;
 }
-    
+
 void WebPageProxy::dynamicViewportUpdateChangedTarget(double newScale, const WebCore::FloatPoint& newScrollPosition)
 {
-    m_pageClient.dynamicViewportUpdateChangedTarget(newScale, newScrollPosition);
+    if (m_dynamicViewportSizeUpdateInProgress) {
+        m_dynamicViewportSizeUpdateInProgress = false;
+        m_pageClient.dynamicViewportUpdateChangedTarget(newScale, newScrollPosition);
+    }
 }
 
 void WebPageProxy::didGetTapHighlightGeometries(uint64_t requestID, const WebCore::Color& color, const Vector<WebCore::FloatQuad>& highlightedQuads, const WebCore::IntSize& topLeftRadius, const WebCore::IntSize& topRightRadius, const WebCore::IntSize& bottomLeftRadius, const WebCore::IntSize& bottomRightRadius)
index a0887aa..5b8f8c3 100644 (file)
@@ -724,6 +724,7 @@ public:
     void setViewportConfigurationMinimumLayoutSizeForMinimalUI(const WebCore::FloatSize&);
     void setMaximumUnobscuredSize(const WebCore::FloatSize&);
     void dynamicViewportSizeUpdate(const WebCore::FloatSize& minimumLayoutSize, const WebCore::FloatSize& minimumLayoutSizeForMinimalUI, const WebCore::FloatSize& maximumUnobscuredSize, const WebCore::FloatRect& targetExposedContentRect, const WebCore::FloatRect& targetUnobscuredRect, const WebCore::FloatRect& targetUnobscuredRectInScrollViewCoordinates, double scale);
+    void synchronizeDynamicViewportUpdate(double& newTargetScale, WebCore::FloatPoint& newScrollPosition);
     void updateVisibleContentRects(const VisibleContentRectUpdateInfo&);
     bool scaleWasSetByUIProcess() const { return m_scaleWasSetByUIProcess; }
     void willStartUserTriggeredZooming();
index 128eac6..636de7d 100644 (file)
@@ -47,6 +47,7 @@ messages -> WebPage LegacyReceiver {
     SetViewportConfigurationMinimumLayoutSizeForMinimalUI(WebCore::FloatSize size)
     SetMaximumUnobscuredSize(WebCore::FloatSize size)
     DynamicViewportSizeUpdate(WebCore::FloatSize minimumLayoutSize, WebCore::FloatSize minimumLayoutSizeForMinimalUI,  WebCore::FloatSize maximumUnobscuredSize, WebCore::FloatRect targetExposedContentRect, WebCore::FloatRect targetUnobscuredRect, WebCore::FloatRect targetUnobscuredRectInScrollViewCoordinates, double scale)
+    SynchronizeDynamicViewportUpdate() -> (double newTargetScale, WebCore::FloatPoint newScrollPosition)
 
     HandleTap(WebCore::IntPoint point)
     PotentialTapAtPosition(uint64_t requestID, WebCore::FloatPoint point)
index 8aa1fef..7fb20ca 100644 (file)
@@ -2151,8 +2151,13 @@ void WebPage::dynamicViewportSizeUpdate(const FloatSize& minimumLayoutSize, cons
     frameView.setCustomSizeForResizeEvent(expandedIntSize(unobscuredContentRectSizeInContentCoordinates));
     frameView.setScrollOffset(roundedUnobscuredContentRect.location());
 
-    if (!withinEpsilon(pageScaleFactor(), targetScale) || roundedIntPoint(targetUnobscuredRect.location()) != frameView.scrollPosition())
-        send(Messages::WebPageProxy::DynamicViewportUpdateChangedTarget(pageScaleFactor(), frameView.scrollPosition()));
+    send(Messages::WebPageProxy::DynamicViewportUpdateChangedTarget(pageScaleFactor(), frameView.scrollPosition()));
+}
+
+void WebPage::synchronizeDynamicViewportUpdate(double& newTargetScale, FloatPoint& newScrollPosition)
+{
+    newTargetScale = pageScaleFactor();
+    newScrollPosition = m_page->mainFrame().view()->scrollPosition();
 }
 
 void WebPage::resetViewportDefaultConfiguration(WebFrame* frame)