Add an SPI policy action to allow clients to explicitly ask for a new process on...
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Jul 2018 16:56:58 +0000 (16:56 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Jul 2018 16:56:58 +0000 (16:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187789

Reviewed by Andy Estes.

Source/WebKit:

At navigation policy time, when a client says "use/allow", they can now say "use/allow in a new process if possible"

* UIProcess/API/C/WKFramePolicyListener.cpp:
(WKFramePolicyListenerUseInNewProcess):
(WKFramePolicyListenerUseInNewProcessWithPolicies):
* UIProcess/API/C/WKFramePolicyListener.h:

* UIProcess/API/Cocoa/WKNavigationDelegatePrivate.h:

* UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):

* UIProcess/WebFrameListenerProxy.h:
(WebKit::WebFrameListenerProxy::setApplyPolicyInNewProcessIfPossible):
(WebKit::WebFrameListenerProxy::applyPolicyInNewProcessIfPossible const):

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

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation):
(WebKit::WebProcessPool::processForNavigationInternal):
* UIProcess/WebProcessPool.h:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
(-[PSONNavigationDelegate init]):
(-[PSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/C/WKFramePolicyListener.cpp
Source/WebKit/UIProcess/API/C/WKFramePolicyListener.h
Source/WebKit/UIProcess/API/Cocoa/WKNavigationDelegatePrivate.h
Source/WebKit/UIProcess/Cocoa/NavigationState.mm
Source/WebKit/UIProcess/WebFrameListenerProxy.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index fb0aa64..e54ed7f 100644 (file)
@@ -1,3 +1,34 @@
+2018-07-19  Brady Eidson  <beidson@apple.com>
+
+        Add an SPI policy action to allow clients to explicitly ask for a new process on a navigation.
+        https://bugs.webkit.org/show_bug.cgi?id=187789
+
+        Reviewed by Andy Estes.
+
+        At navigation policy time, when a client says "use/allow", they can now say "use/allow in a new process if possible"
+
+        * UIProcess/API/C/WKFramePolicyListener.cpp:
+        (WKFramePolicyListenerUseInNewProcess):
+        (WKFramePolicyListenerUseInNewProcessWithPolicies):
+        * UIProcess/API/C/WKFramePolicyListener.h:
+
+        * UIProcess/API/Cocoa/WKNavigationDelegatePrivate.h:
+
+        * UIProcess/Cocoa/NavigationState.mm:
+        (WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
+
+        * UIProcess/WebFrameListenerProxy.h:
+        (WebKit::WebFrameListenerProxy::setApplyPolicyInNewProcessIfPossible):
+        (WebKit::WebFrameListenerProxy::applyPolicyInNewProcessIfPossible const):
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::receivedPolicyDecision):
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::processForNavigation):
+        (WebKit::WebProcessPool::processForNavigationInternal):
+        * UIProcess/WebProcessPool.h:
+
 2018-07-19  Youenn Fablet  <youenn@apple.com>
 
         Ensure experimentalPlugInSandboxProfilesEnabled is set on PluginProcess
index 41de762..ff97e98 100644 (file)
@@ -45,6 +45,12 @@ void WKFramePolicyListenerUse(WKFramePolicyListenerRef policyListenerRef)
     toImpl(policyListenerRef)->use(std::nullopt);
 }
 
