Diff
Modified: trunk/Source/WebKit/ChangeLog (242370 => 242371)
--- trunk/Source/WebKit/ChangeLog 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/ChangeLog 2019-03-04 20:26:06 UTC (rev 242371)
@@ -1,5 +1,65 @@
2019-03-04 Chris Dumez <[email protected]>
+ Do not share WebProcesses between private and regular sessions
+ https://bugs.webkit.org/show_bug.cgi?id=195189
+ <rdar://problem/48421064>
+
+ Reviewed by Alex Christensen.
+
+ Do not share WebProcesses between private and regular sessions. There are some privacy concerns.
+ Also, some of the WebsiteDataStore informations are passed via WebProcessCreationParameters (e.g.
+ ApplicationCache path) and cannot be updated later.
+
+ There were 2 cases where this could happen and that are fixed in the patch:
+ - A process may be prewarmed with a given website data store and then later on used for a page
+ associated with a different data store. We now prevent this. While this is not necessary for
+ privacy reasons, it is still useful because our code currently does not support well uses
+ different sessions inside a single WebProcess, as mentioned above.
+ - The client can force a WebsiteDataStore swap when responding to the decidePolicyForNavigationAction,
+ via the WebsitePolicies. To address the issue, we now force a process swap whenever the client
+ makes such a change.
+
+ As a result, WebProcessProxy::websiteDataStore() now makes sense and is always correct. It can
+ also only contains pages whose WebPageProxy::websiteDataStore() returns the same store.
+
+ * UIProcess/API/C/WKContext.cpp:
+ (WKContextWarmInitialProcess):
+ * UIProcess/API/Cocoa/WKProcessPool.mm:
+ (-[WKProcessPool _warmInitialProcess]):
+ * UIProcess/ProvisionalPageProxy.cpp:
+ (WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
+ (WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::notifyProcessPoolToPrewarm):
+ (WebKit::WebPageProxy::reattachToWebProcess):
+ (WebKit::WebPageProxy::swapToWebProcess):
+ (WebKit::WebPageProxy::close):
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+ (WebKit::WebPageProxy::commitProvisionalPage):
+ (WebKit::WebPageProxy::creationParameters):
+ * UIProcess/WebPageProxy.h:
+ (WebKit::WebPageProxy::websiteDataStore):
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::ensureNetworkProcess):
+ (WebKit::WebProcessPool::tryTakePrewarmedProcess):
+ (WebKit::WebProcessPool::prewarmProcess):
+ (WebKit::WebProcessPool::createWebPage):
+ (WebKit::WebProcessPool::pageBeginUsingWebsiteDataStore):
+ (WebKit::WebProcessPool::pageEndUsingWebsiteDataStore):
+ (WebKit::WebProcessPool::didReachGoodTimeToPrewarm):
+ (WebKit::WebProcessPool::processForNavigation):
+ (WebKit::WebProcessPool::processForNavigationInternal):
+ (WebKit::WebProcessPool::findReusableSuspendedPageProcess):
+ * UIProcess/WebProcessPool.h:
+ (WebKit::WebProcessPool::sendToOneProcess):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::createWebPage):
+ (WebKit::WebProcessProxy::addExistingWebPage):
+ (WebKit::WebProcessProxy::removeWebPage):
+ * UIProcess/WebProcessProxy.h:
+
+2019-03-04 Chris Dumez <[email protected]>
+
[iOS] Improve our file picker
https://bugs.webkit.org/show_bug.cgi?id=195284
<rdar://problem/45655856>
Modified: trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp 2019-03-04 20:26:06 UTC (rev 242371)
@@ -524,7 +524,7 @@
void WKContextWarmInitialProcess(WKContextRef contextRef)
{
- WebKit::toImpl(contextRef)->prewarmProcess(WebKit::WebProcessPool::MayCreateDefaultDataStore::Yes);
+ WebKit::toImpl(contextRef)->prewarmProcess(nullptr, WebKit::WebProcessPool::MayCreateDefaultDataStore::Yes);
}
void WKContextGetStatistics(WKContextRef contextRef, void* context, WKContextGetStatisticsFunction callback)
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm 2019-03-04 20:26:06 UTC (rev 242371)
@@ -389,7 +389,7 @@
- (void)_warmInitialProcess
{
- _processPool->prewarmProcess(WebKit::WebProcessPool::MayCreateDefaultDataStore::Yes);
+ _processPool->prewarmProcess(nullptr, WebKit::WebProcessPool::MayCreateDefaultDataStore::Yes);
}
- (void)_automationCapabilitiesDidChange
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-03-04 20:26:06 UTC (rev 242371)
@@ -41,6 +41,7 @@
#include "WebPageProxy.h"
#include "WebPageProxyMessages.h"
#include "WebProcessMessages.h"
+#include "WebProcessPool.h"
#include "WebProcessProxy.h"
#include <WebCore/ShouldTreatAsContinuingLoad.h>
@@ -66,6 +67,9 @@
if (m_process->state() == AuxiliaryProcessProxy::State::Running)
m_page.webProcessLifetimeTracker().webPageEnteringWebProcess(m_process);
+ if (&m_process->websiteDataStore() != &m_page.websiteDataStore())
+ m_process->processPool().pageBeginUsingWebsiteDataStore(m_page.pageID(), m_process->websiteDataStore());
+
// If we are reattaching to a SuspendedPage, then the WebProcess' WebPage already exists and
// WebPageProxy::didCreateMainFrame() will not be called to initialize m_mainFrame. In such
// case, we need to initialize m_mainFrame to reflect the fact the the WebProcess' WebPage
@@ -90,6 +94,9 @@
if (m_process->state() == AuxiliaryProcessProxy::State::Running)
m_page.webProcessLifetimeTracker().webPageLeavingWebProcess(m_process);
+ if (&m_process->websiteDataStore() != &m_page.websiteDataStore())
+ m_process->processPool().pageEndUsingWebsiteDataStore(m_page.pageID(), m_process->websiteDataStore());
+
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
m_process->send(Messages::WebPage::Close(), m_page.pageID());
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-03-04 20:26:06 UTC (rev 242371)
@@ -562,13 +562,6 @@
return drawingArea();
}
-void WebPageProxy::changeWebsiteDataStore(WebsiteDataStore& websiteDataStore)
-{
- m_process->processPool().pageEndUsingWebsiteDataStore(*this);
- m_websiteDataStore = websiteDataStore;
- m_process->processPool().pageBeginUsingWebsiteDataStore(*this);
-}
-
const API::PageConfiguration& WebPageProxy::configuration() const
{
return m_configuration.get();
@@ -593,7 +586,7 @@
void WebPageProxy::notifyProcessPoolToPrewarm()
{
- m_process->processPool().didReachGoodTimeToPrewarm();
+ m_process->processPool().didReachGoodTimeToPrewarm(m_websiteDataStore);
}
void WebPageProxy::setPreferences(WebPreferences& preferences)
@@ -746,7 +739,7 @@
RELEASE_LOG_IF_ALLOWED(Loading, "reattachToWebProcess: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
- m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::Yes);
+ m_process->removeWebPage(*this, WebProcessProxy::EndsUsingDataStore::Yes);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
auto& processPool = m_process->processPool();
@@ -753,7 +746,7 @@
m_process = processPool.createNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.get());
m_isValid = true;
- m_process->addExistingWebPage(*this, m_pageID, WebProcessProxy::BeginsUsingDataStore::Yes);
+ m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::Yes);
m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
finishAttachingToWebProcess(IsProcessSwap::No);
@@ -814,6 +807,8 @@
RELEASE_LOG_IF_ALLOWED(Loading, "swapToWebProcess: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
m_process = WTFMove(process);
+ m_websiteDataStore = m_process->websiteDataStore();
+
ASSERT(!m_drawingArea);
setDrawingArea(WTFMove(drawingArea));
ASSERT(!m_mainFrame);
@@ -820,7 +815,7 @@
m_mainFrame = WTFMove(mainFrame);
m_isValid = true;
- m_process->addExistingWebPage(*this, m_pageID, WebProcessProxy::BeginsUsingDataStore::No);
+ m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::No);
m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
finishAttachingToWebProcess(IsProcessSwap::Yes);
@@ -1032,7 +1027,7 @@
m_process->processPool().removeAllSuspendedPagesForPage(*this);
m_process->send(Messages::WebPage::Close(), m_pageID);
- m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::Yes);
+ m_process->removeWebPage(*this, WebProcessProxy::EndsUsingDataStore::Yes);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
m_process->processPool().supplement<WebNotificationManagerProxy>()->clearNotifications(this);
@@ -2745,11 +2740,14 @@
void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, API::Navigation* navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, WebFrameProxy& frame, API::WebsitePolicies* policies, Ref<PolicyDecisionSender>&& sender)
{
+ Ref<WebsiteDataStore> websiteDataStore = m_websiteDataStore.copyRef();
Optional<WebsitePoliciesData> data;
if (policies) {
data = ""
- if (policies->websiteDataStore())
- changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
+ if (policies->websiteDataStore() && &policies->websiteDataStore()->websiteDataStore() != websiteDataStore.ptr()) {
+ websiteDataStore = policies->websiteDataStore()->websiteDataStore();
+ processSwapRequestedByClient = ProcessSwapRequestedByClient::Yes;
+ }
}
if (navigation && !navigation->userContentExtensionsEnabled()) {
@@ -2776,7 +2774,7 @@
}
}
- process().processPool().processForNavigation(*this, *navigation, sourceProcess.copyRef(), sourceURL, processSwapRequestedByClient, [this, protectedThis = makeRef(*this), policyAction, navigation = makeRef(*navigation), sourceProcess = sourceProcess.copyRef(),
+ process().processPool().processForNavigation(*this, *navigation, sourceProcess.copyRef(), sourceURL, processSwapRequestedByClient, WTFMove(websiteDataStore), [this, protectedThis = makeRef(*this), policyAction, navigation = makeRef(*navigation), sourceProcess = sourceProcess.copyRef(),
data = "" sender = WTFMove(sender), processSwapRequestedByClient] (Ref<WebProcessProxy>&& processForNavigation, SuspendedPageProxy* destinationSuspendedPage, const String& reason) mutable {
// If the navigation has been destroyed, then no need to proceed.
if (isClosed() || !navigationState().hasNavigation(navigation->navigationID())) {
@@ -2853,7 +2851,7 @@
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
auto* navigation = navigationState().navigation(m_provisionalPage->navigationID());
bool didSuspendPreviousPage = navigation ? suspendCurrentPageIfPossible(*navigation, mainFrameIDInPreviousProcess, m_provisionalPage->processSwapRequestedByClient()) : false;
- m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::No);
+ m_process->removeWebPage(*this, m_websiteDataStore.ptr() == &m_provisionalPage->process().websiteDataStore() ? WebProcessProxy::EndsUsingDataStore::No : WebProcessProxy::EndsUsingDataStore::Yes);
// There is no way we'll be able to return to the page in the previous page so close it.
if (!didSuspendPreviousPage)
@@ -6904,10 +6902,10 @@
parameters.paginationLineGridEnabled = m_paginationLineGridEnabled;
parameters.userAgent = userAgent();
parameters.itemStates = m_backForwardList->itemStates();
- parameters.sessionID = sessionID();
+ parameters.sessionID = process.websiteDataStore().sessionID();
parameters.userContentControllerID = m_userContentController->identifier();
parameters.visitedLinkTableID = m_visitedLinkStore->identifier();
- parameters.websiteDataStoreID = m_websiteDataStore->identifier();
+ parameters.websiteDataStoreID = process.websiteDataStore().identifier();
parameters.canRunBeforeUnloadConfirmPanel = m_uiClient->canRunBeforeUnloadConfirmPanel();
parameters.canRunModal = m_canRunModal;
parameters.deviceScaleFactor = deviceScaleFactor();
@@ -6989,7 +6987,7 @@
#endif
#if ENABLE(SERVICE_WORKER)
- parameters.hasRegisteredServiceWorkers = process.processPool().mayHaveRegisteredServiceWorkers(m_websiteDataStore);
+ parameters.hasRegisteredServiceWorkers = process.processPool().mayHaveRegisteredServiceWorkers(process.websiteDataStore());
#endif
parameters.needsFontAttributes = m_needsFontAttributes;
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-03-04 20:26:06 UTC (rev 242371)
@@ -377,7 +377,6 @@
WebNavigationState& navigationState() { return *m_navigationState.get(); }
WebsiteDataStore& websiteDataStore() { return m_websiteDataStore; }
- void changeWebsiteDataStore(WebsiteDataStore&);
void addPreviouslyVisitedPath(const String&);
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-03-04 20:26:06 UTC (rev 242371)
@@ -604,7 +604,7 @@
}
// Make sure the network process knows about all the sessions that have been registered before it started.
- for (auto& sessionID : m_sessionToPagesMap.keys()) {
+ for (auto& sessionID : m_sessionToPageIDsMap.keys()) {
if (auto* websiteDataStore = WebsiteDataStore::existingNonDefaultDataStoreForSessionID(sessionID))
m_networkProcess->addSession(*websiteDataStore);
}
@@ -784,6 +784,9 @@
if (!m_prewarmedProcess)
return nullptr;
+ if (&m_prewarmedProcess->websiteDataStore() != &websiteDataStore)
+ return nullptr;
+
ASSERT(m_prewarmedProcess->isPrewarmed());
m_prewarmedProcess->markIsNoLongerInPrewarmedPool();
@@ -980,22 +983,31 @@
#endif
}
-void WebProcessPool::prewarmProcess(MayCreateDefaultDataStore mayCreateDefaultDataStore)
+void WebProcessPool::prewarmProcess(WebsiteDataStore* websiteDataStore, MayCreateDefaultDataStore mayCreateDefaultDataStore)
{
+ if (m_prewarmedProcess && websiteDataStore && &m_prewarmedProcess->websiteDataStore() != websiteDataStore) {
+ RELEASE_LOG(PerformanceLogging, "Shutting down prewarmed process %i because we needed a prewarmed process with a different data store", m_prewarmedProcess->processIdentifier());
+ m_prewarmedProcess->shutDown();
+ ASSERT(!m_prewarmedProcess);
+ }
+
if (m_prewarmedProcess)
return;
- auto* websiteDataStore = m_websiteDataStore ? &m_websiteDataStore->websiteDataStore() : nullptr;
if (!websiteDataStore) {
- if (!m_processes.isEmpty())
- websiteDataStore = &m_processes.last()->websiteDataStore();
- else if (mayCreateDefaultDataStore == MayCreateDefaultDataStore::Yes || API::WebsiteDataStore::defaultDataStoreExists())
- websiteDataStore = &API::WebsiteDataStore::defaultDataStore()->websiteDataStore();
- else {
- RELEASE_LOG(PerformanceLogging, "Unable to prewarming a WebProcess because we could not find a usable data store");
- return;
+ websiteDataStore = m_websiteDataStore ? &m_websiteDataStore->websiteDataStore() : nullptr;
+ if (!websiteDataStore) {
+ if (!m_processes.isEmpty())
+ websiteDataStore = &m_processes.last()->websiteDataStore();
+ else if (mayCreateDefaultDataStore == MayCreateDefaultDataStore::Yes || API::WebsiteDataStore::defaultDataStoreExists())
+ websiteDataStore = &API::WebsiteDataStore::defaultDataStore()->websiteDataStore();
+ else {
+ RELEASE_LOG(PerformanceLogging, "Unable to prewarming a WebProcess because we could not find a usable data store");
+ return;
+ }
}
}
+
ASSERT(websiteDataStore);
RELEASE_LOG(PerformanceLogging, "Prewarming a WebProcess for performance");
@@ -1157,12 +1169,18 @@
process = &createNewWebProcessRespectingProcessCountLimit(pageConfiguration->websiteDataStore()->websiteDataStore());
}
+ auto page = process->createWebPage(pageClient, WTFMove(pageConfiguration));
+
#if ENABLE(SERVICE_WORKER)
ASSERT(!is<ServiceWorkerProcessProxy>(*process));
+
+ if (!m_serviceWorkerPreferences) {
+ m_serviceWorkerPreferences = page->preferencesStore();
+ for (auto* serviceWorkerProcess : m_serviceWorkerProcesses.values())
+ serviceWorkerProcess->updatePreferencesStore(*m_serviceWorkerPreferences);
+ }
#endif
- auto page = process->createWebPage(pageClient, WTFMove(pageConfiguration));
-
bool enableProcessSwapOnCrossSiteNavigation = page->preferences().processSwapOnCrossSiteNavigationEnabled();
#if PLATFORM(IOS_FAMILY)
if (WebCore::IOSApplication::isFirefox() && !linkedOnOrAfter(WebKit::SDKVersion::FirstWithProcessSwapOnCrossSiteNavigation))
@@ -1206,43 +1224,35 @@
}
#endif
-void WebProcessPool::pageBeginUsingWebsiteDataStore(WebPageProxy& page)
+void WebProcessPool::pageBeginUsingWebsiteDataStore(uint64_t pageID, WebsiteDataStore& dataStore)
{
- auto result = m_sessionToPagesMap.add(page.sessionID(), HashSet<WebPageProxy*>()).iterator->value.add(&page);
+ auto result = m_sessionToPageIDsMap.add(dataStore.sessionID(), HashSet<uint64_t>()).iterator->value.add(pageID);
ASSERT_UNUSED(result, result.isNewEntry);
- auto sessionID = page.sessionID();
+ auto sessionID = dataStore.sessionID();
if (sessionID.isEphemeral()) {
- ASSERT(page.websiteDataStore().parameters().networkSessionParameters.sessionID == sessionID);
+ ASSERT(dataStore.parameters().networkSessionParameters.sessionID == sessionID);
if (m_networkProcess)
- m_networkProcess->addSession(makeRef(page.websiteDataStore()));
- page.websiteDataStore().clearPendingCookies();
+ m_networkProcess->addSession(makeRef(dataStore));
+ dataStore.clearPendingCookies();
} else if (sessionID != PAL::SessionID::defaultSessionID()) {
if (m_networkProcess)
- m_networkProcess->addSession(makeRef(page.websiteDataStore()));
- page.websiteDataStore().clearPendingCookies();
+ m_networkProcess->addSession(makeRef(dataStore));
+ dataStore.clearPendingCookies();
}
-
-#if ENABLE(SERVICE_WORKER)
- if (!m_serviceWorkerPreferences) {
- m_serviceWorkerPreferences = page.preferencesStore();
- for (auto* serviceWorkerProcess : m_serviceWorkerProcesses.values())
- serviceWorkerProcess->updatePreferencesStore(*m_serviceWorkerPreferences);
- }
-#endif
}
-void WebProcessPool::pageEndUsingWebsiteDataStore(WebPageProxy& page)
+void WebProcessPool::pageEndUsingWebsiteDataStore(uint64_t pageID, WebsiteDataStore& dataStore)
{
- auto sessionID = page.sessionID();
- auto iterator = m_sessionToPagesMap.find(sessionID);
- ASSERT(iterator != m_sessionToPagesMap.end());
+ auto sessionID = dataStore.sessionID();
+ auto iterator = m_sessionToPageIDsMap.find(sessionID);
+ ASSERT(iterator != m_sessionToPageIDsMap.end());
- auto takenPage = iterator->value.take(&page);
- ASSERT_UNUSED(takenPage, takenPage == &page);
+ auto takenPageID = iterator->value.take(pageID);
+ ASSERT_UNUSED(takenPageID, takenPageID == pageID);
if (iterator->value.isEmpty()) {
- m_sessionToPagesMap.remove(iterator);
+ m_sessionToPageIDsMap.remove(iterator);
if (sessionID == PAL::SessionID::defaultSessionID())
return;
@@ -1310,7 +1320,7 @@
}
}
-void WebProcessPool::didReachGoodTimeToPrewarm()
+void WebProcessPool::didReachGoodTimeToPrewarm(WebsiteDataStore& dataStore)
{
if (!configuration().isAutomaticProcessWarmingEnabled() || !configuration().processSwapsOnNavigation() || usesSingleWebProcess())
return;
@@ -1321,7 +1331,7 @@
return;
}
- prewarmProcess(MayCreateDefaultDataStore::No);
+ prewarmProcess(&dataStore, MayCreateDefaultDataStore::No);
}
void WebProcessPool::populateVisitedLinks()
@@ -2139,9 +2149,9 @@
m_swappedProcessesPerRegistrableDomain.remove(registrableDomain);
}
-void WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, Ref<WebProcessProxy>&& sourceProcess, const URL& sourceURL, ProcessSwapRequestedByClient processSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&& completionHandler)
+void WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, Ref<WebProcessProxy>&& sourceProcess, const URL& sourceURL, ProcessSwapRequestedByClient processSwapRequestedByClient, Ref<WebsiteDataStore>&& dataStore, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&& completionHandler)
{
- processForNavigationInternal(page, navigation, sourceProcess.copyRef(), sourceURL, processSwapRequestedByClient, [this, page = makeRefPtr(page), navigation = makeRef(navigation), sourceProcess = sourceProcess.copyRef(), sourceURL, processSwapRequestedByClient, completionHandler = WTFMove(completionHandler)](Ref<WebProcessProxy>&& process, SuspendedPageProxy* suspendedPage, const String& reason) mutable {
+ processForNavigationInternal(page, navigation, sourceProcess.copyRef(), sourceURL, processSwapRequestedByClient, WTFMove(dataStore), [this, page = makeRefPtr(page), navigation = makeRef(navigation), sourceProcess = sourceProcess.copyRef(), sourceURL, processSwapRequestedByClient, completionHandler = WTFMove(completionHandler)](Ref<WebProcessProxy>&& process, SuspendedPageProxy* suspendedPage, const String& reason) mutable {
// We are process-swapping so automatic process prewarming would be beneficial if the client has not explicitly enabled / disabled it.
bool doingAnAutomaticProcessSwap = processSwapRequestedByClient == ProcessSwapRequestedByClient::No && process.ptr() != sourceProcess.ptr();
if (doingAnAutomaticProcessSwap && !configuration().wasAutomaticProcessWarmingSetByClient() && !configuration().clientWouldBenefitFromAutomaticProcessPrewarming()) {
@@ -2164,22 +2174,22 @@
});
}
-void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, Ref<WebProcessProxy>&& sourceProcess, const URL& pageSourceURL, ProcessSwapRequestedByClient processSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&& completionHandler)
+void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, Ref<WebProcessProxy>&& sourceProcess, const URL& pageSourceURL, ProcessSwapRequestedByClient processSwapRequestedByClient, Ref<WebsiteDataStore>&& dataStore, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&& completionHandler)
{
auto& targetURL = navigation.currentRequest().url();
auto registrableDomain = toRegistrableDomain(targetURL);
- auto createNewProcess = [this, protectedThis = makeRef(*this), page = makeRef(page), targetURL, registrableDomain] () -> Ref<WebProcessProxy> {
- if (auto process = webProcessCache().takeProcess(registrableDomain, page->websiteDataStore()))
+ auto createNewProcess = [this, protectedThis = makeRef(*this), page = makeRef(page), targetURL, registrableDomain, dataStore = dataStore.copyRef()] () -> Ref<WebProcessProxy> {
+ if (auto process = webProcessCache().takeProcess(registrableDomain, dataStore))
return process.releaseNonNull();
// Check if we have a suspended page for the given registrable domain and use its process if we do, for performance reasons.
- if (auto process = findReusableSuspendedPageProcess(registrableDomain, page)) {
+ if (auto process = findReusableSuspendedPageProcess(registrableDomain, page, dataStore)) {
RELEASE_LOG(ProcessSwapping, "Using WebProcess %i from a SuspendedPage", process->processIdentifier());
return process.releaseNonNull();
}
- if (auto process = tryTakePrewarmedProcess(page->websiteDataStore())) {
+ if (auto process = tryTakePrewarmedProcess(dataStore)) {
RELEASE_LOG(ProcessSwapping, "Using prewarmed process %i", process->processIdentifier());
tryPrewarmWithDomainInformation(*process, targetURL);
return process.releaseNonNull();
@@ -2186,7 +2196,7 @@
}
RELEASE_LOG(ProcessSwapping, "Launching a new process");
- return createNewWebProcess(page->websiteDataStore());
+ return createNewWebProcess(dataStore);
};
if (usesSingleWebProcess())
@@ -2264,7 +2274,7 @@
LOG(ProcessSwapping, "(ProcessSwapping) Considering re-use of a previously cached process for domain %s", registrableDomain.utf8().data());
if (auto* process = m_swappedProcessesPerRegistrableDomain.get(registrableDomain)) {
- if (&process->websiteDataStore() == &page.websiteDataStore()) {
+ if (&process->websiteDataStore() == dataStore.ptr()) {
LOG(ProcessSwapping, "(ProcessSwapping) Reusing a previously cached process with pid %i to continue navigation to URL %s", process->processIdentifier(), targetURL.string().utf8().data());
// FIXME: Architecturally we do not currently support multiple WebPage's with the same ID in a given WebProcess.
@@ -2281,10 +2291,10 @@
return completionHandler(createNewProcess(), nullptr, reason);
}
-RefPtr<WebProcessProxy> WebProcessPool::findReusableSuspendedPageProcess(const String& registrableDomain, WebPageProxy& page)
+RefPtr<WebProcessProxy> WebProcessPool::findReusableSuspendedPageProcess(const String& registrableDomain, WebPageProxy& page, WebsiteDataStore& dataStore)
{
auto it = m_suspendedPages.findIf([&](auto& suspendedPage) {
- return suspendedPage->registrableDomain() == registrableDomain && &suspendedPage->process().websiteDataStore() == &page.websiteDataStore();
+ return suspendedPage->registrableDomain() == registrableDomain && &suspendedPage->process().websiteDataStore() == &dataStore;
});
if (it == m_suspendedPages.end())
return nullptr;
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2019-03-04 20:26:06 UTC (rev 242371)
@@ -187,8 +187,8 @@
Ref<WebPageProxy> createWebPage(PageClient&, Ref<API::PageConfiguration>&&);
- void pageBeginUsingWebsiteDataStore(WebPageProxy&);
- void pageEndUsingWebsiteDataStore(WebPageProxy&);
+ void pageBeginUsingWebsiteDataStore(uint64_t pageID, WebsiteDataStore&);
+ void pageEndUsingWebsiteDataStore(uint64_t pageID, WebsiteDataStore&);
const String& injectedBundlePath() const { return m_configuration->injectedBundlePath(); }
@@ -306,7 +306,7 @@
WebProcessProxy& createNewWebProcessRespectingProcessCountLimit(WebsiteDataStore&); // Will return an existing one if limit is met.
enum class MayCreateDefaultDataStore { No, Yes };
- void prewarmProcess(MayCreateDefaultDataStore);
+ void prewarmProcess(WebsiteDataStore*, MayCreateDefaultDataStore);
bool shouldTerminate(WebProcessProxy*);
@@ -455,7 +455,7 @@
BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
#endif
- void processForNavigation(WebPageProxy&, const API::Navigation&, Ref<WebProcessProxy>&& sourceProcess, const URL& sourceURL, ProcessSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&&);
+ void processForNavigation(WebPageProxy&, const API::Navigation&, Ref<WebProcessProxy>&& sourceProcess, const URL& sourceURL, ProcessSwapRequestedByClient, Ref<WebsiteDataStore>&&, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&&);
// SuspendedPageProxy management.
void addSuspendedPage(std::unique_ptr<SuspendedPageProxy>&&);
@@ -465,11 +465,11 @@
void removeSuspendedPage(SuspendedPageProxy&);
bool hasSuspendedPageFor(WebProcessProxy&, WebPageProxy&) const;
unsigned maxSuspendedPageCount() const { return m_maxSuspendedPageCount; }
- RefPtr<WebProcessProxy> findReusableSuspendedPageProcess(const String&, WebPageProxy&);
+ RefPtr<WebProcessProxy> findReusableSuspendedPageProcess(const String&, WebPageProxy&, WebsiteDataStore&);
void clearSuspendedPages(AllowProcessCaching);
- void didReachGoodTimeToPrewarm();
+ void didReachGoodTimeToPrewarm(WebsiteDataStore&);
void didCollectPrewarmInformation(const String& registrableDomain, const WebCore::PrewarmInformation&);
@@ -499,7 +499,7 @@
void platformInitializeWebProcess(WebProcessCreationParameters&);
void platformInvalidateContext();
- void processForNavigationInternal(WebPageProxy&, const API::Navigation&, Ref<WebProcessProxy>&& sourceProcess, const URL& sourceURL, ProcessSwapRequestedByClient, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&&);
+ void processForNavigationInternal(WebPageProxy&, const API::Navigation&, Ref<WebProcessProxy>&& sourceProcess, const URL& sourceURL, ProcessSwapRequestedByClient, Ref<WebsiteDataStore>&&, CompletionHandler<void(Ref<WebProcessProxy>&&, SuspendedPageProxy*, const String&)>&&);
RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);
@@ -730,7 +730,7 @@
};
Paths m_resolvedPaths;
- HashMap<PAL::SessionID, HashSet<WebPageProxy*>> m_sessionToPagesMap;
+ HashMap<PAL::SessionID, HashSet<uint64_t>> m_sessionToPageIDsMap;
RunLoop::Timer<WebProcessPool> m_serviceWorkerProcessesTerminationTimer;
#if PLATFORM(IOS_FAMILY)
@@ -809,7 +809,7 @@
}
if (!messageSent) {
- prewarmProcess(MayCreateDefaultDataStore::No);
+ prewarmProcess(nullptr, MayCreateDefaultDataStore::No);
RefPtr<WebProcessProxy> process = m_processes.last();
if (process->canSendMessage())
process->send(std::forward<T>(message), 0);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-03-04 20:26:06 UTC (rev 242371)
@@ -349,22 +349,23 @@
uint64_t pageID = generatePageID();
Ref<WebPageProxy> webPage = WebPageProxy::create(pageClient, *this, pageID, WTFMove(pageConfiguration));
- addExistingWebPage(webPage.get(), pageID, BeginsUsingDataStore::Yes);
+ addExistingWebPage(webPage.get(), BeginsUsingDataStore::Yes);
return webPage;
}
-void WebProcessProxy::addExistingWebPage(WebPageProxy& webPage, uint64_t pageID, BeginsUsingDataStore beginsUsingDataStore)
+void WebProcessProxy::addExistingWebPage(WebPageProxy& webPage, BeginsUsingDataStore beginsUsingDataStore)
{
- ASSERT(!m_pageMap.contains(pageID));
- ASSERT(!globalPageMap().contains(pageID));
+ ASSERT(!m_pageMap.contains(webPage.pageID()));
+ ASSERT(!globalPageMap().contains(webPage.pageID()));
ASSERT(!m_isInProcessCache);
+ ASSERT(m_websiteDataStore.ptr() == &webPage.websiteDataStore());
if (beginsUsingDataStore == BeginsUsingDataStore::Yes)
- m_processPool->pageBeginUsingWebsiteDataStore(webPage);
+ m_processPool->pageBeginUsingWebsiteDataStore(webPage.pageID(), webPage.websiteDataStore());
- m_pageMap.set(pageID, &webPage);
- globalPageMap().set(pageID, &webPage);
+ m_pageMap.set(webPage.pageID(), &webPage);
+ globalPageMap().set(webPage.pageID(), &webPage);
updateBackgroundResponsivenessTimer();
}
@@ -380,15 +381,15 @@
send(Messages::WebProcess::MarkIsNoLongerPrewarmed(), 0);
}
-void WebProcessProxy::removeWebPage(WebPageProxy& webPage, uint64_t pageID, EndsUsingDataStore endsUsingDataStore)
+void WebProcessProxy::removeWebPage(WebPageProxy& webPage, EndsUsingDataStore endsUsingDataStore)
{
- auto* removedPage = m_pageMap.take(pageID);
+ auto* removedPage = m_pageMap.take(webPage.pageID());
ASSERT_UNUSED(removedPage, removedPage == &webPage);
- removedPage = globalPageMap().take(pageID);
+ removedPage = globalPageMap().take(webPage.pageID());
ASSERT_UNUSED(removedPage, removedPage == &webPage);
if (endsUsingDataStore == EndsUsingDataStore::Yes)
- m_processPool->pageEndUsingWebsiteDataStore(webPage);
+ m_processPool->pageEndUsingWebsiteDataStore(webPage.pageID(), webPage.websiteDataStore());
updateBackgroundResponsivenessTimer();
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (242370 => 242371)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-03-04 20:26:06 UTC (rev 242371)
@@ -126,10 +126,10 @@
Ref<WebPageProxy> createWebPage(PageClient&, Ref<API::PageConfiguration>&&);
enum class BeginsUsingDataStore : bool { No, Yes };
- void addExistingWebPage(WebPageProxy&, uint64_t pageID, BeginsUsingDataStore);
+ void addExistingWebPage(WebPageProxy&, BeginsUsingDataStore);
enum class EndsUsingDataStore : bool { No, Yes };
- void removeWebPage(WebPageProxy&, uint64_t pageID, EndsUsingDataStore);
+ void removeWebPage(WebPageProxy&, EndsUsingDataStore);
void addProvisionalPageProxy(ProvisionalPageProxy& provisionalPage) { ASSERT(!m_provisionalPages.contains(&provisionalPage)); m_provisionalPages.add(&provisionalPage); }
void removeProvisionalPageProxy(ProvisionalPageProxy& provisionalPage) { ASSERT(m_provisionalPages.contains(&provisionalPage)); m_provisionalPages.remove(&provisionalPage); }
Modified: trunk/Tools/ChangeLog (242370 => 242371)
--- trunk/Tools/ChangeLog 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Tools/ChangeLog 2019-03-04 20:26:06 UTC (rev 242371)
@@ -1,3 +1,16 @@
+2019-03-04 Chris Dumez <[email protected]>
+
+ Do not share WebProcesses between private and regular sessions
+ https://bugs.webkit.org/show_bug.cgi?id=195189
+ <rdar://problem/48421064>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+ * TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:
+
2019-03-04 Michael Catanzaro <[email protected]>
[WPE] Enable web process sandbox
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm (242370 => 242371)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm 2019-03-04 20:26:06 UTC (rev 242371)
@@ -62,7 +62,6 @@
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
configuration.get().processPool = pool.get();
- configuration.get().websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (242370 => 242371)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-03-04 20:26:06 UTC (rev 242371)
@@ -2625,6 +2625,80 @@
EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
}
+TEST(ProcessSwap, PrivateAndRegularSessionsShouldGetDifferentProcesses)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto privateWebViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [privateWebViewConfiguration setProcessPool:processPool.get()];
+ [privateWebViewConfiguration setWebsiteDataStore:[WKWebsiteDataStore nonPersistentDataStore]];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [privateWebViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+ auto regularWebViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [regularWebViewConfiguration setProcessPool:processPool.get()];
+ [regularWebViewConfiguration setWebsiteDataStore:[WKWebsiteDataStore defaultDataStore]];
+ [regularWebViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+
+ auto regularWebView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:regularWebViewConfiguration.get()]);
+ [regularWebView1 setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
+ [regularWebView1 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ [regularWebView1 _close];
+ regularWebView1 = nil;
+
+ auto privateWebView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:privateWebViewConfiguration.get()]);
+ [privateWebView setNavigationDelegate:delegate.get()];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+ [privateWebView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto privateSessionWebkitPID = [privateWebView _webProcessIdentifier];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+ [privateWebView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto privateSessionApplePID = [privateWebView _webProcessIdentifier];
+ EXPECT_NE(privateSessionWebkitPID, privateSessionApplePID);
+
+ [privateWebView _close];
+ privateWebView = nil;
+
+ auto regularWebView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:regularWebViewConfiguration.get()]);
+ [regularWebView2 setNavigationDelegate:delegate.get()];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
+ [regularWebView2 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto regularSessionGooglePID = [regularWebView2 _webProcessIdentifier];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+ [regularWebView2 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto regularSessionWebkitPID = [regularWebView2 _webProcessIdentifier];
+ EXPECT_NE(regularSessionGooglePID, regularSessionWebkitPID);
+ EXPECT_NE(privateSessionWebkitPID, regularSessionWebkitPID);
+}
+
static const char* keepNavigatingFrameBytes = R"PSONRESOURCE(
<body>
<iframe id="testFrame1" src=""
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm (242370 => 242371)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm 2019-03-04 20:23:49 UTC (rev 242370)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm 2019-03-04 20:26:06 UTC (rev 242371)
@@ -1486,6 +1486,11 @@
[cookieWebView loadHTMLString:alertOldCookie baseURL:[NSURL URLWithString:@"http://example.com/checkCookies"]];
TestWebKitAPI::Util::run(&done);
done = false;
+
+ auto pid1 = [cookieWebView _webProcessIdentifier];
+
[cookieWebView loadHTMLString:alertOldCookie baseURL:[NSURL URLWithString:@"http://example.com/checkCookies"]];
TestWebKitAPI::Util::run(&done);
+
+ EXPECT_NE(pid1, [cookieWebView _webProcessIdentifier]);
}
