Clean up site isolation process selection logic #25067

achristensen07 · 2024-02-25T02:31:55Z

`4993196`

Clean up site isolation process selection logic
https://bugs.webkit.org/show_bug.cgi?id=270045
rdar://116202371

Reviewed by Charlie Wolfe.

This pulls the process selection and RemotePageProxy management logic from various places into BrowsingContextGroup.
I introduce a new abstraction, FrameProcess, that represents a frame's use of a particular process.
The WebFrameProxy retains a reference to a FrameProcess instead of a RemotePageProxy.  When a FrameProcess is created
or destroyed, the BrowsingContextGroup updates the containers of RemotePageProxy objects.
This change allows us to keep track of opened pages using structures that are more elegant and robust to handle
all possible combinations of window.open and iframe creation or destruction.

Two small changes are observable in site isolation API tests, but they are both progressions because of processes
and frames being managed more correctly in some cases.

* Source/WebKit/NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::allowsFirstPartyForCookies):
* Source/WebKit/Shared/AuxiliaryProcess.cpp:
(WebKit::AuxiliaryProcess::allowsFirstPartyForCookies):
* Source/WebKit/Sources.txt:
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:
* Source/WebKit/UIProcess/BrowsingContextGroup.cpp:
(WebKit::BrowsingContextGroup::ensureProcessForDomain):
(WebKit::BrowsingContextGroup::processForDomain):
(WebKit::BrowsingContextGroup::addFrameProcess):
(WebKit::BrowsingContextGroup::removeFrameProcess):
(WebKit::BrowsingContextGroup::addPage):
(WebKit::BrowsingContextGroup::removePage):
(WebKit::BrowsingContextGroup::forEachRemotePage):
(WebKit::BrowsingContextGroup::remotePageInProcess):
(WebKit::BrowsingContextGroup::takeRemotePageInProcessForProvisionalPage):
(WebKit::BrowsingContextGroup::transitionPageToRemotePage):
(WebKit::BrowsingContextGroup::addProcessForDomain): Deleted.
* Source/WebKit/UIProcess/BrowsingContextGroup.h:
* Source/WebKit/UIProcess/FrameProcess.cpp: Renamed from Source/WebKit/UIProcess/RemotePageProxyState.h.
(WebKit::FrameProcess::FrameProcess):
(WebKit::FrameProcess::~FrameProcess):
* Source/WebKit/UIProcess/FrameProcess.h: Copied from Source/WebKit/UIProcess/BrowsingContextGroup.h.
(WebKit::FrameProcess::domain const):
(WebKit::FrameProcess::process const):
(WebKit::FrameProcess::process):
(WebKit::FrameProcess::create):
* Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp:
(WebKit::ProvisionalFrameProxy::ProvisionalFrameProxy):
(WebKit::ProvisionalFrameProxy::takeFrameProcess):
(WebKit::ProvisionalFrameProxy::process const):
(WebKit::ProvisionalFrameProxy::takeRemotePageProxy): Deleted.
* Source/WebKit/UIProcess/ProvisionalFrameProxy.h:
(WebKit::ProvisionalFrameProxy::process const): Deleted.
* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
(WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
(WebKit::ProvisionalPageProxy::process):
(WebKit::ProvisionalPageProxy::setNavigation):
(WebKit::ProvisionalPageProxy::cancel):
(WebKit::ProvisionalPageProxy::initializeWebPage):
(WebKit::ProvisionalPageProxy::loadData):
(WebKit::ProvisionalPageProxy::loadRequest):
(WebKit::ProvisionalPageProxy::goToBackForwardItem):
(WebKit::ProvisionalPageProxy::didCreateMainFrame):
(WebKit::ProvisionalPageProxy::didPerformClientRedirect):
(WebKit::ProvisionalPageProxy::didStartProvisionalLoadForFrame):
(WebKit::ProvisionalPageProxy::didFailProvisionalLoadForFrame):
(WebKit::ProvisionalPageProxy::didCommitLoadForFrame):
(WebKit::ProvisionalPageProxy::didNavigateWithNavigationData):
(WebKit::ProvisionalPageProxy::didChangeProvisionalURLForFrame):
(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
(WebKit::ProvisionalPageProxy::decidePolicyForResponse):
(WebKit::ProvisionalPageProxy::didPerformServerRedirect):
(WebKit::ProvisionalPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
(WebKit::ProvisionalPageProxy::startURLSchemeTask):
(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionSync):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageFromWebProcess):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess):
(WebKit::ProvisionalPageProxy::backForwardAddItem):
(WebKit::ProvisionalPageProxy::didDestroyNavigation):
(WebKit::ProvisionalPageProxy::contentFilterDidBlockLoadForFrame):
(WebKit::ProvisionalPageProxy::messageSenderConnection const):
(WebKit::ProvisionalPageProxy::sendMessage):
(WebKit::ProvisionalPageProxy::sendMessageWithAsyncReply):
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
(WebKit::ProvisionalPageProxy::browsingContextGroup):
(WebKit::ProvisionalPageProxy::takeRemotePageProxyState): Deleted.
(WebKit::ProvisionalPageProxy::process): Deleted.
* Source/WebKit/UIProcess/RemotePageProxy.cpp:
(WebKit::RemotePageProxy::RemotePageProxy):
(WebKit::RemotePageProxy::injectPageIntoNewProcess):
(WebKit::RemotePageProxy::~RemotePageProxy):
* Source/WebKit/UIProcess/RemotePageProxy.h:
(WebKit::RemotePageProxy::create): Deleted.
* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
(WebKit::SuspendedPageProxy::sendToAllProcesses):
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::WebFrameProxy):
(WebKit::WebFrameProxy::process const):
(WebKit::WebFrameProxy::processID const):
(WebKit::WebFrameProxy::didCreateSubframe):
(WebKit::WebFrameProxy::prepareForProvisionalNavigationInProcess):
(WebKit::WebFrameProxy::commitProvisionalFrame):
(WebKit::WebFrameProxy::setProcess):
(WebKit::WebFrameProxy::webPageIDInCurrentProcess):
(WebKit::WebFrameProxy::rootFrame):
(WebKit::WebFrameProxy::remotePageProxy const): Deleted.
* Source/WebKit/UIProcess/WebFrameProxy.h:
(WebKit::WebFrameProxy::create):
(WebKit::WebFrameProxy::process const): Deleted.
(WebKit::WebFrameProxy::setProcess): Deleted.
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendCurrentPageIfPossible):
(WebKit::WebPageProxy::swapToProvisionalPage):
(WebKit::WebPageProxy::close):
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
(WebKit::WebPageProxy::loadFile):
(WebKit::WebPageProxy::receivedNavigationActionPolicyDecision):
(WebKit::WebPageProxy::continueNavigationInNewProcess):
(WebKit::WebPageProxy::getAllFrameTrees):
(WebKit::WebPageProxy::forceRepaint):
(WebKit::WebPageProxy::didCreateMainFrame):
(WebKit::WebPageProxy::updateRemoteFrameSize):
(WebKit::WebPageProxy::forEachWebContentProcess):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::callAfterNextPresentationUpdate):
(WebKit::WebPageProxy::processForRegistrableDomain):
(WebKit::WebPageProxy::webPageIDInProcessForDomain const):
(WebKit::WebPageProxy::sendToWebPage):
(WebKit::WebPageProxy::addRemotePageProxy): Deleted.
(WebKit::WebPageProxy::removeRemotePageProxy): Deleted.
(WebKit::WebPageProxy::remotePageProxyForRegistrableDomain const): Deleted.
(WebKit::WebPageProxy::setRemotePageProxyInOpenerProcess): Deleted.
(WebKit::WebPageProxy::takeRemotePageProxyInOpenerProcessIfDomainEquals): Deleted.
(WebKit::WebPageProxy::removeOpenedRemotePageProxy): Deleted.
(WebKit::WebPageProxy::takeOpenedRemotePageProxyIfDomainEquals): Deleted.
(WebKit::WebPageProxy::addOpenedRemotePageProxy): Deleted.
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxyInternals.h:
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation):
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::addVisitedLinkStoreUser):
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::WebFrame):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::m_unifiedTextReplacementController):
(WebKit::WebPage::~WebPage):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::indentation):
(TestWebKitAPI::printTree):
(TestWebKitAPI::checkFrameTreesInProcesses):
(TestWebKitAPI::findFramePID):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/275551@main

