Attachment 360191 Details for Bug 193831 – Patch

[patch] Patch

bug-193831-20190125164512.patch (text/plain), 9.76 KB, created by Dean Jackson on 2019-01-25 16:45:13 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Dean Jackson

Created: 2019-01-25 16:45:13 PST

Size: 9.76 KB

patch

obsolete

>Subversion Revision: 240498
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 63628a1607b286c2b476611a148d305f0172d4a6..a8479fa90091972fa6d794a85399edecad8b3afa 100644
>--- 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 NOBODY (OOPS!).
>+
>+        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  Brent Fulgham  <bfulgham@apple.com>
> 
>         Activate the WebResourceLoadStatisticsStore in the NetworkProcess and deactivate it in the UIProcess.
>diff --git a/Source/WebKit/UIProcess/API/APINavigation.h b/Source/WebKit/UIProcess/API/APINavigation.h
>index b15030d4e71331b8aa8c30a25398b6be4efc632a..8b0054a7fd95f3b03bfd6550999dd85298b72940 100644
>--- 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 bdec81ac9178a269b933f63fa348c2d6f0095e4c..58a0f074e0583f804906be91e93066e59e0f67d1 100644
>--- 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 84922d73d73269c7a037742b7f52a4adff5f856b..95da7754c725b0dc692c8adca207707a9486a6e9 100644
>--- 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 NOBODY (OOPS!).
>+
>+        Two new tests that exercise cross-origin and same-origin System
>+        Previews.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2019-01-25  Brent Fulgham  <bfulgham@apple.com>
> 
>         Activate the WebResourceLoadStatisticsStore in the NetworkProcess and deactivate it in the UIProcess.
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 94e969dd6c922c3befda3b740142b2702c211d39..e1d16edd189b5bce2b11d2d52a7fc3ebfcab4611 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -557,7 +557,7 @@ TEST(ProcessSwap, KillWebContentProcessAfterServerRedirectPolicyDecision)
>     done = false;
> }
> 
>-TEST(ProcessSwap, NoSwappingForeTLDPlus2)
>+TEST(ProcessSwap, NoSwappingForTLDPlus2)
> {
>     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>     processPoolConfiguration.get().processSwapsOnNavigation = YES;
>@@ -1583,6 +1583,96 @@ 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];
>+
>+    [webView evaluateJavaScript:@"testLink.click()" completionHandler:nil];
>+
>+    TestWebKitAPI::Util::sleep(1);
>+
>+    // We should still be on webkit.org.
>+    EXPECT_EQ(pidAfterFirstLoad, [webView _webProcessIdentifier]);
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [[webView URL] absoluteString]);
>+}
>+
>+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];
>+
>+    [webView evaluateJavaScript:@"testLink.click()" completionHandler:nil];
>+
>+    TestWebKitAPI::Util::sleep(1);
>+
>+    // We should still be on webkit.org.
>+    EXPECT_EQ(pidAfterFirstLoad, [webView _webProcessIdentifier]);
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [[webView URL] absoluteString]);
>+}
>+
>+#endif
>+
> enum class ShouldEnablePSON { No, Yes };
> static void runClientSideRedirectTest(ShouldEnablePSON shouldEnablePSON)
> {
>@@ -2921,7 +3011,7 @@ TEST(ProcessSwap, ProcessReuse)
>     EXPECT_EQ(pid1, pid3);
> }
> 
>-TEST(ProcessSwap, ProcessReuseeTLDPlus2)
>+TEST(ProcessSwap, ProcessReUseTLDPlus2)
> {
>     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>     [processPoolConfiguration setProcessSwapsOnNavigation:YES];

Flags:

cdumez: review+
cdumez: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 193831: 360128 | 360137 | 360191