REGRESSION: Some USDz from 3rd party websites don't go directly to AR QL

https://bugs.webkit.org/show_bug.cgi?id=193831
<rdar://problem/47399263>

Reviewed by Chris Dumez.

Source/WebKit:

A System Preview (<a rel="ar">) displays in a modal and doesn't trigger
a navigation. If the link was cross origin, it was causing a process swap,
which meant that the response defaulted back to a navigation.

The fix is to not cause a PSON when the navigation is a system preview.

* UIProcess/API/APINavigation.h:
(API::Navigation::shouldForceDownload const): This is now just tracking
the "download" attribute, and not including System Preview.
(API::Navigation::isSystemPreview const): New method to check for a
navigation triggered as a System Preview.
* UIProcess/WebPageProxy.cpp: Move the code from receivedPolicyDecision to
receivedNavigationPolicyDecision, so that downloads and System Previews are
detected before we decide to change process.
(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
(WebKit::WebPageProxy::receivedPolicyDecision):

Tools:

Two new tests that exercise cross-origin and same-origin System
Previews.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 262eb40..6e98674 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,28 @@
+2019-01-25  Dean Jackson  <dino@apple.com>
+
+        REGRESSION: Some USDz from 3rd party websites don't go directly to AR QL
+        https://bugs.webkit.org/show_bug.cgi?id=193831
+        <rdar://problem/47399263>
+
+        Reviewed by Chris Dumez.
+
+        A System Preview (<a rel="ar">) displays in a modal and doesn't trigger
+        a navigation. If the link was cross origin, it was causing a process swap,
+        which meant that the response defaulted back to a navigation.
+
+        The fix is to not cause a PSON when the navigation is a system preview.
+
+        * UIProcess/API/APINavigation.h:
+        (API::Navigation::shouldForceDownload const): This is now just tracking
+        the "download" attribute, and not including System Preview.
+        (API::Navigation::isSystemPreview const): New method to check for a
+        navigation triggered as a System Preview.
+        * UIProcess/WebPageProxy.cpp: Move the code from receivedPolicyDecision to
+        receivedNavigationPolicyDecision, so that downloads and System Previews are
+        detected before we decide to change process.
+        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+        (WebKit::WebPageProxy::receivedPolicyDecision):
+
 2019-01-25  Tim Horton  <timothy_horton@apple.com>
 
         Find-in-page on nyt.com scrolls around without touching the screen when find holes are visible

diff --git a/Source/WebKit/UIProcess/API/APINavigation.h b/Source/WebKit/UIProcess/API/APINavigation.h
index b15030d..8b0054a 100644 (file)
--- a/Source/WebKit/UIProcess/API/APINavigation.h
+++ b/Source/WebKit/UIProcess/API/APINavigation.h
@@ -106,12 +106,14 @@ public:
 
     bool wasUserInitiated() const { return !!m_lastNavigationAction.userGestureTokenIdentifier; }
 
-    bool shouldForceDownload() const
+    bool shouldForceDownload() const { return !m_lastNavigationAction.downloadAttribute.isNull(); }
+
+    bool isSystemPreview() const
     {
 #if USE(SYSTEM_PREVIEW)
-        return !m_lastNavigationAction.downloadAttribute.isNull() || currentRequest().isSystemPreview();
+        return currentRequest().isSystemPreview();
 #else
-        return !m_lastNavigationAction.downloadAttribute.isNull();
+        return false;
 #endif
     }
 

diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
index fd29374..1207c67 100644 (file)
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -2687,6 +2687,9 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
             changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
     }
 
+    if (policyAction == PolicyAction::Use && navigation && (navigation->isSystemPreview() || navigation->shouldForceDownload()))
+        policyAction = PolicyAction::Download;
+
     if (policyAction != PolicyAction::Use || !frame.isMainFrame() || !navigation) {
         receivedPolicyDecision(policyAction, navigation, WTFMove(data), WTFMove(sender));
         return;
@@ -2732,9 +2735,6 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation*
     if (action == PolicyAction::Ignore)
         m_pageLoadState.clearPendingAPIRequestURL(transaction);
 
-    if (navigation && navigation->shouldForceDownload() && action == PolicyAction::Use)
-        action = PolicyAction::Download;
-
     DownloadID downloadID = { };
     if (action == PolicyAction::Download) {
         // Create a download proxy.

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 5a092e2..1dceff2 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,16 @@
+2019-01-25  Dean Jackson  <dino@apple.com>
+
+        REGRESSION: Some USDz from 3rd party websites don't go directly to AR QL
+        https://bugs.webkit.org/show_bug.cgi?id=193831
+        <rdar://problem/47399263>
+
+        Reviewed by Chris Dumez.
+
+        Two new tests that exercise cross-origin and same-origin System
+        Previews.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-01-25  Keith Rollin  <krollin@apple.com>
 
         Update Xcode projects with "Check .xcfilelists" build phase

diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
index 94e969d..2742993 100644 (file)
--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
@@ -1583,6 +1583,100 @@ TEST(ProcessSwap, CrossSiteDownload)
     done = false;
 }
 
+#if USE(SYSTEM_PREVIEW)
+
+static const char* systemPreviewSameOriginTestBytes = R"PSONRESOURCE(
+<body>
+    <a id="testLink" rel="ar" href="pson://www.webkit.org/whatever">
+        <img src="http://www.webkit.org/image">
+    </a>
+</body>
+)PSONRESOURCE";
+
+static const char* systemPreviewCrossOriginTestBytes = R"PSONRESOURCE(
+<body>
+    <a id="testLink" rel="ar" href="pson://www.apple.com/whatever">
+        <img src="http://www.webkit.org/image">
+    </a>
+</body>
+)PSONRESOURCE";
+
+TEST(ProcessSwap, SameOriginSystemPreview)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:systemPreviewSameOriginTestBytes];
+    [handler addMappingFromURLString:@"pson://www.webkit.org/whatever" toData:"Fake USDZ data"];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
+
+    [webViewConfiguration _setSystemPreviewEnabled:YES];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    auto pidAfterFirstLoad = [webView _webProcessIdentifier];
+
+    didStartProvisionalLoad = false;
+    [webView evaluateJavaScript:@"testLink.click()" completionHandler:nil];
+
+    TestWebKitAPI::Util::sleep(0.5);
+
+    // We should still be on webkit.org.
+    EXPECT_EQ(pidAfterFirstLoad, [webView _webProcessIdentifier]);
+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [[webView URL] absoluteString]);
+    EXPECT_FALSE(didStartProvisionalLoad);
+}
+
+TEST(ProcessSwap, CrossOriginSystemPreview)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:systemPreviewCrossOriginTestBytes];
+    [handler addMappingFromURLString:@"pson://www.apple.com/whatever" toData:"Fake USDZ data"];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
+
+    [webViewConfiguration _setSystemPreviewEnabled:YES];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    auto pidAfterFirstLoad = [webView _webProcessIdentifier];
+
+    didStartProvisionalLoad = false;
+    [webView evaluateJavaScript:@"testLink.click()" completionHandler:nil];
+
+    TestWebKitAPI::Util::sleep(0.5);
+
+    // We should still be on webkit.org.
+    EXPECT_EQ(pidAfterFirstLoad, [webView _webProcessIdentifier]);
+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [[webView URL] absoluteString]);
+    EXPECT_FALSE(didStartProvisionalLoad);
+}
+
+#endif
+
 enum class ShouldEnablePSON { No, Yes };
 static void runClientSideRedirectTest(ShouldEnablePSON shouldEnablePSON)
 {