8bca334

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-02-25T02:32:05Z

EWS run on previous version of this PR (hash f1d853f)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 wincairo
❌ 🧪 bindings	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
❌ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 gtk
	❌ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🧪 gtk-wk2
	❌ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 api-gtk
	❌ 🛠 tv-sim	❌ 🧪 mac-wk2-stress
	❌ 🛠 watch
	❌ 🛠 watch-sim

webkit-early-warning-system · 2024-02-27T22:34:20Z

EWS run on previous version of this PR (hash 8b58dd9)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	❌ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 gtk
	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
	~~🛠 tv~~	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-02-27T23:24:32Z

EWS run on previous version of this PR (hash 9f5ad9e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~	~~🧪 api-gtk~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-02-28T01:54:36Z

EWS run on previous version of this PR (hash 75d9771)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	~~🧪 mac-wk1~~	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	~~🧪 gtk-wk2~~
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-02-28T04:35:56Z

EWS run on previous version of this PR (hash 7a074a3)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-02-29T22:40:49Z

EWS run on previous version of this PR (hash 5b02650)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

charliewolfe

Nice

charliewolfe · 2024-03-01T04:43:09Z

Source/WebKit/UIProcess/BrowsingContextGroup.cpp

    m_processMap.set(domain, process);
+    for (auto& page : m_pages) {
+        if (domain == WebCore::RegistrableDomain(URL(page.currentURL())))


Is this the correct URL to compare to here? Instead of using the committed URL?

Actually, I think this is correct.

charliewolfe · 2024-03-01T04:45:54Z

Source/WebKit/UIProcess/BrowsingContextGroup.cpp

+    m_remotePages.removeIf([&] (auto& pair) {
+        auto& set = pair.value;
+        set.removeIf([&] (auto& remotePage) {
+            return remotePage->process().coreProcessIdentifier() == process.process().coreProcessIdentifier();


Can we just compare WebProcessProxy? It doesn't seem like we need to compare pids? Here and everywhere.

This isn't comparing pids, it's comparing coreProcessIdentifier which is unique even if the pid is reused. I think it's better than comparing pointers to see if the identity of the process is the same.

charliewolfe · 2024-03-01T04:51:07Z

Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

            m_webPageID = existingRemotePageProxy->pageID();
            m_mainFrame = existingRemotePageProxy->page()->mainFrame();
            m_messageReceiverRegistration.stopReceivingMessages();
            m_messageReceiverRegistration.transferMessageReceivingFrom(existingRemotePageProxy->messageReceiverRegistration(), *this);
            LocalFrameCreationParameters localFrameCreationParameters {
                std::nullopt
            };
-            m_process->send(Messages::WebPage::TransitionFrameToLocal(localFrameCreationParameters, m_page->mainFrame()->frameID()), m_webPageID);
+            process().send(Messages::WebPage::TransitionFrameToLocal(localFrameCreationParameters, m_page->mainFrame()->frameID()), m_webPageID);


protectedProcess()

charliewolfe · 2024-03-01T04:51:13Z

Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

-        m_process->send(Messages::WebProcess::CreateWebPage(m_webPageID, m_page->creationParametersForProvisionalPage(m_process, *m_drawingArea, WTFMove(websitePolicies), WTFMove(mainFrameIdentifier))), 0);
-        m_process->addVisitedLinkStoreUser(m_page->visitedLinkStore(), m_page->identifier());
+        process().send(Messages::WebProcess::CreateWebPage(m_webPageID, m_page->creationParametersForProvisionalPage(process(), *m_drawingArea, WTFMove(websitePolicies), WTFMove(mainFrameIdentifier))), 0);
+        process().addVisitedLinkStoreUser(m_page->visitedLinkStore(), m_page->identifier());


protectedProcess()

charliewolfe · 2024-03-01T04:54:24Z

Source/WebKit/UIProcess/RemotePageProxy.cpp

-    if (m_page && m_page->drawingArea())
+    if (!m_page)
+        return;
+    if (m_page->drawingArea())


Could if (auto* drawingArea = m_page->drawingArea())

charliewolfe · 2024-03-01T04:58:12Z

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -4359,8 +4350,10 @@ void WebPageProxy::receivedNavigationActionPolicyDecision(WebProcessProxy& proce

        Ref pageClientProtector = pageClient();

-        const bool navigationChangesFrameProcess = processNavigatingTo.ptr() != processNavigatingFrom.ptr();
-        const bool loadContinuingInNonInitiatingProcess = processInitiatingNavigation.ptr() != processNavigatingTo.ptr();
+        Ref processNavigatingFrom { frame->provisionalFrame() ? frame->provisionalFrame()->process() : frame->isMainFrame() && m_provisionalPage ? m_provisionalPage->process() : frame->process() };


This line seems like it could benefit from parenthesis, or maybe another line.

charliewolfe · 2024-03-01T05:15:45Z

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -4395,7 +4388,7 @@ void WebPageProxy::receivedNavigationActionPolicyDecision(WebProcessProxy& proce
            // FIXME: Add more parameters as appropriate. <rdar://116200985>
            LoadParameters loadParameters;
            loadParameters.request = navigation->currentRequest();
-            loadParameters.shouldTreatAsContinuingLoad = WebCore::ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;
+            loadParameters.shouldTreatAsContinuingLoad = navigation->currentRequestIsRedirect() ? WebCore::ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted : WebCore::ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;


Suggested change

loadParameters.shouldTreatAsContinuingLoad = navigation->currentRequestIsRedirect() ? WebCore::ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted : WebCore::ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;

loadParameters.shouldTreatAsContinuingLoad = navigation->currentRequestIsRedirect() ? ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted : ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;

charliewolfe · 2024-03-01T05:17:43Z

Source/WebKit/UIProcess/WebPageProxy.cpp

            aggregator->addFrameTree(WTFMove(data));
        });
-    }
+    });


This all can be replaced with forEachWebContentProcess()

charliewolfe · 2024-03-01T05:19:23Z

Source/WebKit/UIProcess/WebPageProxy.cpp

-        Ref { *remotePageProxy }->sendWithAsyncReply(Messages::DrawingArea::DispatchAfterEnsuringDrawing(), [aggregator] { }, drawingAreaIdentifier);
+    m_browsingContextGroup->forEachRemotePage(*this, [&] (auto& remotePageProxy) {
+        remotePageProxy.sendWithAsyncReply(Messages::DrawingArea::DispatchAfterEnsuringDrawing(), [aggregator] { }, drawingAreaIdentifier);
+    });


This and the above line can be replaced with forEachWebContentProcess().

charliewolfe · 2024-03-01T05:21:40Z

Source/WebKit/UIProcess/XR/PlatformXRSystem.h

@@ -37,7 +37,6 @@
 namespace WebCore {
 class SecurityOriginData;
 }
-


webkit-early-warning-system · 2024-03-01T07:11:01Z

EWS run on current version of this PR (hash 8bca334)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=270045 rdar://116202371 Reviewed by Charlie Wolfe. This pulls the process selection and RemotePageProxy management logic from various places into BrowsingContextGroup. I introduce a new abstraction, FrameProcess, that represents a frame's use of a particular process. The WebFrameProxy retains a reference to a FrameProcess instead of a RemotePageProxy. When a FrameProcess is created or destroyed, the BrowsingContextGroup updates the containers of RemotePageProxy objects. This change allows us to keep track of opened pages using structures that are more elegant and robust to handle all possible combinations of window.open and iframe creation or destruction. Two small changes are observable in site isolation API tests, but they are both progressions because of processes and frames being managed more correctly in some cases. * Source/WebKit/NetworkProcess/NetworkProcess.cpp: (WebKit::NetworkProcess::allowsFirstPartyForCookies): * Source/WebKit/Shared/AuxiliaryProcess.cpp: (WebKit::AuxiliaryProcess::allowsFirstPartyForCookies): * Source/WebKit/Sources.txt: * Source/WebKit/UIProcess/AuxiliaryProcessProxy.h: * Source/WebKit/UIProcess/BrowsingContextGroup.cpp: (WebKit::BrowsingContextGroup::ensureProcessForDomain): (WebKit::BrowsingContextGroup::processForDomain): (WebKit::BrowsingContextGroup::addFrameProcess): (WebKit::BrowsingContextGroup::removeFrameProcess): (WebKit::BrowsingContextGroup::addPage): (WebKit::BrowsingContextGroup::removePage): (WebKit::BrowsingContextGroup::forEachRemotePage): (WebKit::BrowsingContextGroup::remotePageInProcess): (WebKit::BrowsingContextGroup::takeRemotePageInProcessForProvisionalPage): (WebKit::BrowsingContextGroup::transitionPageToRemotePage): (WebKit::BrowsingContextGroup::addProcessForDomain): Deleted. * Source/WebKit/UIProcess/BrowsingContextGroup.h: * Source/WebKit/UIProcess/FrameProcess.cpp: Renamed from Source/WebKit/UIProcess/RemotePageProxyState.h. (WebKit::FrameProcess::FrameProcess): (WebKit::FrameProcess::~FrameProcess): * Source/WebKit/UIProcess/FrameProcess.h: Copied from Source/WebKit/UIProcess/BrowsingContextGroup.h. (WebKit::FrameProcess::domain const): (WebKit::FrameProcess::process const): (WebKit::FrameProcess::process): (WebKit::FrameProcess::create): * Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp: (WebKit::ProvisionalFrameProxy::ProvisionalFrameProxy): (WebKit::ProvisionalFrameProxy::takeFrameProcess): (WebKit::ProvisionalFrameProxy::process const): (WebKit::ProvisionalFrameProxy::takeRemotePageProxy): Deleted. * Source/WebKit/UIProcess/ProvisionalFrameProxy.h: (WebKit::ProvisionalFrameProxy::process const): Deleted. * Source/WebKit/UIProcess/ProvisionalPageProxy.cpp: (WebKit::ProvisionalPageProxy::ProvisionalPageProxy): (WebKit::ProvisionalPageProxy::~ProvisionalPageProxy): (WebKit::ProvisionalPageProxy::process): (WebKit::ProvisionalPageProxy::setNavigation): (WebKit::ProvisionalPageProxy::cancel): (WebKit::ProvisionalPageProxy::initializeWebPage): (WebKit::ProvisionalPageProxy::loadData): (WebKit::ProvisionalPageProxy::loadRequest): (WebKit::ProvisionalPageProxy::goToBackForwardItem): (WebKit::ProvisionalPageProxy::didCreateMainFrame): (WebKit::ProvisionalPageProxy::didPerformClientRedirect): (WebKit::ProvisionalPageProxy::didStartProvisionalLoadForFrame): (WebKit::ProvisionalPageProxy::didFailProvisionalLoadForFrame): (WebKit::ProvisionalPageProxy::didCommitLoadForFrame): (WebKit::ProvisionalPageProxy::didNavigateWithNavigationData): (WebKit::ProvisionalPageProxy::didChangeProvisionalURLForFrame): (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync): (WebKit::ProvisionalPageProxy::decidePolicyForResponse): (WebKit::ProvisionalPageProxy::didPerformServerRedirect): (WebKit::ProvisionalPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame): (WebKit::ProvisionalPageProxy::startURLSchemeTask): (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionSync): (WebKit::ProvisionalPageProxy::logDiagnosticMessageFromWebProcess): (WebKit::ProvisionalPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess): (WebKit::ProvisionalPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess): (WebKit::ProvisionalPageProxy::backForwardAddItem): (WebKit::ProvisionalPageProxy::didDestroyNavigation): (WebKit::ProvisionalPageProxy::contentFilterDidBlockLoadForFrame): (WebKit::ProvisionalPageProxy::messageSenderConnection const): (WebKit::ProvisionalPageProxy::sendMessage): (WebKit::ProvisionalPageProxy::sendMessageWithAsyncReply): * Source/WebKit/UIProcess/ProvisionalPageProxy.h: (WebKit::ProvisionalPageProxy::browsingContextGroup): (WebKit::ProvisionalPageProxy::takeRemotePageProxyState): Deleted. (WebKit::ProvisionalPageProxy::process): Deleted. * Source/WebKit/UIProcess/RemotePageProxy.cpp: (WebKit::RemotePageProxy::RemotePageProxy): (WebKit::RemotePageProxy::injectPageIntoNewProcess): (WebKit::RemotePageProxy::~RemotePageProxy): * Source/WebKit/UIProcess/RemotePageProxy.h: (WebKit::RemotePageProxy::create): Deleted. * Source/WebKit/UIProcess/SuspendedPageProxy.cpp: (WebKit::SuspendedPageProxy::SuspendedPageProxy): (WebKit::SuspendedPageProxy::sendToAllProcesses): * Source/WebKit/UIProcess/SuspendedPageProxy.h: * Source/WebKit/UIProcess/WebFrameProxy.cpp: (WebKit::WebFrameProxy::WebFrameProxy): (WebKit::WebFrameProxy::process const): (WebKit::WebFrameProxy::processID const): (WebKit::WebFrameProxy::didCreateSubframe): (WebKit::WebFrameProxy::prepareForProvisionalNavigationInProcess): (WebKit::WebFrameProxy::commitProvisionalFrame): (WebKit::WebFrameProxy::setProcess): (WebKit::WebFrameProxy::webPageIDInCurrentProcess): (WebKit::WebFrameProxy::rootFrame): (WebKit::WebFrameProxy::remotePageProxy const): Deleted. * Source/WebKit/UIProcess/WebFrameProxy.h: (WebKit::WebFrameProxy::create): (WebKit::WebFrameProxy::process const): Deleted. (WebKit::WebFrameProxy::setProcess): Deleted. * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::suspendCurrentPageIfPossible): (WebKit::WebPageProxy::swapToProvisionalPage): (WebKit::WebPageProxy::close): (WebKit::WebPageProxy::loadRequestWithNavigationShared): (WebKit::WebPageProxy::loadFile): (WebKit::WebPageProxy::receivedNavigationActionPolicyDecision): (WebKit::WebPageProxy::continueNavigationInNewProcess): (WebKit::WebPageProxy::getAllFrameTrees): (WebKit::WebPageProxy::forceRepaint): (WebKit::WebPageProxy::didCreateMainFrame): (WebKit::WebPageProxy::updateRemoteFrameSize): (WebKit::WebPageProxy::forEachWebContentProcess): (WebKit::WebPageProxy::decidePolicyForNavigationAction): (WebKit::WebPageProxy::callAfterNextPresentationUpdate): (WebKit::WebPageProxy::processForRegistrableDomain): (WebKit::WebPageProxy::webPageIDInProcessForDomain const): (WebKit::WebPageProxy::sendToWebPage): (WebKit::WebPageProxy::addRemotePageProxy): Deleted. (WebKit::WebPageProxy::removeRemotePageProxy): Deleted. (WebKit::WebPageProxy::remotePageProxyForRegistrableDomain const): Deleted. (WebKit::WebPageProxy::setRemotePageProxyInOpenerProcess): Deleted. (WebKit::WebPageProxy::takeRemotePageProxyInOpenerProcessIfDomainEquals): Deleted. (WebKit::WebPageProxy::removeOpenedRemotePageProxy): Deleted. (WebKit::WebPageProxy::takeOpenedRemotePageProxyIfDomainEquals): Deleted. (WebKit::WebPageProxy::addOpenedRemotePageProxy): Deleted. * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/UIProcess/WebPageProxyInternals.h: * Source/WebKit/UIProcess/WebProcessPool.cpp: (WebKit::WebProcessPool::processForNavigation): * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::addVisitedLinkStoreUser): * Source/WebKit/WebKit.xcodeproj/project.pbxproj: * Source/WebKit/WebProcess/WebPage/WebFrame.cpp: (WebKit::WebFrame::WebFrame): * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::m_unifiedTextReplacementController): (WebKit::WebPage::~WebPage): * Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm: (TestWebKitAPI::indentation): (TestWebKitAPI::printTree): (TestWebKitAPI::checkFrameTreesInProcesses): (TestWebKitAPI::findFramePID): (TestWebKitAPI::TEST): Canonical link: https://commits.webkit.org/275551@main

webkit-commit-queue · 2024-03-01T14:38:25Z

Committed 275551@main (4993196): https://commits.webkit.org/275551@main

Reviewed commits have been landed. Closing PR #25067 and removing active labels.

achristensen07 requested a review from cdumez as a code owner February 25, 2024 02:31

achristensen07 self-assigned this Feb 25, 2024

achristensen07 added the WebKit Process Model Bugs related to WebKit's multi-process architecture label Feb 25, 2024

achristensen07 force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from f1d853f to 8b58dd9 Compare February 27, 2024 22:34

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 27, 2024

achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Feb 27, 2024

achristensen07 force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from 8b58dd9 to 9f5ad9e Compare February 27, 2024 23:22

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 28, 2024

achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Feb 28, 2024

achristensen07 force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from 9f5ad9e to 75d9771 Compare February 28, 2024 01:54

achristensen07 force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from 75d9771 to 7a074a3 Compare February 28, 2024 04:35

achristensen07 force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from 7a074a3 to 5b02650 Compare February 29, 2024 22:40

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 1, 2024

achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Mar 1, 2024

charliewolfe reviewed Mar 1, 2024

View reviewed changes

charliewolfe approved these changes Mar 1, 2024

View reviewed changes

achristensen07 force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from 5b02650 to 8bca334 Compare March 1, 2024 07:10

achristensen07 added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 1, 2024

webkit-commit-queue force-pushed the eng/Clean-up-site-isolation-process-selection-logic branch from 8bca334 to 4993196 Compare March 1, 2024 14:38

webkit-commit-queue merged commit 4993196 into WebKit:main Mar 1, 2024

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up site isolation process selection logic #25067

Clean up site isolation process selection logic #25067

achristensen07 commented Feb 25, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Feb 25, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 27, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 27, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 28, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 28, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 29, 2024 •

edited

Loading

charliewolfe left a comment

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

achristensen07 Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

charliewolfe Mar 1, 2024

webkit-early-warning-system commented Mar 1, 2024 •

edited

Loading

webkit-commit-queue commented Mar 1, 2024

	loadParameters.shouldTreatAsContinuingLoad = navigation->currentRequestIsRedirect() ? WebCore::ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted : WebCore::ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;
	loadParameters.shouldTreatAsContinuingLoad = navigation->currentRequestIsRedirect() ? ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted : ShouldTreatAsContinuingLoad::YesAfterNavigationPolicyDecision;

Clean up site isolation process selection logic #25067

Clean up site isolation process selection logic #25067

Conversation

achristensen07 commented Feb 25, 2024 • edited by webkit-early-warning-system Loading

webkit-early-warning-system commented Feb 25, 2024 • edited Loading

webkit-early-warning-system commented Feb 27, 2024 • edited Loading

webkit-early-warning-system commented Feb 27, 2024 • edited Loading

webkit-early-warning-system commented Feb 28, 2024 • edited Loading

webkit-early-warning-system commented Feb 28, 2024 • edited Loading

webkit-early-warning-system commented Feb 29, 2024 • edited Loading

charliewolfe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Mar 1, 2024 • edited Loading

webkit-commit-queue commented Mar 1, 2024

achristensen07 commented Feb 25, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Feb 25, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 27, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 27, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 28, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 28, 2024 •

edited

Loading

webkit-early-warning-system commented Feb 29, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 1, 2024 •

edited

Loading