Changeset 241928 in webkit

Timestamp:

Feb 21, 2019 6:58:40 PM (5 years ago)

Author:

rniwa@webkit.org

Message:

Discard cached processes when clearing website data store
​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.

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessCache.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebProcessCache.h (modified) (2 diffs)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebProcessPool.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm

@@ -515 +515 @@
 }
 
+- (NSUInteger)_processCacheCapacity
+{
+    return _processPool->webProcessCache().capacity();
+}
+
 - (size_t)_serviceWorkerProcessCount
 {

trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h

@@ -104 +104 @@
 - (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)

trunk/Source/WebKit/UIProcess/WebProcessCache.cpp

@@ -52 +52 @@
     ASSERT(!process->processPool().hasSuspendedPageFor(process));
 
-    if (!capacity())
+    if (!capacity() || m_isDisabled)
         return false;
 
@@ -86 +86 @@
     ASSERT(!process->processPool().hasSuspendedPageFor(process));
 
-    if (!capacity())
+    if (!capacity() || m_isDisabled)
         return false;
 

trunk/Source/WebKit/UIProcess/WebProcessCache.h

@@ -50 +50 @@
     unsigned size() const { return m_processesPerRegistrableDomain.size(); }
 
+    void setIsDisabled(bool isDisabled) { m_isDisabled = isDisabled; }
+
     void clear();
     void setApplicationIsActive(bool);
@@ -62 +64 @@
 
     unsigned m_capacity { 0 };
+    bool m_isDisabled { false };
 
     class CachedProcess {

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1341 +1341 @@
     ASSERT(!m_prewarmedProcess);
 
-    m_suspendedPages.clear();
+    clearSuspendedPages();
 }
 
@@ -2353 +2353 @@
 }
 
+void WebProcessPool::clearSuspendedPages()
+{
+    m_suspendedPages.clear();
+}
+
 void WebProcessPool::addMockMediaDevice(const MockMediaDevice& device)
 {

trunk/Source/WebKit/UIProcess/WebProcessPool.h

@@ -469 +469 @@
     unsigned maxSuspendedPageCount() const { return m_maxSuspendedPageCount; }
     RefPtr<WebProcessProxy> findReusableSuspendedPageProcess(const String&, WebPageProxy&);
+    void clearSuspendedPages();
 
     void didReachGoodTimeToPrewarm();

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -37 +37 @@
 #include "StorageAccessStatus.h"
 #include "StorageManager.h"
+#include "WebProcessCache.h"
 #include "WebProcessMessages.h"
 #include "WebProcessPool.h"
@@ -779 +780 @@
     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) {

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -53 +53 @@
 #import <wtf/RetainPtr.h>
 #import <wtf/Vector.h>
+#import <wtf/text/StringConcatenateNumbers.h>
 #import <wtf/text/StringHash.h>
 #import <wtf/text/WTFString.h>
@@ -3057 +3058 @@
     EXPECT_EQ(2u, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
     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]);
+
 }