Skip to content

[Site Isolation] Only update page main frame's process in ProvisionalPageProxy::didCommitLoadForFrame when committing the main frame itself#64310

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
basuke:eng/basuke/314129
May 6, 2026

Conversation

@basuke
Copy link
Copy Markdown
Contributor

@basuke basuke commented May 5, 2026

64bbad6

[Site Isolation] Only update page main frame's process in ProvisionalPageProxy::didCommitLoadForFrame when committing the main frame itself
https://bugs.webkit.org/show_bug.cgi?id=314129
rdar://176305054

Reviewed by Sihui Liu.

In ProvisionalPageProxy::didCommitLoadForFrame, the Site Isolation
block computes pageMainFrameProcess and, if the frame process has
changed, assigns the provisional page's FrameProcess to the page's
main frame. This is only correct when the provisional commit is for
the page's main frame itself. Under Site Isolation, a provisional
page may commit for a different frame (e.g., during a multi-process
BFCache restoration), in which case overwriting the page main frame's
FrameProcess with the provisional's FrameProcess leaves the real main
frame pointing to the wrong process.

Move the pageMainFrame == m_mainFrame guard into the outer condition
so the whole block is only entered when committing the page's main
frame. A pre-existing redundant check on the
m_deferredRemoteTransitionSite assignment becomes unnecessary and is
simplified accordingly.

* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::didCommitLoadForFrame):

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

db3814a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 win ⏳ 🛠 ios-apple
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 win-tests ⏳ 🛠 mac-apple
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac ✅ 🧪 api-wpe ⏳ 🛠 vision-apple
🧪 ios-wk2-wpt 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
🧪 api-ios ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 🧪 gtk-wk2
✅ 🛠 vision 🧪 mac-AS-debug-wk2 🧪 api-gtk
✅ 🛠 🧪 merge 🛠 vision-sim ✅ 🧪 mac-wk2-stress 🛠 playstation
🧪 vision-wk2 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
🛠 tv-sim
🛠 watch
🛠 watch-sim

@basuke basuke requested a review from cdumez as a code owner May 5, 2026 23:43
@basuke basuke self-assigned this May 5, 2026
@basuke basuke added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label May 5, 2026
@basuke basuke requested review from rniwa and szewai May 5, 2026 23:54
@@ -496,7 +496,7 @@ void ProvisionalPageProxy::didCommitLoadForFrame(IPC::Connection& connection, Fr
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move pageMainFrame == m_mainFrame to if (page && protect(page->preferences())->siteIsolationEnabled() && pageMainFrame) { instead, if we'd like to do the operation when main frame has not changed?

@basuke basuke changed the title Only update page main frame's process in ProvisionalPageProxy::didCommitLoadForFrame when committing the main frame itself [Site Isolation] Only update page main frame's process in ProvisionalPageProxy::didCommitLoadForFrame when committing the main frame itself May 6, 2026
@basuke basuke force-pushed the eng/basuke/314129 branch from bb2b000 to db3814a Compare May 6, 2026 20:45
@basuke basuke requested a review from szewai May 6, 2026 20:45
@basuke basuke added the merge-queue Applied to send a pull request to merge-queue label May 6, 2026
…PageProxy::didCommitLoadForFrame when committing the main frame itself

https://bugs.webkit.org/show_bug.cgi?id=314129
rdar://176305054

Reviewed by Sihui Liu.

In ProvisionalPageProxy::didCommitLoadForFrame, the Site Isolation
block computes pageMainFrameProcess and, if the frame process has
changed, assigns the provisional page's FrameProcess to the page's
main frame. This is only correct when the provisional commit is for
the page's main frame itself. Under Site Isolation, a provisional
page may commit for a different frame (e.g., during a multi-process
BFCache restoration), in which case overwriting the page main frame's
FrameProcess with the provisional's FrameProcess leaves the real main
frame pointing to the wrong process.

Move the pageMainFrame == m_mainFrame guard into the outer condition
so the whole block is only entered when committing the page's main
frame. A pre-existing redundant check on the
m_deferredRemoteTransitionSite assignment becomes unnecessary and is
simplified accordingly.

* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::didCommitLoadForFrame):

Canonical link: https://commits.webkit.org/312737@main
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 312737@main (64bbad6): https://commits.webkit.org/312737@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 6, 2026
@webkit-commit-queue webkit-commit-queue merged commit 64bbad6 into WebKit:main May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants