REGRESSION (r229828): web view doesn’t update or respond to resizing until client...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Apr 2018 01:55:35 +0000 (01:55 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Apr 2018 01:55:35 +0000 (01:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184210
<rdar://problem/39072354>

Reviewed by Wenson Hsieh.

Source/WebCore:

r229828 tried to have some API tests happy on iOS by freezing the layer tree
during the navigation policy decision. However, this is observable by the client
application and a regression from when the policy delegate was synchronous.

To address the issue, this patch reverts r229828 and instead updates the iOS
API tests to wait for the next presentation update after navigating
before interacting with the view.

* loader/FrameLoaderClient.h:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):

Source/WebKit:

r229828 tried to have some API tests happy on iOS by freezing the layer tree
during the navigation policy decision. However, this is observable by the client
application and a regression from when the policy delegate was synchronous.

To address the issue, this patch reverts r229828 and instead updates the iOS
API tests to wait for the next presentation update after navigating
before interacting with the view.

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
(WebKit::WebFrameLoaderClient::cancelPolicyCheck):
(WebKit::WebFrameLoaderClient::provisionalLoadStarted):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
* WebProcess/WebPage/WebPage.cpp:
* WebProcess/WebPage/WebPage.h:

Tools:

* TestWebKitAPI/Tests/WebKit/large-red-square-image.html:
* TestWebKitAPI/Tests/WebKitCocoa/dragstart-change-selection-offscreen.html:
Add viewport meta tags.

* TestWebKitAPI/cocoa/TestNavigationDelegate.mm:
(-[WKWebView _test_waitForDidFinishNavigation]):
Update _test_waitForDidFinishNavigation to wait for the next presentation update
to make iOS API tests happy without having to modify each of them.

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

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoaderClient.h
Source/WebCore/loader/PolicyChecker.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit/large-red-square-image.html
Tools/TestWebKitAPI/Tests/WebKitCocoa/dragstart-change-selection-offscreen.html
Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.mm

index 35e84e3..34a7a06 100644 (file)
@@ -1,3 +1,23 @@
+2018-04-20  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r229828): web view doesn’t update or respond to resizing until client calls policy decision handler
+        https://bugs.webkit.org/show_bug.cgi?id=184210
+        <rdar://problem/39072354>
+
+        Reviewed by Wenson Hsieh.
+
+        r229828 tried to have some API tests happy on iOS by freezing the layer tree
+        during the navigation policy decision. However, this is observable by the client
+        application and a regression from when the policy delegate was synchronous.
+
+        To address the issue, this patch reverts r229828 and instead updates the iOS
+        API tests to wait for the next presentation update after navigating
+        before interacting with the view.
+
+        * loader/FrameLoaderClient.h:
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy):
+
 2018-04-20  Brent Fulgham  <bfulgham@apple.com>
 
         Limit cookie header access to Network process
index 3da90f2..330b6ef 100644 (file)
@@ -192,7 +192,6 @@ public:
     virtual void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) = 0;
     virtual void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String& frameName, FramePolicyFunction&&) = 0;
     virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool didReceiveRedirectResponse, FormState*, PolicyDecisionMode, FramePolicyFunction&&) = 0;
-    virtual void didDecidePolicyForNavigationAction() { }
     virtual void cancelPolicyCheck() = 0;
 
     virtual void dispatchUnableToImplementPolicy(const ResourceError&) = 0;
index 81318a7..fdeda3f 100644 (file)
@@ -150,8 +150,6 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
     String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
     ResourceRequest requestCopy = request;
     m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename)](PolicyAction policyAction) mutable {
-        m_frame.loader().client().didDecidePolicyForNavigationAction();
-
         m_delegateIsDecidingNavigationPolicy = false;
 
         switch (policyAction) {
index b1b4995..e2290cf 100644 (file)
@@ -1,3 +1,28 @@
+2018-04-20  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r229828): web view doesn’t update or respond to resizing until client calls policy decision handler
+        https://bugs.webkit.org/show_bug.cgi?id=184210
+        <rdar://problem/39072354>
+
+        Reviewed by Wenson Hsieh.
+
+        r229828 tried to have some API tests happy on iOS by freezing the layer tree
+        during the navigation policy decision. However, this is observable by the client
+        application and a regression from when the policy delegate was synchronous.
+
+        To address the issue, this patch reverts r229828 and instead updates the iOS
+        API tests to wait for the next presentation update after navigating
+        before interacting with the view.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        (WebKit::WebFrameLoaderClient::cancelPolicyCheck):
+        (WebKit::WebFrameLoaderClient::provisionalLoadStarted):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        * WebProcess/WebPage/WebPage.h:
+
 2018-04-20  Brent Fulgham  <bfulgham@apple.com>
 
         Limit cookie header access to Network process
index 2700322..ef1c419 100644 (file)
@@ -738,8 +738,6 @@ void WebFrameLoaderClient::dispatchDecidePolicyForResponse(const ResourceRespons
         return;
     }
 
-    ASSERT(!m_isDecidingNavigationPolicyDecision);
-
     RefPtr<API::Object> userData;
 
     // Notify the bundle client.
@@ -825,10 +823,6 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
 
     LOG(Loading, "WebProcess %i - dispatchDecidePolicyForNavigationAction to request url %s", getCurrentProcessID(), request.url().string().utf8().data());
 
-    m_isDecidingNavigationPolicyDecision = true;
-    if (m_frame->isMainFrame())
-        webPage->didStartNavigationPolicyCheck();
-
     // Always ignore requests with empty URLs. 
     if (request.isEmpty()) {
         function(PolicyAction::Ignore);
@@ -908,25 +902,8 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
         m_frame->didReceivePolicyDecision(listenerID, PolicyAction::Ignore, 0, { }, { });
 }
 
-void WebFrameLoaderClient::didDecidePolicyForNavigationAction()
-{
-    if (!m_isDecidingNavigationPolicyDecision)
-        return;
-
-    m_isDecidingNavigationPolicyDecision = false;
-
-    if (!m_frame || !m_frame->isMainFrame())
-        return;
-
-    if (auto* webPage = m_frame->page())
-        webPage->didCompleteNavigationPolicyCheck();
-}
-
 void WebFrameLoaderClient::cancelPolicyCheck()
 {
-    if (m_isDecidingNavigationPolicyDecision)
-        didDecidePolicyForNavigationAction();
-
     m_frame->invalidatePolicyListener();
 }
 
@@ -1333,8 +1310,6 @@ void WebFrameLoaderClient::provisionalLoadStarted()
     if (!webPage)
         return;
 
-    ASSERT(!m_isDecidingNavigationPolicyDecision);
-
     if (m_frame->isMainFrame()) {
         webPage->didStartPageTransition();
         m_didCompletePageTransition = false;
index 7a7ef2a..1f92701 100644 (file)
@@ -126,7 +126,6 @@ private:
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const String& frameName, WebCore::FramePolicyFunction&&) final;
     void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::PolicyDecisionMode, WebCore::FramePolicyFunction&&) final;