+void WKFramePolicyListenerUseInNewProcess(WKFramePolicyListenerRef policyListenerRef)
+{
+    toImpl(policyListenerRef)->setApplyPolicyInNewProcessIfPossible(true);
+    toImpl(policyListenerRef)->use(std::nullopt);
+}
+
 void WKFramePolicyListenerUseWithPolicies(WKFramePolicyListenerRef policyListenerRef, WKWebsitePoliciesRef websitePolicies)
 {
     auto data = toImpl(websitePolicies)->data();
@@ -60,6 +66,12 @@ void WKFramePolicyListenerUseWithPolicies(WKFramePolicyListenerRef policyListene
     toImpl(policyListenerRef)->use(WTFMove(data));
 }
 
+void WKFramePolicyListenerUseInNewProcessWithPolicies(WKFramePolicyListenerRef policyListenerRef, WKWebsitePoliciesRef websitePolicies)
+{
+    toImpl(policyListenerRef)->setApplyPolicyInNewProcessIfPossible(true);
+    WKFramePolicyListenerUseWithPolicies(policyListenerRef, websitePolicies);
+}
+
 void WKFramePolicyListenerDownload(WKFramePolicyListenerRef policyListenerRef)
 {
     toImpl(policyListenerRef)->download();
index 9d2ccfc..2885c7e 100644 (file)
@@ -34,9 +34,11 @@ extern "C" {
 WK_EXPORT WKTypeID WKFramePolicyListenerGetTypeID();
 
 WK_EXPORT void WKFramePolicyListenerUse(WKFramePolicyListenerRef);
+WK_EXPORT void WKFramePolicyListenerUseInNewProcess(WKFramePolicyListenerRef);
 WK_EXPORT void WKFramePolicyListenerDownload(WKFramePolicyListenerRef);
 WK_EXPORT void WKFramePolicyListenerIgnore(WKFramePolicyListenerRef);
 WK_EXPORT void WKFramePolicyListenerUseWithPolicies(WKFramePolicyListenerRef, WKWebsitePoliciesRef);
+WK_EXPORT void WKFramePolicyListenerUseInNewProcessWithPolicies(WKFramePolicyListenerRef, WKWebsitePoliciesRef);
 
 #ifdef __cplusplus
 }
index 465709f..1e2297b 100644 (file)
@@ -56,7 +56,8 @@ typedef NS_ENUM(NSInteger, _WKProcessTerminationReason) {
 } WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 static const WKNavigationActionPolicy _WKNavigationActionPolicyDownload = (WKNavigationActionPolicy)(WKNavigationActionPolicyAllow + 1);
-static const WKNavigationActionPolicy WK_API_AVAILABLE(macosx(10.11), ios(9.0)) _WKNavigationActionPolicyAllowWithoutTryingAppLink = (WKNavigationActionPolicy)(WKNavigationActionPolicyAllow + 2);
+static const WKNavigationActionPolicy WK_API_AVAILABLE(macosx(10.11), ios(9.0)) _WKNavigationActionPolicyAllowWithoutTryingAppLink = (WKNavigationActionPolicy)(_WKNavigationActionPolicyDownload + 1);
+static const WKNavigationActionPolicy WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)) _WKNavigationActionPolicyAllowInNewProcess = (WKNavigationActionPolicy)(_WKNavigationActionPolicyAllowWithoutTryingAppLink + 1);
 
 static const WKNavigationResponsePolicy _WKNavigationResponsePolicyBecomeDownload = (WKNavigationResponsePolicy)(WKNavigationResponsePolicyAllow + 1);
 
