Skip to content

Fix some pages with COOP: same-origin header getting a same-site process from cache#45664

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
zakariaridouh:eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache
Jun 5, 2025
Merged

Fix some pages with COOP: same-origin header getting a same-site process from cache#45664
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
zakariaridouh:eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache

Conversation

@zakariaridouh
Copy link
Contributor

@zakariaridouh zakariaridouh commented May 20, 2025 •
edited by webkit-early-warning-system
Loading

5412065

Fix some pages with COOP: same-origin header getting a same-site process from cache
https://bugs.webkit.org/show_bug.cgi?id=293321
rdar://150643925

Reviewed by Chris Dumez.

We have an issue in our process selection system for loading pages,
where if we have a suspended process of a certain site, we sometimes end
up giving that process to a new site that is same domain, but different
origin. If this site also has a Cross-Origin-Opener-Policy: same-origin
header, we end up in a process swapping loop where we keep trying to get
a new process to load in to satisfy our policy, but keep getting handed
a process that we can't load in.

Fix this by disqualifying COOP + COEP process swaps from eligibility in
participating in process cache selection.

Also, added an API test to verify this fixed logic.

* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::findReusableSuspendedPageProcess):
(WebKit::SuspendedPageProxy::create):
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendCurrentPageIfPossible):
(WebKit::WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForSite):
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
((ProcessSwap, MultitabCOOPSwapSameOriginProcessPool)):

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

0f9f28b

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

@zakariaridouh zakariaridouh requested a review from cdumez as a code owner May 20, 2025 22:21
@zakariaridouh zakariaridouh self-assigned this May 20, 2025
@zakariaridouh zakariaridouh added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label May 20, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 2cdba2b)

Details

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

@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from 2cdba2b to 4883b51 Compare May 22, 2025 08:36
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 22, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 4883b51)

Details

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

cdumez
cdumez reviewed May 22, 2025
View reviewed changes
cdumez
cdumez reviewed May 22, 2025
View reviewed changes
@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from 4883b51 to 7acf47d Compare June 3, 2025 01:11
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 3, 2025 •
edited
Loading

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

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

achristensen07
achristensen07 requested changes Jun 3, 2025
View reviewed changes
charliewolfe
charliewolfe reviewed Jun 3, 2025
View reviewed changes
Copy link
Member

@charliewolfe charliewolfe left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@cdumez How did this work before? Or hassame-origin never worked since COOP/COEP was implemented? Radar indicates this broke recently.

@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from 7acf47d to 6036b58 Compare June 4, 2025 02:14
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 4, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 6036b58)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

cdumez
cdumez requested changes Jun 5, 2025
View reviewed changes
@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from 6036b58 to a081244 Compare June 5, 2025 20:25
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 5, 2025 •
edited
Loading

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

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ❌ 🛠 ios   🛠 mac   🛠 wpe   🛠 win
✅ 🧪 bindings   🛠 ios-sim   🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
  🧪 api-ios   🧪 mac-wk2   🛠 gtk
  🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
  🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
⏳ 🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
❌ 🛠 tv   🛠 mac-safer-cpp
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from a081244 to f829edf Compare June 5, 2025 20:28
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 5, 2025 •
edited
Loading

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

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ✅ 🧪 api-mac   🧪 api-wpe
⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2 ⏳ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress   🧪 api-gtk
  🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv   🛠 mac-safer-cpp
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@zakariaridouh zakariaridouh requested a review from cdumez June 5, 2025 20:35
cdumez
cdumez approved these changes Jun 5, 2025
View reviewed changes
@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from f829edf to c46311f Compare June 5, 2025 20:43
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 5, 2025 •
edited
Loading

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

Details

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

@zakariaridouh zakariaridouh added the merge-queue Applied to send a pull request to merge-queue label Jun 5, 2025
@cdumez cdumez added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Jun 5, 2025
cdumez
cdumez reviewed Jun 5, 2025
View reviewed changes
@zakariaridouh zakariaridouh removed the merging-blocked Applied to prevent a change from being merged label Jun 5, 2025
@zakariaridouh zakariaridouh force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from c46311f to 0f9f28b Compare June 5, 2025 20:48
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 5, 2025 •
edited
Loading

EWS run on current version of this PR (hash 0f9f28b)

Details

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

@zakariaridouh zakariaridouh added the merge-queue Applied to send a pull request to merge-queue label Jun 5, 2025
@zakariaridouh
Fix some pages with COOP: same-origin header getting a same-site proc…
5412065 
…ess from cache

https://bugs.webkit.org/show_bug.cgi?id=293321
rdar://150643925

Reviewed by Chris Dumez.

We have an issue in our process selection system for loading pages,
where if we have a suspended process of a certain site, we sometimes end
up giving that process to a new site that is same domain, but different
origin. If this site also has a Cross-Origin-Opener-Policy: same-origin
header, we end up in a process swapping loop where we keep trying to get
a new process to load in to satisfy our policy, but keep getting handed
a process that we can't load in.

Fix this by disqualifying COOP + COEP process swaps from eligibility in
participating in process cache selection.

Also, added an API test to verify this fixed logic.

* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::findReusableSuspendedPageProcess):
(WebKit::SuspendedPageProxy::create):
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendCurrentPageIfPossible):
(WebKit::WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForSite):
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
((ProcessSwap, MultitabCOOPSwapSameOriginProcessPool)):

Canonical link: https://commits.webkit.org/295891@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch from 0f9f28b to 5412065 Compare June 5, 2025 21:24
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Jun 5, 2025

Committed 295891@main (5412065): https://commits.webkit.org/295891@main

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

@webkit-commit-queue webkit-commit-queue merged commit 5412065 into WebKit:main Jun 5, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 5, 2025
@zakariaridouh zakariaridouh deleted the eng/Fix-some-pages-with-COOP-same-origin-header-getting-a-same-site-process-from-cache branch June 5, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdumez cdumez cdumez approved these changes

@charliewolfe charliewolfe Awaiting requested review from charliewolfe

@achristensen07 achristensen07 Awaiting requested review from achristensen07

Assignees

@zakariaridouh zakariaridouh

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@zakariaridouh @webkit-early-warning-system @webkit-commit-queue @cdumez @achristensen07 @charliewolfe