Attachment 362668 Details for Bug 194894 – Fixes the bug

[patch] Fixes the bug

bug-194894-20190221162712.patch (text/plain), 12.00 KB, created by Ryosuke Niwa on 2019-02-21 16:27:12 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-02-21 16:27:12 PST

Size: 12.00 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 241922)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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  Chris Dumez  <cdumez@apple.com>
> 
>         Fix API test crashes after r241855.
>Index: Source/WebKit/UIProcess/WebProcessCache.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebProcessCache.cpp	(revision 241922)
>+++ Source/WebKit/UIProcess/WebProcessCache.cpp	(working copy)
>@@ -51,7 +51,7 @@ bool WebProcessCache::addProcessIfPossib
>     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 S
>     ASSERT(!process->provisionalPageCount());
>     ASSERT(!process->processPool().hasSuspendedPageFor(process));
> 
>-    if (!capacity())
>+    if (!capacity() || m_isDisabled)
>         return false;
> 
>     if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
>Index: Source/WebKit/UIProcess/WebProcessCache.h
>===================================================================
>--- Source/WebKit/UIProcess/WebProcessCache.h	(revision 241922)
>+++ Source/WebKit/UIProcess/WebProcessCache.h	(working copy)
>@@ -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: Source/WebKit/UIProcess/WebProcessPool.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebProcessPool.cpp	(revision 241922)
>+++ Source/WebKit/UIProcess/WebProcessPool.cpp	(working copy)
>@@ -1340,7 +1340,7 @@ void WebProcessPool::handleMemoryPressur
>         m_prewarmedProcess->shutDown();
>     ASSERT(!m_prewarmedProcess);
> 
>-    m_suspendedPages.clear();
>+    clearSuspendedPages();
> }
> 
> #if ENABLE(NETSCAPE_PLUGIN_API)
>@@ -2352,6 +2352,11 @@ bool WebProcessPool::hasSuspendedPageFor
>     }) != m_suspendedPages.end();
> }
> 
>+void WebProcessPool::clearSuspendedPages()
>+{
>+    m_suspendedPages.clear();
>+}
>+
> void WebProcessPool::addMockMediaDevice(const MockMediaDevice& device)
> {
> #if ENABLE(MEDIA_STREAM)
>Index: Source/WebKit/UIProcess/WebProcessPool.h
>===================================================================
>--- Source/WebKit/UIProcess/WebProcessPool.h	(revision 241922)
>+++ Source/WebKit/UIProcess/WebProcessPool.h	(working copy)
>@@ -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: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	(revision 241922)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	(working copy)
>@@ -514,6 +514,11 @@ - (NSUInteger)_maximumSuspendedPageCount
>     return _processPool->maxSuspendedPageCount();
> }
> 
>+- (NSUInteger)_processCacheCapacity
>+{
>+    return _processPool->webProcessCache().capacity();
>+}
>+
> - (size_t)_serviceWorkerProcessCount
> {
> #if ENABLE(SERVICE_WORKER)
>Index: Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	(revision 241922)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	(working copy)
>@@ -103,6 +103,7 @@
> - (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: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(revision 241922)
>+++ Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	(working copy)
>@@ -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(Option
> 
>     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: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 241922)
>+++ Tools/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        Added a test case.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+        (TestWebKitAPI.ProcessSwap.NumberOfCachedProcesses): Added.
>+
> 2019-02-21  David Kilzer  <ddkilzer@apple.com>
> 
>         Leak of CFErrorRef objects (1.92 Kbytes) in com.apple.WebKit.WebContent.Development running WebKit layout tests on iOS Simulator
>Index: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	(revision 241922)
>+++ Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	(working copy)
>@@ -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, NumberOfPrewarmedProce
>     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) {

Flags:

cdumez: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194894: 362587 | 362650 | 362668