Add a setting for keeping around all processes and always reusing them per-origin.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2018 16:39:21 +0000 (16:39 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2018 16:39:21 +0000 (16:39 +0000)
<rdar://problem/39695798> and https://bugs.webkit.org/show_bug.cgi?id=185020

Reviewed by Andy Estes.

Source/WebKit:

* UIProcess/API/APIProcessPoolConfiguration.cpp:
(API::ProcessPoolConfiguration::copy):
* UIProcess/API/APIProcessPoolConfiguration.h:

* UIProcess/API/C/WKContextConfigurationRef.cpp:
(WKContextConfigurationAlwaysKeepAndReuseSwappedProcesses):
(WKContextConfigurationSetAlwaysKeepAndReuseSwappedProcesses):
* UIProcess/API/C/WKContextConfigurationRef.h:

* UIProcess/API/Cocoa/_WKProcessPoolConfiguration.h:
* UIProcess/API/Cocoa/_WKProcessPoolConfiguration.mm:
(-[_WKProcessPoolConfiguration setAlwaysKeepAndReuseSwappedProcesses:]):
(-[_WKProcessPoolConfiguration alwaysKeepAndReuseSwappedProcesses]):

* UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::webProcessDidClose):
(WebKit::SuspendedPageProxy::destroyWebPageInWebProcess):
* UIProcess/SuspendedPageProxy.h:

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendedPageClosed):
(WebKit::WebPageProxy::suspendedPageProcessClosed): Deleted.
* UIProcess/WebPageProxy.h:
(WebKit::WebPageProxy::suspendedPage const):

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::shouldTerminate):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::addProcessToOriginCacheSet):
(WebKit::WebProcessPool::removeProcessFromOriginCacheSet):
(WebKit::WebProcessPool::processForNavigation): If a swap will occur, cache the old process.
(WebKit::WebProcessPool::processForNavigationInternal): Consider re-using a previously cached process.
* UIProcess/WebProcessPool.h:

* WebProcess/WebPage/WebPage.cpp:
(WebKit::m_credentialsMessenger):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

16 files changed:
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.cpp
Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.h
Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp
Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h
Source/WebKit/UIProcess/API/Cocoa/_WKProcessPoolConfiguration.h
Source/WebKit/UIProcess/API/Cocoa/_WKProcessPoolConfiguration.mm
Source/WebKit/UIProcess/SuspendedPageProxy.cpp
Source/WebKit/UIProcess/SuspendedPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index 779c76e..ace7d32 100644 (file)
@@ -1,3 +1,47 @@
+2018-04-26  Brady Eidson  <beidson@apple.com>
+
+        Add a setting for keeping around all processes and always reusing them per-origin.
+        <rdar://problem/39695798> and https://bugs.webkit.org/show_bug.cgi?id=185020
+
+        Reviewed by Andy Estes.
+
+        * UIProcess/API/APIProcessPoolConfiguration.cpp:
+        (API::ProcessPoolConfiguration::copy):
+        * UIProcess/API/APIProcessPoolConfiguration.h:
+
+        * UIProcess/API/C/WKContextConfigurationRef.cpp:
+        (WKContextConfigurationAlwaysKeepAndReuseSwappedProcesses):
+        (WKContextConfigurationSetAlwaysKeepAndReuseSwappedProcesses):
+        * UIProcess/API/C/WKContextConfigurationRef.h:
+
+        * UIProcess/API/Cocoa/_WKProcessPoolConfiguration.h:
+        * UIProcess/API/Cocoa/_WKProcessPoolConfiguration.mm:
+        (-[_WKProcessPoolConfiguration setAlwaysKeepAndReuseSwappedProcesses:]):
+        (-[_WKProcessPoolConfiguration alwaysKeepAndReuseSwappedProcesses]):
+
+        * UIProcess/SuspendedPageProxy.cpp:
+        (WebKit::SuspendedPageProxy::webProcessDidClose):
+        (WebKit::SuspendedPageProxy::destroyWebPageInWebProcess):
+        * UIProcess/SuspendedPageProxy.h:
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::suspendedPageClosed):
+        (WebKit::WebPageProxy::suspendedPageProcessClosed): Deleted.
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::suspendedPage const):
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::shouldTerminate):
+        (WebKit::WebProcessPool::disconnectProcess):
+        (WebKit::WebProcessPool::addProcessToOriginCacheSet):
+        (WebKit::WebProcessPool::removeProcessFromOriginCacheSet):
+        (WebKit::WebProcessPool::processForNavigation): If a swap will occur, cache the old process.
+        (WebKit::WebProcessPool::processForNavigationInternal): Consider re-using a previously cached process.
+        * UIProcess/WebProcessPool.h:
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::m_credentialsMessenger):
+
 2018-04-26  Andy VanWagoner  <thetalecrafter@gmail.com>
 
         [INTL] Implement Intl.PluralRules