-    void didDecidePolicyForNavigationAction() final;
     void cancelPolicyCheck() final;
     
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final;
@@ -278,7 +277,6 @@ private:
     WebFrame* m_frame;
     RefPtr<PluginView> m_pluginView;
     bool m_hasSentResponseToPluginView;
-    bool m_isDecidingNavigationPolicyDecision { false };
     bool m_didCompletePageTransition;
     bool m_frameHasCustomContentProvider;
     bool m_frameCameFromPageCache;
index b84c0eb..561a83b 100644 (file)
@@ -2839,16 +2839,6 @@ void WebPage::continueWillSubmitForm(uint64_t frameID, uint64_t listenerID)
     frame->continueWillSubmitForm(listenerID);
 }
 
-void WebPage::didStartNavigationPolicyCheck()
-{
-    m_drawingArea->setLayerTreeStateIsFrozen(true);
-}
-
-void WebPage::didCompleteNavigationPolicyCheck()
-{
-    m_drawingArea->setLayerTreeStateIsFrozen(false);
-}
-
 void WebPage::didStartPageTransition()
 {
     m_drawingArea->setLayerTreeStateIsFrozen(true);
index 2de69ad..d1a1006 100644 (file)
@@ -312,8 +312,6 @@ public:
     // -- Called from WebCore clients.
     bool handleEditingKeyboardEvent(WebCore::KeyboardEvent*);
 
-    void didStartNavigationPolicyCheck();
-    void didCompleteNavigationPolicyCheck();
     void didStartPageTransition();
     void didCompletePageTransition();
     void didCommitLoad(WebFrame*);
index 403ae9e..4cbe36f 100644 (file)
@@ -1,3 +1,20 @@
+2018-04-20  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r229828): web view doesn’t update or respond to resizing until client calls policy decision handler
+        https://bugs.webkit.org/show_bug.cgi?id=184210
+        <rdar://problem/39072354>
+
+        Reviewed by Wenson Hsieh.
+
+        * TestWebKitAPI/Tests/WebKit/large-red-square-image.html:
+        * TestWebKitAPI/Tests/WebKitCocoa/dragstart-change-selection-offscreen.html:
+        Add viewport meta tags.
+
+        * TestWebKitAPI/cocoa/TestNavigationDelegate.mm:
+        (-[WKWebView _test_waitForDidFinishNavigation]):
+        Update _test_waitForDidFinishNavigation to wait for the next presentation update
+        to make iOS API tests happy without having to modify each of them.
+
 2018-04-20  Timothy Hatcher  <timothy@apple.com>
 
         REGRESSION: API test WebKit.BackgroundColorSystemColor is failing
index 580809f..18e58f2 100644 (file)
@@ -1,3 +1,7 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
 <style>
     .red-box {
         background-image: url('large-red-square.png');
@@ -8,6 +12,8 @@
         left:0px;
     }
 </style>
+</head>
 <body>
     <div class="red-box"></div>
 </body>
+</html>
index 640b521..e39caf8 100644 (file)
@@ -1,5 +1,7 @@
 <!DOCTYPE html>
 <html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
 <style>
 body {
     font-size: 200px;
@@ -11,6 +13,8 @@ body {
     left: -500vw;
 }
 </style>
+</head>
+</body>
 <div id="onscreen">DRAG ME</div>
 <div id="offscreen">FAR OFFSCREEN</div>
 <script>
@@ -25,4 +29,5 @@ function selectChildNodesOfElement(element)
 selectChildNodesOfElement(onscreen);
 document.body.addEventListener("dragstart", () => selectChildNodesOfElement(offscreen));
 </script>
+</body>
 </html>
index 2bffcef..f5dfd5d 100644 (file)
     [navigationDelegate waitForDidFinishNavigation];
 
     self.navigationDelegate = nil;
+
+#if PLATFORM(IOS)
+    __block bool presentationUpdateHappened = false;
+    [self _doAfterNextPresentationUpdate:^{
+        presentationUpdateHappened = true;
+    }];
+    TestWebKitAPI::Util::run(&presentationUpdateHappened);
+#endif
 }
 
 @end