[PSON] Add mechanism to clear suspended pages while bypassing the Web…

…Process cache https://bugs.webkit.org/show_bug.cgi?id=195089 Reviewed by Geoffrey Garen. Add a convenient mechanism to clear suspended pages while bypassing the WebProcess cache since we need to do this on memory pressure and when clearing website data. * UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::receivedNavigationPolicyDecision): * UIProcess/WebProcessCache.cpp: (WebKit::WebProcessCache::addProcessIfPossible): (WebKit::WebProcessCache::addProcess): (WebKit::WebProcessCache::takeProcess): (WebKit::WebProcessCache::CachedProcess::~CachedProcess): * UIProcess/WebProcessCache.h: (WebKit::WebProcessCache::setIsDisabled): Deleted. * UIProcess/WebProcessPool.cpp: (WebKit::WebProcessPool::handleMemoryPressureWarning): (WebKit::WebProcessPool::hasSuspendedPageFor const): (WebKit::WebProcessPool::clearSuspendedPages): * UIProcess/WebProcessPool.h: * UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::maybeShutDown): (WebKit::WebProcessProxy::canTerminateAuxiliaryProcess): * UIProcess/WebProcessProxy.h: (WebKit::WebProcessProxy::suspendedPageCount const): * UIProcess/WebsiteData/WebsiteDataStore.cpp: (WebKit::WebsiteDataStore::removeData): Canonical link: https://commits.webkit.org/209444@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242124 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Feb 27, 2019 · 167c61e4f16a122c1566ba5c6dde72bdb51934e5 · 167c61e
1 parent ff3b188
commit 167c61e4f16a122c1566ba5c6dde72bdb51934e5
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 29 deletions.
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,35 @@
+2019-02-26  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Add mechanism to clear suspended pages while bypassing the WebProcess cache
+        https://bugs.webkit.org/show_bug.cgi?id=195089
+
+        Reviewed by Geoffrey Garen.
+
+        Add a convenient mechanism to clear suspended pages while bypassing the WebProcess
+        cache since we need to do this on memory pressure and when clearing website data.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+        * UIProcess/WebProcessCache.cpp:
+        (WebKit::WebProcessCache::addProcessIfPossible):
+        (WebKit::WebProcessCache::addProcess):
+        (WebKit::WebProcessCache::takeProcess):
+        (WebKit::WebProcessCache::CachedProcess::~CachedProcess):
+        * UIProcess/WebProcessCache.h:
+        (WebKit::WebProcessCache::setIsDisabled): Deleted.
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::handleMemoryPressureWarning):
+        (WebKit::WebProcessPool::hasSuspendedPageFor const):
+        (WebKit::WebProcessPool::clearSuspendedPages):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::maybeShutDown):
+        (WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
+        * UIProcess/WebProcessProxy.h:
+        (WebKit::WebProcessProxy::suspendedPageCount const):
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::removeData):
+
 2019-02-26  Alex Christensen  <achristensen@webkit.org>
 
         Move ephemeral local storage from WebProcess to UIProcess

diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -2790,7 +2790,7 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
         // FIXME: Architecturally we do not currently support multiple WebPage's with the same ID in a given WebProcess.
         // In the case where the destination WebProcess has a SuspendedPageProxy for this WebPage, we should have thrown
         // it away to support WebProcess re-use.
-        ASSERT(destinationSuspendedPage || !process().processPool().hasSuspendedPageFor(processForNavigation, this));
+        ASSERT(destinationSuspendedPage || !process().processPool().hasSuspendedPageFor(processForNavigation, *this));
 
         auto suspendedPage = destinationSuspendedPage ? process().processPool().takeSuspendedPage(*destinationSuspendedPage) : nullptr;
         if (suspendedPage && suspendedPage->failedToSuspend())