index bacffa7..fe1e9c9 100644 (file)
@@ -128,6 +128,7 @@ Ref<ProcessPoolConfiguration> ProcessPoolConfiguration::copy()
 #endif
     copy->m_presentingApplicationPID = this->m_presentingApplicationPID;
     copy->m_processSwapsOnNavigation = this->m_processSwapsOnNavigation;
+    copy->m_alwaysKeepAndReuseSwappedProcesses = this->m_alwaysKeepAndReuseSwappedProcesses;
     copy->m_processSwapsOnWindowOpenWithOpener = this->m_processSwapsOnWindowOpenWithOpener;
 
     return copy;
index 3dc2b18..e3b7d49 100644 (file)
@@ -138,6 +138,9 @@ public:
     bool processSwapsOnNavigation() const { return m_processSwapsOnNavigation; }
     void setProcessSwapsOnNavigation(bool swaps) { m_processSwapsOnNavigation = swaps; }
 
+    bool alwaysKeepAndReuseSwappedProcesses() const { return m_alwaysKeepAndReuseSwappedProcesses; }
+    void setAlwaysKeepAndReuseSwappedProcesses(bool keepAndReuse) { m_alwaysKeepAndReuseSwappedProcesses = keepAndReuse; }
+
     bool processSwapsOnWindowOpenWithOpener() const { return m_processSwapsOnWindowOpenWithOpener; }
     void setProcessSwapsOnWindowOpenWithOpener(bool swaps) { m_processSwapsOnWindowOpenWithOpener = swaps; }
 
@@ -173,6 +176,7 @@ private:
     bool m_shouldCaptureAudioInUIProcess { false };
     ProcessID m_presentingApplicationPID { getCurrentProcessID() };
     bool m_processSwapsOnNavigation { false };
+    bool m_alwaysKeepAndReuseSwappedProcesses { false };
     bool m_processSwapsOnWindowOpenWithOpener { false };
 
 #if PLATFORM(IOS)
index f7896ce..a3d4a58 100644 (file)
@@ -168,6 +168,16 @@ void WKContextConfigurationSetProcessSwapsOnNavigation(WKContextConfigurationRef
     toImpl(configuration)->setProcessSwapsOnNavigation(swaps);
 }
 
+bool WKContextConfigurationAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration)
+{
+    return toImpl(configuration)->alwaysKeepAndReuseSwappedProcesses();
+}
+
+void WKContextConfigurationSetAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration, bool keepAndReuse)
+{
+    toImpl(configuration)->setAlwaysKeepAndReuseSwappedProcesses(keepAndReuse);
+}
+
 bool WKContextConfigurationProcessSwapsOnWindowOpenWithOpener(WKContextConfigurationRef configuration)
 {
     return toImpl(configuration)->processSwapsOnWindowOpenWithOpener();
index 77ae171..a759342 100644 (file)
@@ -71,6 +71,9 @@ WK_EXPORT void WKContextConfigurationSetShouldCaptureAudioInUIProcess(WKContextC
 WK_EXPORT bool WKContextConfigurationProcessSwapsOnNavigation(WKContextConfigurationRef configuration);
 WK_EXPORT void WKContextConfigurationSetProcessSwapsOnNavigation(WKContextConfigurationRef configuration, bool swaps);
 
+WK_EXPORT bool WKContextConfigurationAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration);
+WK_EXPORT void WKContextConfigurationSetAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration, bool keepAndReuse);
+
 WK_EXPORT bool WKContextConfigurationProcessSwapsOnWindowOpenWithOpener(WKContextConfigurationRef configuration);
 WK_EXPORT void WKContextConfigurationSetProcessSwapsOnWindowOpenWithOpener(WKContextConfigurationRef configuration, bool swaps);
 
