Discard cached processes when clearing website data store
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2019 02:58:40 +0000 (02:58 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2019 02:58:40 +0000 (02:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194894

Reviewed by Chris Dumez.

Source/WebKit:

Clear the process cache when clearing the website data store so that there is no way to infer
which site the user had visited by observing for which sites WebContent processes had been cached.

There is one sublty in WebsiteDataStore::removeData that we have to delay the clearing of
the web process cache until the next run loop because SuspendedPageProxy::~SuspendedPageProxy
invokes WebProcessProxy::maybeShutDown in the next run loop. We also have to disable the process
cache during this time as it would otherwise trigger the responsiveness check of WebContent process
can take arbitrarily long time.

* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _processCacheCapacity]): Added for testing.
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/WebProcessCache.cpp:
(WebKit::WebProcessCache::addProcess): Avoid adding web processes to the cache while the suspended
pages are being cleared.
* UIProcess/WebProcessCache.h:
(WebKit::WebProcessCache::disabled const): Added.
(WebKit::WebProcessCache::setDisabled): Added.
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::handleMemoryPressureWarning):
(WebKit::WebProcessPool::clearSuspendedPages): Added.
* UIProcess/WebProcessPool.h:
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::removeData):

Tools:

Added a test case.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
(TestWebKitAPI.ProcessSwap.NumberOfCachedProcesses): Added.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/WebProcessCache.cpp
Source/WebKit/UIProcess/WebProcessCache.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index d5cca5c..ed3a5e3 100644 (file)
@@ -1,3 +1,35 @@
+2019-02-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Discard cached processes when clearing website data store
+        https://bugs.webkit.org/show_bug.cgi?id=194894
+
+        Reviewed by Chris Dumez.
+
+        Clear the process cache when clearing the website data store so that there is no way to infer
+        which site the user had visited by observing for which sites WebContent processes had been cached.
+
+        There is one sublty in WebsiteDataStore::removeData that we have to delay the clearing of
+        the web process cache until the next run loop because SuspendedPageProxy::~SuspendedPageProxy
+        invokes WebProcessProxy::maybeShutDown in the next run loop. We also have to disable the process
+        cache during this time as it would otherwise trigger the responsiveness check of WebContent process
+        can take arbitrarily long time.
+
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _processCacheCapacity]): Added for testing.
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        * UIProcess/WebProcessCache.cpp:
+        (WebKit::WebProcessCache::addProcess): Avoid adding web processes to the cache while the suspended
+        pages are being cleared.
+        * UIProcess/WebProcessCache.h:
+        (WebKit::WebProcessCache::disabled const): Added.
+        (WebKit::WebProcessCache::setDisabled): Added.
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::handleMemoryPressureWarning):
+        (WebKit::WebProcessPool::clearSuspendedPages): Added.
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::removeData):
+
 2019-02-21  Alex Christensen  <achristensen@webkit.org>
 
         Clicking "Go Back" on a safe browsing warning before a WKWebView has loaded any page should request to close the WKWebView
index c345cd9..5184da9 100644 (file)
@@ -514,6 +514,11 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     return _processPool->maxSuspendedPageCount();
 }
 
+- (NSUInteger)_processCacheCapacity
+{
+    return _processPool->webProcessCache().capacity();
+}
+
 - (size_t)_serviceWorkerProcessCount
 {
 #if ENABLE(SERVICE_WORKER)
index 075f719..7dc3da5 100644 (file)
 - (void)_makeNextWebProcessLaunchFailForTesting WK_API_AVAILABLE(macosx(10.14), ios(12.0));
 - (void)_makeNextNetworkProcessLaunchFailForTesting WK_API_AVAILABLE(macosx(10.14), ios(12.0));
 - (NSUInteger)_maximumSuspendedPageCount WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (NSUInteger)_processCacheCapacity WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 // Test only. Returns web processes running web pages (does not include web processes running service workers)
 - (size_t)_webPageContentProcessCount WK_API_AVAILABLE(macosx(10.13.4), ios(11.3));
index bfe20f1..803c6a8 100644 (file)
@@ -51,7 +51,7 @@ bool WebProcessCache::addProcessIfPossible(const String& registrableDomain, Ref<
     ASSERT(!process->provisionalPageCount());
     ASSERT(!process->processPool().hasSuspendedPageFor(process));
 
-    if (!capacity())
+    if (!capacity() || m_isDisabled)
         return false;
 
     if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
@@ -85,7 +85,7 @@ bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcess
     ASSERT(!process->provisionalPageCount());
     ASSERT(!process->processPool().hasSuspendedPageFor(process));
 
-    if (!capacity())
+    if (!capacity() || m_isDisabled)
         return false;
 
     if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
index c52225f..1a6424c 100644 (file)
@@ -49,6 +49,8 @@ public:
 
     unsigned size() const { return m_processesPerRegistrableDomain.size(); }
 
+    void setIsDisabled(bool isDisabled) { m_isDisabled = isDisabled; }
+
     void clear();
     void setApplicationIsActive(bool);
 
@@ -61,6 +63,7 @@ private:
     bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
 
     unsigned m_capacity { 0 };
+    bool m_isDisabled { false };
 
     class CachedProcess {
         WTF_MAKE_FAST_ALLOCATED;
index b2f9afc..f3598b8 100644 (file)
@@ -1340,7 +1340,7 @@ void WebProcessPool::handleMemoryPressureWarning(Critical)
         m_prewarmedProcess->shutDown();
     ASSERT(!m_prewarmedProcess);
 
-    m_suspendedPages.clear();
+    clearSuspendedPages();
 }
 
 #if ENABLE(NETSCAPE_PLUGIN_API)
@@ -2352,6 +2352,11 @@ bool WebProcessPool::hasSuspendedPageFor(WebProcessProxy& process, WebPageProxy*
     }) != m_suspendedPages.end();
 }
 
