Terminate iframe processes when navigating away

https://bugs.webkit.org/show_bug.cgi?id=262056
rdar://116004705

Reviewed by Pascoe.

While deciding what to do about all the processes that are currently being kept alive
by the back/forward cache with site isolation on, I investigated and found that Chrome
and Firefox both terminate iframe processes immediately after navigating away from a
page.  This can be seen by going to a site with many third-party iframes then opening
example.com

To accomplish this in WebKit, instead of keeping the domain-to-remote-page map on the
SuspendedPageProxy, we remove all RemotePageProxy objects from all frames going into
the back/forward cache when a SuspendedPageProxy is created.

To get the process to actually terminate, I needed to change the WebProcessProxy's
conditions for not shutting down from watching its ProvisionalFrameProxy and
WebFrameProxy-with-remote-page count to just watching its RemotePageProxy count.
This simplifies things, makes them easier to keep track of, and reflects the progression
of how site isolation has developed: we started by having frames manage process lifetimes
more and progressed to having the RemotePageProxy be in charge.

* Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp:
(WebKit::ProvisionalFrameProxy::ProvisionalFrameProxy):
(WebKit::ProvisionalFrameProxy::~ProvisionalFrameProxy): Deleted.
* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
(WebKit::ProvisionalPageProxy::takeRemotePageMap): Deleted.
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
(WebKit::ProvisionalPageProxy::provisionalURL const):
* Source/WebKit/UIProcess/RemotePageProxy.cpp:
(WebKit::RemotePageProxy::RemotePageProxy):
(WebKit::RemotePageProxy::~RemotePageProxy):
* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
(WebKit::SuspendedPageProxy::takeRemotePageMap): Deleted.
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::commitProvisionalFrame):
(WebKit::WebFrameProxy::removeRemotePagesForSuspension):
* Source/WebKit/UIProcess/WebFrameProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::swapToProvisionalPage):
(WebKit::WebPageProxy::takeRemotePageMap): Deleted.
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::addRemotePageProxy):
(WebKit::WebProcessProxy::removeRemotePageProxy):
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
(WebKit::WebProcessProxy::addProvisionalFrameProxy): Deleted.
(WebKit::WebProcessProxy::removeProvisionalFrameProxy): Deleted.
(WebKit::WebProcessProxy::provisionalFrameCommitted): Deleted.
(WebKit::WebProcessProxy::removeFrameWithRemoteFrameProcess): Deleted.
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::processStillRunning):
(TestWebKitAPI::TEST):

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

Source/WebKit/UIProcess/ProvisionalFrameProxy.cpp

@@ -44,13 +44,9 @@ ProvisionalFrameProxy::ProvisionalFrameProxy(WebFrameProxy& frame, WebProcessPro
 {
     ASSERT(!m_remotePageProxy || m_remotePageProxy->process().coreProcessIdentifier() == process.coreProcessIdentifier());
     m_process->markProcessAsRecentlyUsed();
-    m_process->addProvisionalFrameProxy(*this);
 }
 
-ProvisionalFrameProxy::~ProvisionalFrameProxy()
-{
-    m_process->removeProvisionalFrameProxy(*this);
-}
+ProvisionalFrameProxy::~ProvisionalFrameProxy() = default;
 
 RefPtr<RemotePageProxy> ProvisionalFrameProxy::takeRemotePageProxy()
 {

Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

@@ -104,7 +104,6 @@ ProvisionalPageProxy::ProvisionalPageProxy(WebPageProxy& page, Ref<WebProcessPro
         ASSERT(&suspendedPage->process() == m_process.ptr());
         suspendedPage->unsuspend();
         m_mainFrame = &suspendedPage->mainFrame();
-        m_remotePageMap = suspendedPage->takeRemotePageMap();
     }
 
     initializeWebPage(websitePolicies);
@@ -164,11 +163,6 @@ void ProvisionalPageProxy::setNavigation(API::Navigation& navigation)
     navigation.setProcessID(m_process->coreProcessIdentifier());
 }
 
-std::optional<HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>>> ProvisionalPageProxy::takeRemotePageMap()
-{
-    return std::exchange(m_remotePageMap, std::nullopt);
-}
-
 void ProvisionalPageProxy::cancel()
 {
     // If the provisional load started, then indicate that it failed due to cancellation by calling didFailProvisionalLoadForFrame().

Source/WebKit/UIProcess/ProvisionalPageProxy.h

@@ -92,7 +92,6 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
     ProcessSwapRequestedByClient processSwapRequestedByClient() const { return m_processSwapRequestedByClient; }
     uint64_t navigationID() const { return m_navigationID; }
     const URL& provisionalURL() const { return m_provisionalLoadURL; }
-    std::optional<HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>>> takeRemotePageMap();
 
     bool isProcessSwappingOnNavigationResponse() const { return m_isProcessSwappingOnNavigationResponse; }
 
@@ -208,7 +207,6 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
     LayerHostingContextID m_contextIDForVisibilityPropagationInGPUProcess { 0 };
 #endif
 #endif
-    std::optional<HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>>> m_remotePageMap;
 };
 
 } // namespace WebKit