index 2a13728..60295fe 100644 (file)
@@ -58,6 +58,7 @@ WK_CLASS_AVAILABLE(macosx(10.10), ios(8.0))
 #endif
 @property (nonatomic) pid_t presentingApplicationPID WK_API_AVAILABLE(macosx(10.13), ios(11.0));
 @property (nonatomic) BOOL processSwapsOnNavigation WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+@property (nonatomic) BOOL alwaysKeepAndReuseSwappedProcesses WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 @property (nonatomic) BOOL processSwapsOnWindowOpenWithOpener WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 @end
index cd29da8..547ec89 100644 (file)
     return _processPoolConfiguration->processSwapsOnNavigation();
 }
 
+- (void)setAlwaysKeepAndReuseSwappedProcesses:(BOOL)swaps
+{
+    _processPoolConfiguration->setAlwaysKeepAndReuseSwappedProcesses(swaps);
+}
+
+- (BOOL)alwaysKeepAndReuseSwappedProcesses
+{
+    return _processPoolConfiguration->alwaysKeepAndReuseSwappedProcesses();
+}
+
 - (void)setProcessSwapsOnWindowOpenWithOpener:(BOOL)swaps
 {
     _processPoolConfiguration->setProcessSwapsOnWindowOpenWithOpener(swaps);
index 6e0653f..2664d9f 100644 (file)
@@ -102,10 +102,16 @@ void SuspendedPageProxy::webProcessDidClose(WebProcessProxy& process)
     m_process->processPool().unregisterSuspendedPageProxy(*this);
     m_process = nullptr;
 
-    m_page.suspendedPageProcessClosed(*this);
+    m_page.suspendedPageClosed(*this);
     m_backForwardListItem->setSuspendedPage(nullptr);
 }
 