index 6f07ee1..2387d62 100644 (file)
@@ -545,7 +545,13 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP
 
         switch (actionPolicy) {
         case WKNavigationActionPolicyAllow:
-            tryAppLink(WTFMove(navigationAction), mainFrameURLString, [localListener = WTFMove(localListener), data = WTFMove(data)](bool followedLinkToApp) mutable {
+// FIXME: Once we have a new enough compiler everywhere we don't need to ignore -Wswitch.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wswitch"
+        case _WKNavigationActionPolicyAllowInNewProcess:
+#pragma clang diagnostic pop
+            tryAppLink(WTFMove(navigationAction), mainFrameURLString, [actionPolicy, localListener = WTFMove(localListener), data = WTFMove(data)](bool followedLinkToApp) mutable {
+                localListener->setApplyPolicyInNewProcessIfPossible(actionPolicy == _WKNavigationActionPolicyAllowInNewProcess);
                 if (followedLinkToApp) {
                     localListener->ignore();
                     return;
index 3dde8f9..f0bac5f 100644 (file)
@@ -47,6 +47,10 @@ public:
     void setNavigation(Ref<API::Navigation>&& navigation) { m_navigation = WTFMove(navigation); }
 
     void changeWebsiteDataStore(WebsiteDataStore&);
+
+    void setApplyPolicyInNewProcessIfPossible(bool applyPolicyInNewProcessIfPossible) { m_applyPolicyInNewProcessIfPossible = applyPolicyInNewProcessIfPossible; }
+    bool applyPolicyInNewProcessIfPossible() const { return m_applyPolicyInNewProcessIfPossible; }
+
     bool isMainFrame() const;
 
 protected:
@@ -58,6 +62,7 @@ private:
     RefPtr<WebFrameProxy> m_frame;
     uint64_t m_listenerID;
     RefPtr<API::Navigation> m_navigation;
+    bool m_applyPolicyInNewProcessIfPossible { false };
 };
 
 } // namespace WebKit
index 48cc400..e00f6ed 100644 (file)
@@ -2438,7 +2438,7 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
         ASSERT(activePolicyListener->listenerID() == listenerID);
 
         if (action == PolicyAction::Use && navigation && frame.isMainFrame()) {
-            auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, action);
+            auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, activePolicyListener->applyPolicyInNewProcessIfPossible(), action);
 
             if (proposedProcess.ptr() != &process()) {
                 LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), proposedProcess->processIdentifier(), navigation->navigationID(), navigation->loggingString());
index 02c6267..b9b1317 100644 (file)
@@ -2106,9 +2106,9 @@ void WebProcessPool::removeProcessFromOriginCacheSet(WebProcessProxy& process)
         m_swappedProcesses.remove(origin);
 }
 
-Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, PolicyAction& action)
+Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, bool shouldProcessSwapIfPossible, PolicyAction& action)
 {
-    auto process = processForNavigationInternal(page, navigation, action);
+    auto process = processForNavigationInternal(page, navigation, shouldProcessSwapIfPossible, action);
 
     if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page.process()) {
         static std::once_flag onceFlag;
@@ -2124,9 +2124,9 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, co
     return process;
 }
 
-Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, PolicyAction& action)
+Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, bool shouldProcessSwapIfPossible, PolicyAction& action)
 {
-    if (!m_configuration->processSwapsOnNavigation())
+    if (!m_configuration->processSwapsOnNavigation() && !shouldProcessSwapIfPossible)
         return page.process();
 
     if (page.inspectorFrontendCount() > 0)
@@ -2169,14 +2169,16 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
         }
     }
 
-    if (navigation.treatAsSameOriginNavigation())
-        return page.process();
-
     auto targetURL = navigation.currentRequest().url();
-    auto url = URL { ParsedURLString, page.pageLoadState().url() };
-    if (!url.isValid() || !targetURL.isValid() || url.isEmpty() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL))
-        return page.process();
+    if (!shouldProcessSwapIfPossible) {
+        if (navigation.treatAsSameOriginNavigation())
+            return page.process();
 
+        auto url = URL { ParsedURLString, page.pageLoadState().url() };
+        if (!url.isValid() || !targetURL.isValid() || url.isEmpty() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL))
+            return page.process();
+    }
+    
     if (m_configuration->alwaysKeepAndReuseSwappedProcesses()) {
         auto origin = SecurityOriginData::fromURL(targetURL);
         LOG(ProcessSwapping, "(ProcessSwapping) Considering re-use of a previously cached process to URL %s", origin.debugString().utf8().data());
index 7cec709..6676f73 100644 (file)
@@ -452,7 +452,7 @@ public:
     BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
 #endif
 
-    Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, WebCore::PolicyAction&);
+    Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, bool shouldProcessSwapIfPossible, WebCore::PolicyAction&);
     void registerSuspendedPageProxy(SuspendedPageProxy&);
     void unregisterSuspendedPageProxy(SuspendedPageProxy&);
     void didReachGoodTimeToPrewarm();