Source/WebKit/UIProcess/RemotePageProxy.cpp

@@ -55,6 +55,9 @@ RemotePageProxy::RemotePageProxy(WebPageProxy& page, WebProcessProxy& process, c
         m_messageReceiverRegistration.transferMessageReceivingFrom(*registrationToTransfer, *this);
     else
         m_messageReceiverRegistration.startReceivingMessages(m_process, m_webPageID, *this);
+
+    m_process->addRemotePageProxy(*this);
+
     page.addRemotePageProxy(domain, *this);
 }
 
@@ -81,6 +84,7 @@ void RemotePageProxy::injectPageIntoNewProcess()
 
 RemotePageProxy::~RemotePageProxy()
 {
+    m_process->removeRemotePageProxy(*this);
     if (m_page)
         m_page->removeRemotePageProxy(m_domain);
 }

Source/WebKit/UIProcess/SuspendedPageProxy.cpp

@@ -111,11 +111,11 @@ SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&
     , m_contextIDForVisibilityPropagationInGPUProcess(page.contextIDForVisibilityPropagationInGPUProcess())
 #endif
 #endif
-    , m_remotePageMap(page.takeRemotePageMap())
 {
     allSuspendedPages().add(*this);
     m_process->addSuspendedPageProxy(*this);
     m_messageReceiverRegistration.startReceivingMessages(m_process, m_webPageID, *this);
+    m_mainFrame->removeRemotePagesForSuspension();
     m_suspensionTimeoutTimer.startOneShot(suspensionTimeout);
     send(Messages::WebPage::SetIsSuspended(true));
 }
@@ -144,11 +144,6 @@ Ref<WebPageProxy> SuspendedPageProxy::protectedPage() const
     return m_page.get();
 }
 
-HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> SuspendedPageProxy::takeRemotePageMap()
-{
-    return std::exchange(m_remotePageMap, { });
-}
-
 void SuspendedPageProxy::didDestroyNavigation(uint64_t navigationID)
 {
     protectedPage()->didDestroyNavigationShared(m_process.copyRef(), navigationID);

Source/WebKit/UIProcess/SuspendedPageProxy.h

@@ -66,7 +66,6 @@ class SuspendedPageProxy final: public IPC::MessageReceiver, public IPC::Message
     WebCore::PageIdentifier webPageID() const { return m_webPageID; }
     WebProcessProxy& process() const { return m_process.get(); }
     WebFrameProxy& mainFrame() { return m_mainFrame.get(); }
-    HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> takeRemotePageMap();
 
     WebBackForwardCache& backForwardCache() const;
 
@@ -130,7 +129,6 @@ class SuspendedPageProxy final: public IPC::MessageReceiver, public IPC::Message
     LayerHostingContextID m_contextIDForVisibilityPropagationInGPUProcess { 0 };
 #endif
 #endif
-    HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> m_remotePageMap;
 };
 
 } // namespace WebKit

Source/WebKit/UIProcess/WebFrameProxy.cpp