+void SuspendedPageProxy::destroyWebPageInWebProcess()
+{
+    m_process->send(Messages::WebPage::Close(), m_page.pageID());
+    m_page.suspendedPageClosed(*this);
+}
+
 void SuspendedPageProxy::didFinishLoad()
 {
     ASSERT(m_process);
index 99d9b0c..5c84b22 100644 (file)
@@ -54,6 +54,7 @@ public:
     bool finishedSuspending() const { return m_finishedSuspending; }
 
     void webProcessDidClose(WebProcessProxy&);
+    void destroyWebPageInWebProcess();
 
 #if !LOG_DISABLED
     const char* loggingString() const;
index d72ce2e..37a0685 100644 (file)
@@ -687,7 +687,7 @@ SuspendedPageProxy* WebPageProxy::maybeCreateSuspendedPage(WebProcessProxy& proc
     return m_suspendedPage.get();
 }
 
-void WebPageProxy::suspendedPageProcessClosed(SuspendedPageProxy& page)
+void WebPageProxy::suspendedPageClosed(SuspendedPageProxy& page)
 {
     ASSERT_UNUSED(page, &page == m_suspendedPage.get());
     m_suspendedPage = nullptr;
index 0dd9f0c..14f91ad 100644 (file)
@@ -1298,7 +1298,8 @@ public:
     WebPreferencesStore preferencesStore() const;
 
     SuspendedPageProxy* maybeCreateSuspendedPage(WebProcessProxy&, API::Navigation&);
-    void suspendedPageProcessClosed(SuspendedPageProxy&);
+    SuspendedPageProxy* suspendedPage() const { return m_suspendedPage.get(); }
+    void suspendedPageClosed(SuspendedPageProxy&);
 
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, uint64_t pageID, Ref<API::PageConfiguration>&&);
index 7c000ab..fbbcf2c 100644 (file)
@@ -975,7 +975,7 @@ bool WebProcessPool::shouldTerminate(WebProcessProxy* process)
 {
     ASSERT(m_processes.contains(process));
 
-    if (!m_processTerminationEnabled)
+    if (!m_processTerminationEnabled || m_configuration->alwaysKeepAndReuseSwappedProcesses())
         return false;
 
     return true;
@@ -1040,6 +1040,8 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
         processStoppedUsingGamepads(*process);
 #endif
 
+    removeProcessFromOriginCacheSet(*process);
+
 #if ENABLE(SERVICE_WORKER)
     // FIXME: We should do better than this. For now, we just destroy the ServiceWorker process
     // whenever there is no regular WebContent process remaining.
@@ -2016,8 +2018,53 @@ ServiceWorkerProcessProxy* WebProcessPool::serviceWorkerProcessProxyFromPageID(u
 }
 #endif
 
+void WebProcessPool::addProcessToOriginCacheSet(WebPageProxy& page)
+{
+    auto origin = SecurityOriginData::fromURL({ ParsedURLString, page.pageLoadState().url() });
+    auto result = m_swappedProcesses.add(origin, &page.process());
+    if (!result.isNewEntry)
+        result.iterator->value = &page.process();
+
+    LOG(ProcessSwapping, "(ProcessSwapping) Security origin %s just saved a cached process with pid %i", origin.debugString().utf8().data(), page.process().processIdentifier());
+    if (!result.isNewEntry)
+        LOG(ProcessSwapping, "(ProcessSwapping) Note: It already had one saved");
+}
+
+void WebProcessPool::removeProcessFromOriginCacheSet(WebProcessProxy& process)
+{
+    LOG(ProcessSwapping, "(ProcessSwapping) Removing process with pid %i from the origin cache set", process.processIdentifier());
+
+    // FIXME: This can be very inefficient as the number of remembered origins and processes grows
+    Vector<SecurityOriginData> originsToRemove;
+    for (auto entry : m_swappedProcesses) {
+        if (entry.value == &process)
+            originsToRemove.append(entry.key);
+    }
+
+    for (auto& origin : originsToRemove)
+        m_swappedProcesses.remove(origin);
+}
+
 Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, PolicyAction& action)
 {
+    auto process = processForNavigationInternal(page, navigation, action);
+
+    if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page.process()) {
+        static std::once_flag onceFlag;
+        std::call_once(onceFlag, [] {
+            WTFLogAlways("WARNING: The option to always keep swapped web processes alive is active. This is meant for debugging and testing only.");
+        });
+
+        addProcessToOriginCacheSet(page);
+
+        LOG(ProcessSwapping, "(ProcessSwapping) Navigating from %s to %s, keeping around old process. Now holding on to old processes for %u origins.", page.currentURL().utf8().data(), navigation.currentRequest().url().string().utf8().data(), m_swappedProcesses.size());
+    }
+
+    return process;
+}
+
+Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, PolicyAction& action)
+{
     if (!m_configuration->processSwapsOnNavigation())
         return page.process();
 
@@ -2052,6 +2099,28 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, co
     if (!url.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());
+
+        if (auto* process = m_swappedProcesses.get(origin)) {
+            if (&process->websiteDataStore() == &page.websiteDataStore()) {
+                LOG(ProcessSwapping, "(ProcessSwapping) Reusing a previously cached process with pid %i to continue navigation to URL %s", process->processIdentifier(), targetURL.string().utf8().data());
+
+                // FIXME: Architecturally we do not currently support multiple WebPage's with the same ID in a given WebProcess.
+                // In the case where this WebProcess has a SuspendedPageProxy for this WebPage, we can throw it away to support
+                // WebProcess re-use.
+                // In the future it would be great to refactor-out this limitation.
+                if (auto* suspendedPage = page.suspendedPage()) {
+                    LOG(ProcessSwapping, "(ProcessSwapping) Destroying suspended page for that swap");
+                    suspendedPage->destroyWebPageInWebProcess();
+                }
+
+                return makeRef(*process);
+            }
+        }
+    }
+
     action = PolicyAction::Suspend;
     if (RefPtr<WebProcessProxy> process = tryTakePrewarmedProcess(page.websiteDataStore()))
         return process.releaseNonNull();
