Do not pass API::FrameInfo for source frame or clear out page of target frame on
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jul 2017 20:10:19 +0000 (20:10 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jul 2017 20:10:19 +0000 (20:10 +0000)
API navigation
https://bugs.webkit.org/show_bug.cgi?id=174170
<rdar://problem/33140328>

Reviewed by Brady Eidson.

Source/WebKit2:

As a step towards making it straightforward for an embedding client to determine whether
a WebPageProxy::decidePolicyForNavigationAction() callback was initiated from API we
should not pass frame info for the source frame and should not nullify the page pointer
in the target frame info.

Currently we always pass frame info for the source frame and nullify the page pointer
in both the source frame info and target frame info if the navigation was initiated from
API. This seems subtle and error prone. Instead we should not pass frame info for
the source frame and not nullify the page pointer in the target frame info as a step
towards making using this API less error-prone.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction):

Tools:

Update tests as needed for the behavior change.

* TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm:
(TEST):

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebPageProxy.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm

index e5ab4b0..e2cd7d9 100644 (file)
@@ -1,3 +1,26 @@
+2017-07-05  Daniel Bates  <dabates@apple.com>
+
+        Do not pass API::FrameInfo for source frame or clear out page of target frame on
+        API navigation
+        https://bugs.webkit.org/show_bug.cgi?id=174170
+        <rdar://problem/33140328>
+
+        Reviewed by Brady Eidson.
+
+        As a step towards making it straightforward for an embedding client to determine whether
+        a WebPageProxy::decidePolicyForNavigationAction() callback was initiated from API we
+        should not pass frame info for the source frame and should not nullify the page pointer
+        in the target frame info.
+
+        Currently we always pass frame info for the source frame and nullify the page pointer
+        in both the source frame info and target frame info if the navigation was initiated from
+        API. This seems subtle and error prone. Instead we should not pass frame info for
+        the source frame and not nullify the page pointer in the target frame info as a step
+        towards making using this API less error-prone.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+
 2017-07-05  Chris Dumez  <cdumez@apple.com>
 
         Add a few more WebKit2 owners
index 44856ab..b4b8928 100644 (file)
@@ -3639,14 +3639,10 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
     if (m_navigationClient) {
         RefPtr<API::FrameInfo> destinationFrameInfo = API::FrameInfo::create(*frame, frameSecurityOrigin.securityOrigin());
         RefPtr<API::FrameInfo> sourceFrameInfo;
-        if (originatingFrame == frame)
+        if (!fromAPI && originatingFrame == frame)
             sourceFrameInfo = destinationFrameInfo;
-        else
+        else if (!fromAPI)
             sourceFrameInfo = API::FrameInfo::create(originatingFrameInfoData, m_process->webPage(originatingPageID));
-        if (fromAPI) {
-            sourceFrameInfo->clearPage();
-            destinationFrameInfo->clearPage();
-        }
 
         auto userInitiatedActivity = m_process->userInitiatedActivity(navigationActionData.userGestureTokenIdentifier);
         bool shouldOpenAppLinks = !m_shouldSuppressAppLinksInNextNavigationPolicyDecision && (!destinationFrameInfo || destinationFrameInfo->isMainFrame()) && !hostsAreEqual(URL(ParsedURLString, m_mainFrame->url()), request.url()) && navigationActionData.navigationType != WebCore::NavigationType::BackForward;
index 6246025..45aab8d 100644 (file)
@@ -1,3 +1,17 @@
+2017-07-05  Daniel Bates  <dabates@apple.com>
+
+        Do not pass API::FrameInfo for source frame or clear out page of target frame on
+        API navigation
+        https://bugs.webkit.org/show_bug.cgi?id=174170
+        <rdar://problem/33140328>
+
+        Reviewed by Brady Eidson.
+
+        Update tests as needed for the behavior change.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm:
+        (TEST):
+
 2017-07-05  Jonathan Bedard  <jbedard@apple.com>
 
         Add WebKitPrivateFrameworkStubs for iOS 11
index 7b2b8ab..3c14284 100644 (file)
@@ -91,10 +91,11 @@ TEST(WebKit2, DecidePolicyForNavigationActionReload)
     TestWebKitAPI::Util::run(&decidedPolicy);
 
     EXPECT_EQ(WKNavigationTypeReload, [action navigationType]);
-    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_TRUE([action sourceFrame] != [action targetFrame]);
+    EXPECT_EQ(nil, [action sourceFrame]);
     EXPECT_WK_STREQ(firstURL, [[[action request] URL] absoluteString]);
-    EXPECT_WK_STREQ(firstURL, [[[[action sourceFrame] request] URL] absoluteString]);
-    EXPECT_EQ(nil, [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ(firstURL, [[[[action targetFrame] request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action targetFrame] webView]);
 
     newWebView = nullptr;
     action = nullptr;
@@ -120,10 +121,11 @@ TEST(WebKit2, DecidePolicyForNavigationActionReloadFromOrigin)
     TestWebKitAPI::Util::run(&decidedPolicy);
 
     EXPECT_EQ(WKNavigationTypeReload, [action navigationType]);
-    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_TRUE([action sourceFrame] != [action targetFrame]);
+    EXPECT_EQ(nil, [action sourceFrame]);
     EXPECT_WK_STREQ(firstURL, [[[action request] URL] absoluteString]);
-    EXPECT_WK_STREQ(firstURL, [[[[action sourceFrame] request] URL] absoluteString]);
-    EXPECT_EQ(nil, [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ(firstURL, [[[[action targetFrame] request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action targetFrame] webView]);
 
     newWebView = nullptr;
     action = nullptr;
@@ -153,10 +155,11 @@ TEST(WebKit2, DecidePolicyForNavigationActionGoBack)
     TestWebKitAPI::Util::run(&decidedPolicy);
 
     EXPECT_EQ(WKNavigationTypeBackForward, [action navigationType]);
-    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_TRUE([action sourceFrame] != [action targetFrame]);
+    EXPECT_EQ(nil, [action sourceFrame]);
     EXPECT_WK_STREQ(firstURL, [[[action request] URL] absoluteString]);
-    EXPECT_WK_STREQ(secondURL, [[[[action sourceFrame] request] URL] absoluteString]);
-    EXPECT_EQ(nil, [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ(secondURL, [[[[action targetFrame] request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action targetFrame] webView]);
 
     newWebView = nullptr;
     action = nullptr;
@@ -190,10 +193,11 @@ TEST(WebKit2, DecidePolicyForNavigationActionGoForward)
     TestWebKitAPI::Util::run(&decidedPolicy);
 
     EXPECT_EQ(WKNavigationTypeBackForward, [action navigationType]);
-    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_TRUE([action sourceFrame] != [action targetFrame]);
+    EXPECT_EQ(nil, [action sourceFrame]);
     EXPECT_WK_STREQ(secondURL, [[[action request] URL] absoluteString]);
-    EXPECT_WK_STREQ(firstURL, [[[[action sourceFrame] request] URL] absoluteString]);
-    EXPECT_EQ(nil, [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ(firstURL, [[[[action targetFrame] request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action targetFrame] webView]);
 
     newWebView = nullptr;
     action = nullptr;