@@ -432,7 +432,6 @@ void WebFrameProxy::commitProvisionalFrame(FrameIdentifier frameID, FrameInfoDat
 {
     ASSERT(m_page);
     if (m_provisionalFrame) {
-        m_provisionalFrame->process().provisionalFrameCommitted(*this);
         m_process->send(Messages::WebPage::DidCommitLoadInAnotherProcess(frameID, m_provisionalFrame->layerHostingContextIdentifier()), m_page->webPageID());
         m_process = m_provisionalFrame->process();
         m_remotePageProxy = m_provisionalFrame->takeRemotePageProxy();
@@ -504,6 +503,13 @@ RefPtr<RemotePageProxy> WebFrameProxy::remotePageProxy()
     return m_remotePageProxy;
 }
 
+void WebFrameProxy::removeRemotePagesForSuspension()
+{
+    m_remotePageProxy = nullptr;
+    for (auto& child : m_childFrames)
+        child->removeRemotePagesForSuspension();
+}
+
 } // namespace WebKit
 
 #undef MESSAGE_CHECK

Source/WebKit/UIProcess/WebFrameProxy.h

@@ -164,6 +164,7 @@ class WebFrameProxy : public API::ObjectImpl<API::Object::Type::Frame>, public C
     ProvisionalFrameProxy* provisionalFrame() { return m_provisionalFrame.get(); }
     std::unique_ptr<ProvisionalFrameProxy> takeProvisionalFrame();
     RefPtr<RemotePageProxy> remotePageProxy();
+    void removeRemotePagesForSuspension();
 
 private:
     WebFrameProxy(WebPageProxy&, WebProcessProxy&, WebCore::FrameIdentifier);

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -1157,9 +1157,6 @@ void WebPageProxy::swapToProvisionalPage(std::unique_ptr<ProvisionalPageProxy> p
     ASSERT(!m_mainFrame);
     m_mainFrame = provisionalPage->mainFrame();
 
-    if (auto map = provisionalPage->takeRemotePageMap())
-        internals().domainToRemotePageProxyMap = WTFMove(*map);
-
     m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::No);
     addAllMessageReceivers();
 
@@ -12756,11 +12753,6 @@ void WebPageProxy::addOpenedRemotePageProxy(Ref<RemotePageProxy>&& page)
     internals().openedRemotePageProxies.add(WTFMove(page));
 }
 
-HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> WebPageProxy::takeRemotePageMap()
-{
-    return std::exchange(internals().domainToRemotePageProxyMap, { });
-}
-
 #if ENABLE(ADVANCED_PRIVACY_PROTECTIONS)
 
 Vector<LinkDecorationFilteringData>& WebPageProxy::cachedAllowedQueryParametersForAdvancedPrivacyProtections()

Source/WebKit/UIProcess/WebPageProxy.h

@@ -2162,7 +2162,6 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ
     void removeRemotePageProxy(const WebCore::RegistrableDomain&);
     void setRemotePageProxyInOpenerProcess(Ref<RemotePageProxy>&&);
     void addOpenedRemotePageProxy(Ref<RemotePageProxy>&&);
-    HashMap<WebCore::RegistrableDomain, WeakPtr<RemotePageProxy>> takeRemotePageMap();
 
     void createRemoteSubframesInOtherProcesses(WebFrameProxy&);
     void broadcastFrameRemovalToOtherProcesses(IPC::Connection&, WebCore::FrameIdentifier);

Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -459,40 +459,24 @@ void WebProcessProxy::removeProvisionalPageProxy(ProvisionalPageProxy& provision
     }
 }
 
-void WebProcessProxy::addProvisionalFrameProxy(ProvisionalFrameProxy& provisionalFrame)
+void WebProcessProxy::addRemotePageProxy(RemotePageProxy& remotePage)
 {
-    WEBPROCESSPROXY_RELEASE_LOG(Loading, "addProvisionalFrameProxy: provisionalFrame=%p", &provisionalFrame);
+    WEBPROCESSPROXY_RELEASE_LOG(Loading, "addRemotePageProxy: remotePage=%p", &remotePage);
 
     ASSERT(!m_isInProcessCache);
-    ASSERT(!m_provisionalFrames.contains(provisionalFrame));
+    ASSERT(!m_remotePages.contains(remotePage));
+    m_remotePages.add(remotePage);
     markProcessAsRecentlyUsed();
-    m_provisionalFrames.add(provisionalFrame);
-    updateRegistrationWithDataStore();
 }
 