diff --git a/Source/WebKit/UIProcess/WebProcessCache.cpp b/Source/WebKit/UIProcess/WebProcessCache.cpp
@@ -49,9 +49,9 @@ bool WebProcessCache::addProcessIfPossible(const String& registrableDomain, Ref<
     ASSERT(!registrableDomain.isEmpty());
     ASSERT(!process->pageCount());
     ASSERT(!process->provisionalPageCount());
-    ASSERT(!process->processPool().hasSuspendedPageFor(process));
+    ASSERT(!process->suspendedPageCount());
 
-    if (!capacity() || m_isDisabled)
+    if (!capacity())
         return false;
 
     if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
@@ -83,9 +83,9 @@ bool WebProcessCache::addProcess(const String& registrableDomain, Ref<WebProcess
 {
     ASSERT(!process->pageCount());
     ASSERT(!process->provisionalPageCount());
-    ASSERT(!process->processPool().hasSuspendedPageFor(process));
+    ASSERT(!process->suspendedPageCount());
 
-    if (!capacity() || m_isDisabled)
+    if (!capacity())
         return false;
 
     if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
@@ -126,7 +126,7 @@ RefPtr<WebProcessProxy> WebProcessCache::takeProcess(const String& registrableDo
 
     ASSERT(!process->pageCount());
     ASSERT(!process->provisionalPageCount());
-    ASSERT(!process->processPool().hasSuspendedPageFor(process));
+    ASSERT(!process->suspendedPageCount());
 
     return WTFMove(process);
 }
@@ -199,7 +199,7 @@ WebProcessCache::CachedProcess::~CachedProcess()
 
     ASSERT(!m_process->pageCount());
     ASSERT(!m_process->provisionalPageCount());
-    ASSERT(!m_process->processPool().hasSuspendedPageFor(*m_process));
+    ASSERT(!m_process->suspendedPageCount());
 
     m_process->setIsInProcessCache(false);
     m_process->shutDown();

diff --git a/Source/WebKit/UIProcess/WebProcessCache.h b/Source/WebKit/UIProcess/WebProcessCache.h
@@ -49,8 +49,6 @@ class WebProcessCache {
 
     unsigned size() const { return m_processesPerRegistrableDomain.size(); }
 
-    void setIsDisabled(bool isDisabled) { m_isDisabled = isDisabled; }
-
     void clear();
     void setApplicationIsActive(bool);
 
@@ -63,7 +61,6 @@ class WebProcessCache {
     bool addProcess(const String& registrableDomain, Ref<WebProcessProxy>&&);
 
     unsigned m_capacity { 0 };
-    bool m_isDisabled { false };
 
     class CachedProcess {
         WTF_MAKE_FAST_ALLOCATED;

diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
@@ -1334,13 +1334,13 @@ void WebProcessPool::handleMemoryPressureWarning(Critical)
 {
     RELEASE_LOG(PerformanceLogging, "%p - WebProcessPool::handleMemoryPressureWarning", this);
 
+
+    clearSuspendedPages(AllowProcessCaching::No);
     m_webProcessCache->clear();
 
     if (m_prewarmedProcess)
         m_prewarmedProcess->shutDown();
     ASSERT(!m_prewarmedProcess);
-
-    clearSuspendedPages();
 }
 
 #if ENABLE(NETSCAPE_PLUGIN_API)
@@ -2345,16 +2345,23 @@ void WebProcessPool::removeSuspendedPage(SuspendedPageProxy& suspendedPage)
         m_suspendedPages.remove(it);
 }
 
-bool WebProcessPool::hasSuspendedPageFor(WebProcessProxy& process, WebPageProxy* page) const
+bool WebProcessPool::hasSuspendedPageFor(WebProcessProxy& process, WebPageProxy& page) const
 {
-    return m_suspendedPages.findIf([&process, page](auto& suspendedPage) {
-        return &suspendedPage->process() == &process && (!page || &suspendedPage->page() == page);
+    return m_suspendedPages.findIf([&process, &page](auto& suspendedPage) {
+        return &suspendedPage->process() == &process && &suspendedPage->page() == &page;
     }) != m_suspendedPages.end();
 }
 
-void WebProcessPool::clearSuspendedPages()
+void WebProcessPool::clearSuspendedPages(AllowProcessCaching allowProcessCaching)
 {
+    HashSet<RefPtr<WebProcessProxy>> processes;
+    for (auto& suspendedPage : m_suspendedPages)
+        processes.add(&suspendedPage->process());
+
     m_suspendedPages.clear();
+
+    for (auto& process : processes)
+        process->maybeShutDown(allowProcessCaching);
 }
 
 void WebProcessPool::addMockMediaDevice(const MockMediaDevice& device)

diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
@@ -465,10 +465,11 @@ class WebProcessPool final : public API::ObjectImpl<API::Object::Type::ProcessPo
     void closeFailedSuspendedPagesForPage(WebPageProxy&);
     std::unique_ptr<SuspendedPageProxy> takeSuspendedPage(SuspendedPageProxy&);
     void removeSuspendedPage(SuspendedPageProxy&);
-    bool hasSuspendedPageFor(WebProcessProxy&, WebPageProxy* = nullptr) const;
+    bool hasSuspendedPageFor(WebProcessProxy&, WebPageProxy&) const;
     unsigned maxSuspendedPageCount() const { return m_maxSuspendedPageCount; }
     RefPtr<WebProcessProxy> findReusableSuspendedPageProcess(const String&, WebPageProxy&);
-    void clearSuspendedPages();
+
+    void clearSuspendedPages(AllowProcessCaching);
 
     void didReachGoodTimeToPrewarm();
 

diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp
@@ -842,20 +842,20 @@ bool WebProcessProxy::canBeAddedToWebProcessCache() const
     return true;
 }
 
-void WebProcessProxy::maybeShutDown()
+void WebProcessProxy::maybeShutDown(AllowProcessCaching allowProcessCaching)
 {
     if (state() == State::Terminated || !canTerminateAuxiliaryProcess())
         return;
 
-    if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(registrableDomain(), *this))
+    if (allowProcessCaching == AllowProcessCaching::Yes && canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(registrableDomain(), *this))
         return;
 
     shutDown();
 }
 
 bool WebProcessProxy::canTerminateAuxiliaryProcess()
 {
-    if (!m_pageMap.isEmpty() || m_processPool->hasSuspendedPageFor(*this) || !m_provisionalPages.isEmpty() || m_isInProcessCache)
+    if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache)
         return false;
 
     if (!m_processPool->shouldTerminate(this))

diff --git a/Source/WebKit/UIProcess/WebProcessProxy.h b/Source/WebKit/UIProcess/WebProcessProxy.h
@@ -90,6 +90,8 @@ typedef RefCounter<BackgroundWebProcessCounterType> BackgroundWebProcessCounter;
 typedef BackgroundWebProcessCounter::Token BackgroundWebProcessToken;
 #endif
 
+enum class AllowProcessCaching { No, Yes };
+
 class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer::Client, public ThreadSafeRefCounted<WebProcessProxy>, public CanMakeWeakPtr<WebProcessProxy>, private ProcessThrottlerClient {
 public:
     typedef HashMap<uint64_t, RefPtr<WebFrameProxy>> WebFrameProxyMap;
@@ -106,6 +108,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer
 
     WebConnection* webConnection() const { return m_webConnection.get(); }
 
+    unsigned suspendedPageCount() const { return m_suspendedPageCount; }
     void incrementSuspendedPageCount();
     void decrementSuspendedPageCount();
 
@@ -255,7 +258,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer
     // Called when the web process has crashed or we know that it will terminate soon.
     // Will potentially cause the WebProcessProxy object to be freed.
     void shutDown();
-    void maybeShutDown();
+    void maybeShutDown(AllowProcessCaching = AllowProcessCaching::Yes);
 
     void didStartProvisionalLoadForMainFrame(const URL&);
 

diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
@@ -758,13 +758,8 @@ 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);
-            });
+            processPool->clearSuspendedPages(AllowProcessCaching::No);
+            processPool->webProcessCache().clear();
         }
 
         for (auto& process : processes()) {