Diff
Modified: trunk/Source/WebKit/ChangeLog (251047 => 251048)
--- trunk/Source/WebKit/ChangeLog 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/ChangeLog 2019-10-12 16:17:52 UTC (rev 251048)
@@ -1,3 +1,79 @@
+2019-10-12 Chris Dumez <cdu...@apple.com>
+
+ Clearing Website data for a given session should not shut down cached processes for other sessions
+ https://bugs.webkit.org/show_bug.cgi?id=202865
+ <rdar://problem/56202912>
+
+ Reviewed by Antti Koivisto.
+
+ When clearing Website data for a given data store, we now only clear cached processes
+ (either if BackForwardCache or WebProcessCache) if they are associated with this
+ particular data store. It is very wasteful otherwise.
+
+ * UIProcess/ProvisionalPageProxy.cpp:
+ (WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::~SuspendedPageProxy):
+ ProvisionalPageProxy & SuspendedPageProxy's destructors no longer call
+ WebProcessProxy::maybeShutdown() asynchronously. We now call maybeShutdown()
+ synchronously in WebProcessProxy::decrementSuspendedPageCount() and
+ WebProcessProxy::removeProvisionalPageProxy() instead. This makes things a
+ lot more predictable.
+
+ * UIProcess/WebBackForwardCache.cpp:
+ (WebKit::WebBackForwardCache::removeEntriesForSession):
+ Add new removeEntriesForSession() method to clear only back / forward cache
+ entries associated with a particular session.
+
+ (WebKit::WebBackForwardCache::clear):
+ Stop taking a parameter indicating if we allow the processes to enter the
+ process cache. Now that we call maybeShutdown() synchronously when destroying
+ a SuspendedPageProxy, we can simply allow the processes to enter the process
+ cache unconditionally. If the caller does not want this processes in the page
+ cache, they can clear the process cache afterwards.
+
+ * UIProcess/WebBackForwardCache.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+ Now that destroying a SuspendedPageProxy or a ProvisionalPageProxy may shutdown
+ their process synchronously, add a scope here to prevent shutdown of the process
+ for the duration of this scope, since we're about to use it for a navigation and
+ we don't want it to get shutdown, even if there is no longer anything using it.
+
+ (WebKit::WebPageProxy::continueNavigationInNewProcess):
+ Add new assertion and rename parameter for consistency.
+
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::handleMemoryPressureWarning):
+
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::addProvisionalPageProxy):
+ Add new assertion.
+
+ (WebKit::WebProcessProxy::removeProvisionalPageProxy):
+ Call maybeShutDown() if this was the last provisional page.
+
+ (WebKit::WebProcessProxy::maybeShutDown):
+ Drop parameter indicating if we want to allow the process to enter the process
+ cache and allow caching unconditionally.
+
+ (WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
+ Prevent termination if there is a ScopePreventingShutdown.
+
+ (WebKit::WebProcessProxy::decrementSuspendedPageCount):
+ Call maybeShutDown() if this was the last suspended page.
+
+ * UIProcess/WebProcessProxy.h:
+ (WebKit::WebProcessProxy::ScopePreventingShutdown::ScopePreventingShutdown):
+ (WebKit::WebProcessProxy::ScopePreventingShutdown::~ScopePreventingShutdown):
+ (WebKit::WebProcessProxy::makeScopePreventingShutdown):
+ Add new facility to prevent shutdown of a process for the duration of the scope.
+
+ * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+ (WebKit::WebsiteDataStore::removeData):
+ When removing website data, only clear cached processes if they are associated with
+ the current data store.
+
2019-10-12 Chris Fleizach <cfleiz...@apple.com>
AX: Make AXIsolatedTree compile again
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -88,21 +88,16 @@
ProvisionalPageProxy::~ProvisionalPageProxy()
{
- m_process->removeProvisionalPageProxy(*this);
+ if (!m_wasCommitted) {
+ if (&m_process->websiteDataStore() != &m_page.websiteDataStore())
+ m_process->processPool().pageEndUsingWebsiteDataStore(m_page.identifier(), m_process->websiteDataStore());
- if (m_wasCommitted)
- return;
+ m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
+ m_process->send(Messages::WebPage::Close(), m_webPageID);
+ m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
+ }
- if (&m_process->websiteDataStore() != &m_page.websiteDataStore())
- m_process->processPool().pageEndUsingWebsiteDataStore(m_page.identifier(), m_process->websiteDataStore());
-
- m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
- m_process->send(Messages::WebPage::Close(), m_webPageID);
- m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
-
- RunLoop::main().dispatch([process = m_process.copyRef()] {
- process->maybeShutDown();
- });
+ m_process->removeProvisionalPageProxy(*this);
}
void ProvisionalPageProxy::processDidTerminate()
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -116,7 +116,6 @@
SuspendedPageProxy::~SuspendedPageProxy()
{
- m_process->decrementSuspendedPageCount();
allSuspendedPages().remove(this);
if (m_readyToUnsuspendHandler) {
@@ -125,21 +124,16 @@
});
}
- if (m_suspensionState == SuspensionState::Resumed)
- return;
+ if (m_suspensionState != SuspensionState::Resumed) {
+ // If the suspended page was not consumed before getting destroyed, then close the corresponding page
+ // on the WebProcess side.
+ close();
- // If the suspended page was not consumed before getting destroyed, then close the corresponding page
- // on the WebProcess side.
- close();
+ if (m_suspensionState == SuspensionState::Suspending)
+ m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
+ }
- if (m_suspensionState == SuspensionState::Suspending)
- m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
-
- // We call maybeShutDown() asynchronously since the SuspendedPage is currently being removed from the WebProcessPool
- // and we want to avoid re-entering WebProcessPool methods.
- RunLoop::main().dispatch([process = m_process.copyRef()] {
- process->maybeShutDown();
- });
+ m_process->decrementSuspendedPageCount();
}
void SuspendedPageProxy::setBackForwardListItem(WebBackForwardListItem& item)
Modified: trunk/Source/WebKit/UIProcess/WebBackForwardCache.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebBackForwardCache.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebBackForwardCache.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -30,6 +30,7 @@
#include "WebBackForwardListItem.h"
#include "WebPageProxy.h"
#include "WebProcessProxy.h"
+#include "WebsiteDataStore.h"
namespace WebKit {
@@ -83,6 +84,13 @@
});
}
+void WebBackForwardCache::removeEntriesForSession(PAL::SessionID sessionID)
+{
+ removeEntriesMatching([sessionID](auto& item) {
+ return item.suspendedPage()->process().websiteDataStore().sessionID() == sessionID;
+ });
+}
+
void WebBackForwardCache::removeEntriesForPage(WebPageProxy& page)
{
removeEntriesMatching([pageID = page.identifier()](auto& item) {
@@ -102,14 +110,11 @@
}
}
-void WebBackForwardCache::clear(AllowProcessCaching allowProcessCaching)
+void WebBackForwardCache::clear()
{
auto itemsWithCachedPage = WTFMove(m_itemsWithCachedPage);
- for (auto* item : itemsWithCachedPage) {
- auto process = makeRef(item->suspendedPage()->process());
+ for (auto* item : itemsWithCachedPage)
item->setSuspendedPage(nullptr);
- process->maybeShutDown(allowProcessCaching);
- }
}
} // namespace WebKit.
Modified: trunk/Source/WebKit/UIProcess/WebBackForwardCache.h (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebBackForwardCache.h 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebBackForwardCache.h 2019-10-12 16:17:52 UTC (rev 251048)
@@ -25,6 +25,7 @@
#pragma once
+#include <pal/SessionID.h>
#include <wtf/Forward.h>
#include <wtf/ListHashSet.h>
@@ -34,7 +35,6 @@
class WebBackForwardListItem;
class WebPageProxy;
class WebProcessProxy;
-enum class AllowProcessCaching;
class WebBackForwardCache {
WTF_MAKE_FAST_ALLOCATED;
@@ -46,9 +46,10 @@
unsigned capacity() const { return m_capacity; }
unsigned size() const { return m_itemsWithCachedPage.size(); }
- void clear(AllowProcessCaching);
+ void clear();
void removeEntriesForProcess(WebProcessProxy&);
void removeEntriesForPage(WebPageProxy&);
+ void removeEntriesForSession(PAL::SessionID);
void addEntry(WebBackForwardListItem&, std::unique_ptr<SuspendedPageProxy>&&);
void removeEntry(WebBackForwardListItem&);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -2954,6 +2954,9 @@
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "decidePolicyForNavigationAction: keep using process %i for navigation, reason: %{public}s", processIdentifier(), reason.utf8().data());
if (shouldProcessSwap) {
+ // Make sure the process to be used for the navigation does not get shutDown now due to destroying SuspendedPageProxy or ProvisionalPageProxy objects.
+ auto preventNavigationProcessShutdown = processForNavigation->makeScopePreventingShutdown();
+
auto suspendedPage = destinationSuspendedPage ? backForwardCache().takeEntry(*destinationSuspendedPage->backForwardListItem()) : nullptr;
if (suspendedPage && suspendedPage->pageIsClosedOrClosing())
suspendedPage = nullptr;
@@ -3035,10 +3038,11 @@
m_provisionalPage = nullptr;
}
-void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPageProxy, Ref<WebProcessProxy>&& newProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, Optional<WebsitePoliciesData>&& websitePolicies)
+void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPage, Ref<WebProcessProxy>&& newProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, Optional<WebsitePoliciesData>&& websitePolicies)
{
- RELEASE_LOG_IF_ALLOWED(Loading, "continueNavigationInNewProcess: newProcessPID = %i, hasSuspendedPage = %i", newProcess->processIdentifier(), !!suspendedPageProxy);
+ RELEASE_LOG_IF_ALLOWED(Loading, "continueNavigationInNewProcess: newProcessPID = %i, hasSuspendedPage = %i", newProcess->processIdentifier(), !!suspendedPage);
LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString());
+ RELEASE_ASSERT(!newProcess->isInProcessCache());
if (m_provisionalPage) {
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "continueNavigationInNewProcess: There is already a pending provisional load, cancelling it (provisonalNavigationID: %llu, navigationID: %llu)", m_provisionalPage->navigationID(), navigation.navigationID());
@@ -3047,7 +3051,7 @@
m_provisionalPage = nullptr;
}
- m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, newProcess.copyRef(), WTFMove(suspendedPageProxy), navigation.navigationID(), navigation.currentRequestIsRedirect(), navigation.currentRequest(), processSwapRequestedByClient);
+ m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, newProcess.copyRef(), WTFMove(suspendedPage), navigation.navigationID(), navigation.currentRequestIsRedirect(), navigation.currentRequest(), processSwapRequestedByClient);
if (auto* item = navigation.targetItem()) {
LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -1386,8 +1386,9 @@
{
RELEASE_LOG(PerformanceLogging, "%p - WebProcessPool::handleMemoryPressureWarning", this);
-
- m_backForwardCache->clear(AllowProcessCaching::No);
+ // Clear back/forward cache first as processes removed from the back/forward cache will likely
+ // be added to the WebProcess cache.
+ m_backForwardCache->clear();
m_webProcessCache->clear();
if (m_prewarmedProcess)
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -248,6 +248,7 @@
void WebProcessProxy::addProvisionalPageProxy(ProvisionalPageProxy& provisionalPage)
{
+ ASSERT(!m_isInProcessCache);
ASSERT(!m_provisionalPages.contains(&provisionalPage));
m_provisionalPages.add(&provisionalPage);
updateRegistrationWithDataStore();
@@ -258,6 +259,8 @@
ASSERT(m_provisionalPages.contains(&provisionalPage));
m_provisionalPages.remove(&provisionalPage);
updateRegistrationWithDataStore();
+ if (m_provisionalPages.isEmpty())
+ maybeShutDown();
}
void WebProcessProxy::getLaunchOptions(ProcessLauncher::LaunchOptions& launchOptions)
@@ -937,7 +940,7 @@
return true;
}
-void WebProcessProxy::maybeShutDown(AllowProcessCaching allowProcessCaching)
+void WebProcessProxy::maybeShutDown()
{
if (processPool().dummyProcessProxy() == this && m_pageMap.isEmpty()) {
ASSERT(state() == State::Terminated);
@@ -948,7 +951,7 @@
if (state() == State::Terminated || !canTerminateAuxiliaryProcess())
return;
- if (allowProcessCaching == AllowProcessCaching::Yes && canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(*this))
+ if (canBeAddedToWebProcessCache() && processPool().webProcessCache().addProcessIfPossible(*this))
return;
shutDown();
@@ -956,7 +959,7 @@
bool WebProcessProxy::canTerminateAuxiliaryProcess()
{
- if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache)
+ if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache || m_shutdownPreventingScopeCount)
return false;
if (!m_processPool->shouldTerminate(this))
@@ -1483,8 +1486,10 @@
{
ASSERT(m_suspendedPageCount);
--m_suspendedPageCount;
- if (!m_suspendedPageCount)
+ if (!m_suspendedPageCount) {
send(Messages::WebProcess::SetHasSuspendedPageProxy(false), 0);
+ maybeShutDown();
+ }
}
WebProcessPool* WebProcessProxy::processPoolIfExists() const
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-10-12 16:17:52 UTC (rev 251048)
@@ -94,8 +94,6 @@
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<WebCore::FrameIdentifier, RefPtr<WebFrameProxy>> WebFrameProxyMap;
@@ -272,8 +270,28 @@
// 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(AllowProcessCaching = AllowProcessCaching::Yes);
+ class ScopePreventingShutdown {
+ public:
+ explicit ScopePreventingShutdown(WebProcessProxy& process)
+ : m_process(process)
+ {
+ ++(m_process->m_shutdownPreventingScopeCount);
+ }
+
+ ~ScopePreventingShutdown()
+ {
+ ASSERT(m_process->m_shutdownPreventingScopeCount);
+ if (!--(m_process->m_shutdownPreventingScopeCount))
+ m_process->maybeShutDown();
+ }
+
+ private:
+ Ref<WebProcessProxy> m_process;
+ };
+
+ ScopePreventingShutdown makeScopePreventingShutdown() { return ScopePreventingShutdown { *this }; }
+
void didStartProvisionalLoadForMainFrame(const URL&);
// ProcessThrottlerClient
@@ -404,6 +422,8 @@
void updateRegistrationWithDataStore();
+ void maybeShutDown();
+
enum class IsWeak { No, Yes };
template<typename T> class WeakOrStrongPtr {
public:
@@ -482,6 +502,7 @@
#endif
unsigned m_suspendedPageCount { 0 };
+ unsigned m_shutdownPreventingScopeCount { 0 };
bool m_hasCommittedAnyProvisionalLoads { false };
bool m_isPrewarmed;
bool m_hasAudibleWebPage { false };
Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (251047 => 251048)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2019-10-12 09:19:28 UTC (rev 251047)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2019-10-12 16:17:52 UTC (rev 251048)
@@ -726,8 +726,10 @@
auto webProcessAccessType = computeWebProcessAccessTypeForDataRemoval(dataTypes, !isPersistent());
if (webProcessAccessType != ProcessAccessType::None) {
for (auto& processPool : processPools()) {
- processPool->backForwardCache().clear(AllowProcessCaching::No);
- processPool->webProcessCache().clear();
+ // Clear back/forward cache first as processes removed from the back/forward cache will likely
+ // be added to the WebProcess cache.
+ processPool->backForwardCache().removeEntriesForSession(sessionID());
+ processPool->webProcessCache().clearAllProcessesForSession(sessionID());
}
for (auto& process : processes()) {