index 943eec4..8c6a57c 100644 (file)
@@ -456,6 +456,8 @@ private:
     void platformInitializeWebProcess(WebProcessCreationParameters&);
     void platformInvalidateContext();
 
+    Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, WebCore::PolicyAction&);
+
     RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);
 
     WebProcessProxy& createNewWebProcess(WebsiteDataStore&, WebProcessProxy::IsInPrewarmedPool = WebProcessProxy::IsInPrewarmedPool::No);
@@ -513,6 +515,9 @@ private:
     void resolvePathsForSandboxExtensions();
     void platformResolvePathsForSandboxExtensions();
 
+    void addProcessToOriginCacheSet(WebPageProxy&);
+    void removeProcessFromOriginCacheSet(WebProcessProxy&);
+
     Ref<API::ProcessPoolConfiguration> m_configuration;
 
     IPC::MessageReceiverMap m_messageReceiverMap;
@@ -688,6 +693,7 @@ private:
 #endif
 
     HashMap<WebCore::SecurityOriginData, Vector<SuspendedPageProxy*>> m_suspendedPages;
+    HashMap<WebCore::SecurityOriginData, RefPtr<WebProcessProxy>> m_swappedProcesses;
 };
 
 template<typename T>
index a0da712..cc1c601 100644 (file)
@@ -527,7 +527,7 @@ WebPage::WebPage(uint64_t pageID, WebPageCreationParameters&& parameters)
     m_userAgent = parameters.userAgent;
     
     if (!parameters.itemStates.isEmpty())
-        restoreSessionInternal(parameters.itemStates, WasRestoredByAPIRequest::No, WebBackForwardListProxy::OverwriteExistingItem::No);
+        restoreSessionInternal(parameters.itemStates, WasRestoredByAPIRequest::No, WebBackForwardListProxy::OverwriteExistingItem::Yes);
 
     if (parameters.sessionID.isValid())
         setSessionID(parameters.sessionID);
index 70a4be7..e085e95 100644 (file)
@@ -1,3 +1,12 @@
+2018-04-26  Brady Eidson  <beidson@apple.com>
+
+        Add a setting for keeping around all processes and always reusing them per-origin.
+        <rdar://problem/39695798> and https://bugs.webkit.org/show_bug.cgi?id=185020
+
+        Reviewed by Andy Estes.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-04-26  Andy VanWagoner  <thetalecrafter@gmail.com>
 
         [INTL] Implement Intl.PluralRules
index 3a37ecd..81be14b 100644 (file)
@@ -1228,4 +1228,51 @@ TEST(ProcessSwap, NavigateToDataURL)
     EXPECT_EQ(pid1, pid2);
 }
 
+TEST(ProcessSwap, ProcessReuse)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    [processPoolConfiguration setProcessSwapsOnNavigation:YES];
+    [processPoolConfiguration setAlwaysKeepAndReuseSwappedProcesses: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]);
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host1/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView _webProcessIdentifier];
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host2/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid2 = [webView _webProcessIdentifier];
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host1/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid3 = [webView _webProcessIdentifier];
+
+    // Two process swaps have occurred, but we should only have ever seen 2 pids.
+    EXPECT_EQ(2u, seenPIDs.size());
+    EXPECT_NE(pid1, pid2);
+    EXPECT_NE(pid2, pid3);
+    EXPECT_EQ(pid1, pid3);
+}
+
 #endif // WK_API_ENABLED