-void WebProcessProxy::removeProvisionalFrameProxy(ProvisionalFrameProxy& provisionalFrame)
+void WebProcessProxy::removeRemotePageProxy(RemotePageProxy& remotePage)
 {
-    WEBPROCESSPROXY_RELEASE_LOG(Loading, "removeProvisionalFrameProxy: provisionalFrame=%p", &provisionalFrame);
-
-    ASSERT(m_provisionalFrames.contains(provisionalFrame));
-    m_provisionalFrames.remove(provisionalFrame);
-    updateRegistrationWithDataStore();
-    if (m_provisionalFrames.isEmptyIgnoringNullReferences())
+    WEBPROCESSPROXY_RELEASE_LOG(Loading, "removeRemotePageProxy: remotePage=%p", &remotePage);
+    m_remotePages.remove(remotePage);
+    if (m_remotePages.isEmptyIgnoringNullReferences())
         maybeShutDown();
 }
 
-void WebProcessProxy::provisionalFrameCommitted(WebFrameProxy& frame)
-{
-    ASSERT(!m_frameMap.contains(frame.frameID()));
-    m_frameMap.set(frame.frameID(), WeakPtr { frame });
-}
-
-void WebProcessProxy::removeFrameWithRemoteFrameProcess(WebFrameProxy& frame)
-{
-    ASSERT(m_frameMap.contains(frame.frameID()));
-    m_frameMap.remove(frame.frameID());
-}
-
 void WebProcessProxy::getLaunchOptions(ProcessLauncher::LaunchOptions& launchOptions)
 {
     launchOptions.processType = ProcessLauncher::ProcessType::Web;
@@ -1448,10 +1432,9 @@ void WebProcessProxy::maybeShutDown()
 bool WebProcessProxy::canTerminateAuxiliaryProcess()
 {
     if (!m_pageMap.isEmpty()
-        || !m_frameMap.isEmpty()
+        || !m_remotePages.isEmptyIgnoringNullReferences()
         || !m_suspendedPages.isEmptyIgnoringNullReferences()
         || !m_provisionalPages.isEmptyIgnoringNullReferences()
-        || !m_provisionalFrames.isEmptyIgnoringNullReferences()
         || m_isInProcessCache
         || m_shutdownPreventingScopeCounter.value()) {
         WEBPROCESSPROXY_RELEASE_LOG(Process, "canTerminateAuxiliaryProcess: returns false (pageCount=%u, provisionalPageCount=%u, suspendedPageCount=%u, m_isInProcessCache=%d, m_shutdownPreventingScopeCounter=%lu)", m_pageMap.size(), m_provisionalPages.computeSize(), m_suspendedPages.computeSize(), m_isInProcessCache, m_shutdownPreventingScopeCounter.value());

Source/WebKit/UIProcess/WebProcessProxy.h

@@ -98,8 +98,8 @@ namespace WebKit {
 class AudioSessionRoutingArbitratorProxy;
 class ObjCObjectGraph;
 class PageClient;
-class ProvisionalFrameProxy;
 class ProvisionalPageProxy;
+class RemotePageProxy;
 class SuspendedPageProxy;
 class UserMediaCaptureManagerProxy;
 class VisitedLinkStore;
@@ -146,7 +146,6 @@ class WebProcessProxy : public AuxiliaryProcessProxy, private ProcessThrottlerCl
 public:
     using WebPageProxyMap = HashMap<WebPageProxyIdentifier, WeakPtr<WebPageProxy>>;
     using UserInitiatedActionByAuthorizationTokenMap = HashMap<WTF::UUID, RefPtr<API::UserInitiatedAction>>;
-    typedef HashMap<WebCore::FrameIdentifier, WeakPtr<WebFrameProxy>> WebFrameProxyMap;
     typedef HashMap<uint64_t, RefPtr<API::UserInitiatedAction>> UserInitiatedActionMap;
 
     enum class IsPrewarmed : bool { No, Yes };
@@ -210,10 +209,8 @@ class WebProcessProxy : public AuxiliaryProcessProxy, private ProcessThrottlerCl
 
     void addProvisionalPageProxy(ProvisionalPageProxy&);
     void removeProvisionalPageProxy(ProvisionalPageProxy&);
-    void addProvisionalFrameProxy(ProvisionalFrameProxy&);
-    void removeProvisionalFrameProxy(ProvisionalFrameProxy&);
-    void provisionalFrameCommitted(WebFrameProxy&);
-    void removeFrameWithRemoteFrameProcess(WebFrameProxy&);
+    void addRemotePageProxy(RemotePageProxy&);
+    void removeRemotePageProxy(RemotePageProxy&);
 
     Vector<RefPtr<WebPageProxy>> pages() const;
     unsigned pageCount() const { return m_pageMap.size(); }
@@ -665,9 +662,8 @@ class WebProcessProxy : public AuxiliaryProcessProxy, private ProcessThrottlerCl
     HashSet<String> m_previouslyApprovedFilePaths;
 
     WebPageProxyMap m_pageMap;
-    WebFrameProxyMap m_frameMap;
+    WeakHashSet<RemotePageProxy> m_remotePages;
     WeakHashSet<ProvisionalPageProxy> m_provisionalPages;
-    WeakHashSet<ProvisionalFrameProxy> m_provisionalFrames;
     WeakHashSet<SuspendedPageProxy> m_suspendedPages;
     UserInitiatedActionMap m_userInitiatedActionMap;
     UserInitiatedActionByAuthorizationTokenMap m_userInitiatedActionByAuthorizationTokenMap;

Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm

@@ -60,6 +60,11 @@ static void enableWindowOpenPSON(WKWebViewConfiguration *configuration)
     }
 }
 
+static bool processStillRunning(pid_t pid)
+{
+    return !kill(pid, 0);
+}
+
 static bool frameTreesMatch(_WKFrameTreeNode *actualRoot, const ExpectedFrameTree& expectedRoot)
 {
     WKFrameInfo *info = actualRoot.info;
@@ -977,10 +982,11 @@ HTTPServer server({
     EXPECT_WK_STREQ([webView _test_waitForAlert], "done");
 
     __block bool done { false };
+    __block pid_t childFramePid { 0 };
     [webView _frames:^(_WKFrameTreeNode *mainFrame) {
         _WKFrameTreeNode *childFrame = mainFrame.childFrames.firstObject;
         pid_t mainFramePid = mainFrame.info._processIdentifier;
-        pid_t childFramePid = childFrame.info._processIdentifier;
+        childFramePid = childFrame.info._processIdentifier;
         EXPECT_NE(mainFramePid, 0);
         EXPECT_NE(childFramePid, 0);
         EXPECT_EQ(mainFramePid, childFramePid);
@@ -995,6 +1001,10 @@ HTTPServer server({
             { { "https://example.com"_s } }
         }
     });
+
+    // FIXME: We ought to be able to use processStillRunning to verify that childFramePid stops running at this point,
+    // but it is currently kept running, probably because we don't delete the WebFrameProxy's m_remotePageProxy when
+    // navigating to the main frame's process.
 }
 
 TEST(SiteIsolation, ChildBeingNavigatedToSameDomainByParent)
@@ -1337,4 +1347,35 @@ HTTPServer server({
 }
 #endif
 
+TEST(SiteIsolation, ShutDownFrameProcessesAfterNavigation)
+{
+    HTTPServer server({
+        { "/example"_s, { "<iframe src='https://webkit.org/webkit'></iframe>"_s } },
+        { "/webkit"_s, { "hello"_s } },
+        { "/apple"_s, { "hello"_s } }
+    }, HTTPServer::Protocol::HttpsProxy);
+
+    auto navigationDelegate = adoptNS([TestNavigationDelegate new]);
+    [navigationDelegate allowAnyTLSCertificate];
+    auto configuration = server.httpsProxyConfiguration();
+    enableSiteIsolation(configuration);
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:configuration]);
+    webView.get().navigationDelegate = navigationDelegate.get();
+
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://example.com/example"]]];
+    [navigationDelegate waitForDidFinishNavigation];
+    pid_t iframePID = findFramePID(frameTrees(webView.get()).get(), FrameType::Remote);
+    checkFrameTreesInProcesses(webView.get(), {
+        { "https://example.com"_s, { { RemoteFrame } } },
+        { RemoteFrame, { { "https://webkit.org"_s } } }
+    });
+
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://apple.com/apple"]]];
+    [navigationDelegate waitForDidFinishNavigation];
+    checkFrameTreesInProcesses(webView.get(), { { "https://apple.com"_s } });
+
+    while (processStillRunning(iframePID))
+        Util::spinRunLoop();
+}
+
 }