Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bcee9e49e4dce52384ea23f7d7ab8afee3482367
      
https://github.com/WebKit/WebKit/commit/bcee9e49e4dce52384ea23f7d7ab8afee3482367
  Author: Sihui Liu <[email protected]>
  Date:   2026-08-26 (Wed, 26 Aug 2026)

  Changed paths:
    M Source/WebKit/UIProcess/API/APINavigation.cpp
    M Source/WebKit/UIProcess/API/APINavigation.h
    M Source/WebKit/UIProcess/BrowsingContextGroup.cpp
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebProcessCache.cpp

  Log Message:
  -----------
  [Site Isolation] Shared process outlives every frame using it because 
API::Navigation keeps a reference
https://bugs.webkit.org/show_bug.cgi?id=322446
rdar://185728345

Reviewed by Per Arne Vollan.

With the shared process enabled, removing the last frame hosted in the shared 
process leaves that process holding a page
with no frames in it, and this could cause API test SiteIsolation.RemoveFrames 
to fail -- the test still finds a remote
frame tree after both example.com frames are gone. This is because 
BrowsingContextGroup tracks the shared FrameProcess
weakly and only disconnects the process's RemotePageProxy from ~FrameProcess, 
so any strong reference outliving the
frames keeps the page, and the process, alive.

The root cause is API::Navigation takes a strong reference to the shared 
FrameProcess via setPendingSharedProcess(). The
reference was added to bridge the gap between the point a process is picked for 
a navigation and the point a
ProvisionalFrameProxy or ProvisionalPageProxy takes ownership of it (see 
301070@main), with WebFrameProxy taking over on
commit through takeFrameProcess(). However, API::Navigation never releases the 
reference, so the FrameProcess ends up
living as long as the API::Navigation, which outlives the frames that use the 
process.

To fix this, this patch captures the strong reference in the completion handler 
passed to
BrowsingContextGroup::sharedProcessForSite() instead. The reference is released 
once continueWithProcessForNavigation()
returns, by which point either a ProvisionalFrameProxy or a 
ProvisionalPageProxy owns the FrameProcess, or the
navigation has been abandoned and nobody needs it. It has to cover that whole 
call rather than stop at the handoff,
because prepareForProvisionalLoadInProcess() destroys the previous provisional 
frame before calling
ensureProcessForSite(), which would otherwise find a null m_sharedProcess for a 
process that still reports
isSharedProcess().

The fix above exposes another bug, since a shared process can now be torn down 
and recreated within a page's lifetime:
removeFrameProcess() cleared m_sharedProcess and m_sharedProcessSites but not 
m_pagesInSharedProcess, which records the
pages already injected into the shared process. BCG would therefore skip 
injecting a page into the newly created shared
process even though that process does not have the page, and the page's 
LoadRequest would go unhandled.

It also makes SiteIsolation.SharedProcessBasicWebProcessCacheCrash fail on the 
bots, where the child frames ended up in
a different process after the second navigation. Now that the shared process is 
really released, it is put in the
WebProcessCache and taken back out within the same navigation, and 
addProcessIfPossible() only adds a process to
m_sharedProcessesPerSite once a responsiveness ping to that process comes back. 
takeSharedProcess() only looked at that
map, so whenever the next navigation picked a process before the ping returned, 
it started a second process for the same
site instead. Let takeSharedProcess() also return a process still sitting in 
m_pendingAddRequests; removeProcess()
already treats those entries as part of the cache, and taking the process back 
cancels the pending add, since the ping
handler drops a request that is no longer there. takeProcess() has the same 
gap, but is left alone here.

SiteIsolation.RemoveFrames covers this once SiteIsolationSharedProcessEnabled 
is on, and
SiteIsolation.SharedProcessBasicWebProcessCacheCrash covers the rebuild path. 
The preference is off by default, so no
test change is included.

* Source/WebKit/UIProcess/API/APINavigation.cpp:
(API::Navigation::setPendingSharedProcess): Deleted.
* Source/WebKit/UIProcess/API/APINavigation.h:
* Source/WebKit/UIProcess/BrowsingContextGroup.cpp:
(WebKit::BrowsingContextGroup::removeFrameProcess):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::receivedNavigationActionPolicyDecision):
* Source/WebKit/UIProcess/WebProcessCache.cpp:
(WebKit::WebProcessCache::takeSharedProcess):

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



To unsubscribe from these emails, change your notification settings at 
https://github.com/WebKit/WebKit/settings/notifications

Reply via email to