WebLocks: Evict BFCached pages on Web Lock contention [chromium/src : main]

[Anna Sato (Gerrit)]

Attention needed from Fergal Daly

Anna Sato voted and added 1 comment

Votes added by Anna Sato

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Anna Sato . resolved

This CL is for the weblock BFCache eviction-on-contention logic. PTAL!

Open in Gerrit

Related details

Attention is currently required from:

• Fergal Daly

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6541d1adea0cf0de11b8adb36c7e69cec602c512

Gerrit-Change-Number: 6918376

Gerrit-PatchSet: 9

Gerrit-Owner: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Fergal Daly <fer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: bfcach...@chromium.org <bfcach...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: devtools...@chromium.org

Gerrit-Attention: Fergal Daly <fer...@chromium.org>

Gerrit-Comment-Date: Mon, 22 Sep 2025 02:01:48 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Fergal Daly (Gerrit)]

Attention needed from Anna Sato

Fergal Daly added 2 comments

File content/browser/back_forward_cache_features_browsertest.cc

Line 5082, Patchset 9 (Latest):IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, WebLocksNotCached) {

Fergal Daly . unresolved

This seems like it's covered by WPTs now, we can delete it.

File third_party/blink/web_tests/external/wpt/performance-timeline/not-restored-reasons/performance-navigation-timing-lock.https.tentative.window-expected.txt

Line 2, Patchset 9 (Latest):[FAIL] Ensure that if WebLock is held upon entering bfcache, it cannot enter bfcache and gets reported.

Fergal Daly . unresolved

I think we should delete these tests (as long as there is a new test that covers the same case using contention).

Open in Gerrit

Related details

Attention is currently required from:

• Anna Sato

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6541d1adea0cf0de11b8adb36c7e69cec602c512

Gerrit-Change-Number: 6918376

Gerrit-PatchSet: 9

Gerrit-Owner: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Fergal Daly <fer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Code Review Nudger <android-build...@prod.google.com>

Gerrit-CC: bfcach...@chromium.org <bfcach...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: devtools...@chromium.org

Gerrit-Attention: Anna Sato <anna...@chromium.org>

Gerrit-Comment-Date: Thu, 25 Sep 2025 04:00:29 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Anna Sato (Gerrit)]

Attention needed from Fergal Daly

Anna Sato added 3 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Anna Sato . resolved

Thanks for the review!

File content/browser/back_forward_cache_features_browsertest.cc

Line 5082, Patchset 9:IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, WebLocksNotCached) {

Fergal Daly . resolved

This seems like it's covered by WPTs now, we can delete it.

Anna Sato

Deleted.

File third_party/blink/web_tests/external/wpt/performance-timeline/not-restored-reasons/performance-navigation-timing-lock.https.tentative.window-expected.txt

Line 2, Patchset 9:[FAIL] Ensure that if WebLock is held upon entering bfcache, it cannot enter bfcache and gets reported.

Fergal Daly . unresolved

I think we should delete these tests (as long as there is a new test that covers the same case using contention).

Anna Sato

Removed tests that check BfCache ineligibility for WebLock, while leaving two of them that allow pages to enter BFCache (should we remove them as well?):

• [third_party/blink/web_tests/external/wpt/web-locks/bfcache/abort.tentative.https.html](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/web-locks/bfcache/abort.tentative.https.html;bpv=0;bpt=0)
• [third_party/blink/web_tests/external/wpt/web-locks/bfcache/release.tentative.https.html](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/web-locks/bfcache/release.tentative.https.html;bpv=0;bpt=0)

Also added a new test `contention.window.js` in external.

Open in Gerrit

Related details

Attention is currently required from:

• Fergal Daly

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6541d1adea0cf0de11b8adb36c7e69cec602c512

Gerrit-Change-Number: 6918376

Gerrit-PatchSet: 10

Gerrit-Owner: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Fergal Daly <fer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Code Review Nudger <android-build...@prod.google.com>

Gerrit-CC: bfcach...@chromium.org <bfcach...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: devtools...@chromium.org

Gerrit-Attention: Fergal Daly <fer...@chromium.org>

Gerrit-Comment-Date: Tue, 30 Sep 2025 06:37:45 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Fergal Daly <fer...@chromium.org>

[Fergal Daly (Gerrit)]

Attention needed from Anna Sato

Fergal Daly added 2 comments

File third_party/blink/web_tests/external/wpt/web-locks/bfcache/held.tentative.https.html

Line 27, Patchset 10 (Parent): }, "A held lock on a worker must prevent bfcache");

Fergal Daly . unresolved

Deleting these removes coverage for workers holding locks.

I think what you should be doing is updating these to all use contention instead of only just holding the lock.

A similar note applies to the other file.

File third_party/blink/web_tests/external/wpt/web-locks/bfcache/sharedworker-multiple.tentative.https.html

Line 1, Patchset 10 (Parent):<!DOCTYPE html>

Fergal Daly . unresolved

why is it OK to delete this?

Open in Gerrit

Related details

Attention is currently required from:

• Anna Sato

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I6541d1adea0cf0de11b8adb36c7e69cec602c512

Gerrit-Change-Number: 6918376

Gerrit-PatchSet: 10

Gerrit-Owner: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Anna Sato <anna...@chromium.org>

Gerrit-Reviewer: Fergal Daly <fer...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Code Review Nudger <android-build...@prod.google.com>

Gerrit-CC: bfcach...@chromium.org <bfcach...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: devtools...@chromium.org

Gerrit-Attention: Anna Sato <anna...@chromium.org>

Gerrit-Comment-Date: Fri, 03 Oct 2025 07:21:20 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Anna Sato (Gerrit)]

Anna Sato abandoned this change

Related details

Attention set is empty

Submit Requirements:

• Code-Coverage

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: abandon