+void WebProcessPool::clearSuspendedPages()
+{
+    m_suspendedPages.clear();
+}
+
 void WebProcessPool::addMockMediaDevice(const MockMediaDevice& device)
 {
 #if ENABLE(MEDIA_STREAM)
index 37a142d..7c11b7b 100644 (file)
@@ -468,6 +468,7 @@ public:
     bool hasSuspendedPageFor(WebProcessProxy&, WebPageProxy* = nullptr) const;
     unsigned maxSuspendedPageCount() const { return m_maxSuspendedPageCount; }
     RefPtr<WebProcessProxy> findReusableSuspendedPageProcess(const String&, WebPageProxy&);
+    void clearSuspendedPages();
 
     void didReachGoodTimeToPrewarm();
 
index ecf3ace..ede93fd 100644 (file)
@@ -36,6 +36,7 @@
 #include "ShouldGrandfatherStatistics.h"
 #include "StorageAccessStatus.h"
 #include "StorageManager.h"
+#include "WebProcessCache.h"
 #include "WebProcessMessages.h"
 #include "WebProcessPool.h"
 #include "WebResourceLoadStatisticsStore.h"
@@ -778,6 +779,16 @@ void WebsiteDataStore::removeData(OptionSet<WebsiteDataType> dataTypes, WallTime
 
     auto webProcessAccessType = computeWebProcessAccessTypeForDataRemoval(dataTypes, !isPersistent());
     if (webProcessAccessType != ProcessAccessType::None) {
+        for (auto& processPool : processPools()) {
+            processPool->webProcessCache().setIsDisabled(true);
+            processPool->clearSuspendedPages();
+            // FIXME: We need to delay the clearing of the process cache because ~SuspendedPageProxy() calls maybeShutDown asynchronously.
+            RunLoop::main().dispatch([pool = makeRef(*processPool)] {
+                pool->webProcessCache().clear();
+                pool->webProcessCache().setIsDisabled(false);
+            });
+        }
+
         for (auto& process : processes()) {
             switch (webProcessAccessType) {
             case ProcessAccessType::OnlyIfLaunched:
index 1e025d4..61e0731 100644 (file)
@@ -1,3 +1,15 @@
+2019-02-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Discard cached processes when clearing website data store
+        https://bugs.webkit.org/show_bug.cgi?id=194894
+
+        Reviewed by Chris Dumez.
+
+        Added a test case.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        (TestWebKitAPI.ProcessSwap.NumberOfCachedProcesses): Added.
+
 2019-02-21  Alex Christensen  <achristensen@webkit.org>
 
         Clicking "Go Back" on a safe browsing warning before a WKWebView has loaded any page should request to close the WKWebView
index 7ff0f74..43d3d05 100644 (file)
@@ -52,6 +52,7 @@
 #import <wtf/HashSet.h>
 #import <wtf/RetainPtr.h>
 #import <wtf/Vector.h>
+#import <wtf/text/StringConcatenateNumbers.h>
 #import <wtf/text/StringHash.h>
 #import <wtf/text/WTFString.h>
 
@@ -3058,6 +3059,62 @@ TEST(ProcessSwap, NumberOfPrewarmedProcesses)
     EXPECT_TRUE([processPool _hasPrewarmedWebProcess]);
 }
 
+TEST(ProcessSwap, NumberOfCachedProcesses)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    processPoolConfiguration.get().usesWebProcessCache = YES;
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = NO;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    EXPECT_GT([processPool _maximumSuspendedPageCount], 0u);
+    EXPECT_GT([processPool _processCacheCapacity], 0u);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+
+    const unsigned maxSuspendedPageCount = [processPool _maximumSuspendedPageCount];
+    for (unsigned i = 0; i < maxSuspendedPageCount + 2; i++)
+        [handler addMappingFromURLString:makeString("pson://www.domain-", i, ".com") toData:pageCache1Bytes];
+    [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()];
+
+    for (unsigned i = 0; i < maxSuspendedPageCount + 1; i++) {
+        NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:makeString("pson://www.domain-", i, ".com")]];
+        [webView loadRequest:request];
+        TestWebKitAPI::Util::run(&done);
+        done = false;
+
+        EXPECT_EQ(i + 1, [processPool _webProcessCount]);
+        EXPECT_EQ(i + 1, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+        EXPECT_FALSE([processPool _hasPrewarmedWebProcess]);
+    }
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:makeString("pson://www.domain-", maxSuspendedPageCount + 1, ".com")]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_EQ(maxSuspendedPageCount + 2, [processPool _webProcessCount]);
+    EXPECT_EQ(maxSuspendedPageCount + 1, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+    EXPECT_FALSE([processPool _hasPrewarmedWebProcess]);
+
+    static bool readyToContinue = false;
+    [[WKWebsiteDataStore defaultDataStore] removeDataOfTypes:[WKWebsiteDataStore _allWebsiteDataTypesIncludingPrivate] modifiedSince:[NSDate distantPast] completionHandler:^() {
+        readyToContinue = true;
+    }];
+    TestWebKitAPI::Util::run(&readyToContinue);
+
+    EXPECT_EQ(1u, [processPool _webProcessCount]);
+    EXPECT_EQ(1u, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+    EXPECT_FALSE([processPool _hasPrewarmedWebProcess]);
+
+}
+
 static const char* visibilityBytes = R"PSONRESOURCE(
 <script>
 window.addEventListener('pageshow', function(event) {