@@ -470,7 +470,7 @@ private:
     void platformInitializeWebProcess(WebProcessCreationParameters&);
     void platformInvalidateContext();
 
-    Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, WebCore::PolicyAction&);
+    Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, bool shouldProcessSwapIfPossible, WebCore::PolicyAction&);
 
     RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);
 
index 9dcf871..2aaf024 100644 (file)
@@ -1,3 +1,14 @@
+2018-07-19  Brady Eidson  <beidson@apple.com>
+
+        Add an SPI policy action to allow clients to explicitly ask for a new process on a navigation.
+        https://bugs.webkit.org/show_bug.cgi?id=187789
+
+        Reviewed by Andy Estes.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        (-[PSONNavigationDelegate init]):
+        (-[PSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
+
 2018-07-19  Thibault Saunier  <tsaunier@igalia.com>
 
         [Flatpak] Let flatpak process handle SIGINT
index dd59052..ef4f85c 100644 (file)
@@ -28,7 +28,7 @@
 #import "PlatformUtilities.h"
 #import "Test.h"
 #import <WebKit/WKInspector.h>
-#import <WebKit/WKNavigationDelegate.h>
+#import <WebKit/WKNavigationDelegatePrivate.h>
 #import <WebKit/WKNavigationPrivate.h>
 #import <WebKit/WKPreferencesPrivate.h>
 #import <WebKit/WKProcessPoolPrivate.h>
@@ -85,11 +85,20 @@ static HashSet<pid_t> seenPIDs;
 }
 @end
 
-@interface PSONNavigationDelegate : NSObject <WKNavigationDelegate>
+@interface PSONNavigationDelegate : NSObject <WKNavigationDelegate> {
+    @public WKNavigationActionPolicy navigationActionPolicyToUse;
+}
 @end
 
 @implementation PSONNavigationDelegate
 
+- (instancetype) init
+{
+    self = [super init];
+    navigationActionPolicyToUse = WKNavigationActionPolicyAllow;
+    return self;
+}
+
 - (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(WKNavigation *)navigation withError:(NSError *)error
 {
     seenPIDs.add([webView _webProcessIdentifier]);
@@ -106,7 +115,7 @@ static HashSet<pid_t> seenPIDs;
 {
     ++numberOfDecidePolicyCalls;
     seenPIDs.add([webView _webProcessIdentifier]);
-    decisionHandler(WKNavigationActionPolicyAllow);
+    decisionHandler(navigationActionPolicyToUse);
 }
 
 - (void)webView:(WKWebView *)webView didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
@@ -1434,4 +1443,42 @@ TEST(ProcessSwap, NavigateToDataURLThenBack)
     EXPECT_EQ(pid2, pid3);
 }
 
+TEST(ProcessSwap, APIControlledProcessSwapping)
+{
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    RetainPtr<PSONScheme> handler = adoptNS([[PSONScheme alloc] initWithBytes:"Hello World!"]);
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    numberOfDecidePolicyCalls = 0;
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/1"]]];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    auto pid1 = [webView _webProcessIdentifier];
+
+    // Navigating from the above URL to this URL normally should not process swap.
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/2"]]];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    auto pid2 = [webView _webProcessIdentifier];
+
+    EXPECT_EQ(1u, seenPIDs.size());
+    EXPECT_EQ(pid1, pid2);
+
+    // Navigating from the above URL to this URL normally should not process swap,
+    // but we'll explicitly ask for a swap.
+    navigationDelegate->navigationActionPolicyToUse = _WKNavigationActionPolicyAllowInNewProcess;
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/3"]]];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    auto pid3 = [webView _webProcessIdentifier];
+    
+    EXPECT_EQ(3, numberOfDecidePolicyCalls);
+    EXPECT_EQ(2u, seenPIDs.size());
+    EXPECT_NE(pid1, pid3);
+}
+
 #endif // WK_API_ENABLED