Lingqi Chi (Gerrit)

unread,

Nov 26, 2025, 12:20:08 AM (4 days ago) Nov 26

to Rubber Stamper, AyeAye, Vovo Yang, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org

Attention needed from Joe Mason and Vovo Yang

Lingqi Chi voted

Auto-Submit	+1
Owners-Override	+1

Open in Gerrit

Related details

Attention is currently required from:

Joe Mason
Vovo Yang

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rubber Stamper (Gerrit)

unread,

Nov 26, 2025, 12:20:20 AM (4 days ago) Nov 26

to Lingqi Chi, AyeAye, Vovo Yang, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org

Attention needed from Joe Mason and Vovo Yang

Rubber Stamper voted

Bot-Commit	+1
Commit-Queue	+2

Open in Gerrit

Related details

Attention is currently required from:

Joe Mason
Vovo Yang

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Nov 26, 2025, 12:20:44 AM (4 days ago) Nov 26

to Lingqi Chi, Rubber Stamper, AyeAye, Vovo Yang, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Revert "[resource coordinator] TabManager discards via PageDiscardingHelper"

This reverts commit 0d54250ca6a72b86a12cc0e50d87bd355648e373.

Reason for revert: newly added test of DiscardEligibilityPolicyBrowserTest.CannotDiscardVisibleInSplit
failing https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome/55030/overview

Original change's description:
> [resource coordinator] TabManager discards via PageDiscardingHelper
>
> The TabManager and TabLifecycleUnit code to select the page to discard
> should be replaced by PageDiscardingHelper::DiscardAPage.
>
> Modified some browser tests because of the difference between
> TabLifecycleUnit::CanDiscard and DiscardEligibilityPolicy::CanDiscard.
> Some DiscardEligibilityPolicy::CanDiscard differences:
> 1. It always returns eligible for external discards. So a few tests are
>    modified to use urgent discard instead of external discard.
> 2. ScopedSetClocksForTesting doesn't work. Passing
>    minimum_time_in_background_to_discard = 0 instead.
> 3. Discarding a tab with file:// url is not eligible.
>
> Some subtests of TabLifecycleUnitTest are removed:
>
> DiscardEligibilityPolicyTest::TestCannotDiscardVisiblePage replaces
> SetFocused and SetFocusedSplit in TabLifecycleUnitTest. In
> DiscardEligibilityPolicy, all visible pages are protected instead of
> only the focused page.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardPageWithoutMainFrame
> replaces TabLifecycleUnitTest::CannotDiscardCrashed.
> DiscardEligibilityPolicy::CanDiscard returns kDisallowed for tab without
> a main frame, which includes the crashed tab case.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardVisiblePage replaces
> TabLifecycleUnitTest::CannotDiscardActive. In DiscardEligibilityPolicy,
> all visible pages are protected.
>
> TabLifecycleUnit::CanDiscard returns false when the discard count is not
> zero. It's replaced by the DiscardAttemptMarker mechanism in
> DiscardEligibilityPolicy. So TabLifecycleUnitTest::UrgentDiscardProtections
> is replaced by
> DiscardEligibilityPolicyTest::TestCannotDiscardPageWithDiscardAttemptMarker.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardPageCapturingVideo
> replaces TabLifecycleUnitTest::CannotDiscardVideoCapture.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardPageWithFormInteractions
> replaces TabLifecycleUnitTest::CannotDiscardHasFormInteractions.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardPageCapturingDisplay
> replaces TabLifecycleUnitTest::CannotDiscardDesktopCapture.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardRecentlyAudiblePage
> replaces TabLifecycleUnitTest::CannotDiscardRecentlyAudible.
>
> DiscardEligibilityPolicyTest::TestCanDiscardNeverAudiblePage replaces
> TabLifecycleUnitTest::CanDiscardNeverAudibleTab.
>
> DiscardEligibilityPolicyTest::TestCannotDiscardPdf replaces
> TabLifecycleUnitTest::CannotDiscardPDF.
>
> DiscardEligibilityPolicyBrowserTest::CannotDiscardVisibleInSplit
> replaces TabLifecycleUnitTest::SetFocusedSplit.
>
> Bug: 394889323
> Change-Id: Ie433615347033d4f0563d7e44fc53a67a29e7c14
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7129640
> Commit-Queue: Vovo Yang <vo...@chromium.org>
> Reviewed-by: Joe Mason <joenot...@google.com>
> Cr-Commit-Position: refs/heads/main@{#1550197}

Bug: 394889323

No-Presubmit: true

No-Tree-Checks: true

No-Try: true

Change-Id: I7ce0d4275e4a7011c7b606a19bc0d82271ada30a

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7206488

Auto-Submit: Lingqi Chi <lin...@chromium.org>

Owners-Override: Lingqi Chi <lin...@chromium.org>

Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>

Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>

Cr-Commit-Position: refs/heads/main@{#1550243}

Files:

M chrome/browser/BUILD.gn
M chrome/browser/performance_manager/policies/cannot_discard_reason.cc
M chrome/browser/performance_manager/policies/cannot_discard_reason.h
M chrome/browser/performance_manager/policies/discard_eligibility_policy.cc
D chrome/browser/performance_manager/policies/discard_eligibility_policy_browsertest.cc
M chrome/browser/performance_manager/policies/discard_eligibility_policy_unittest.cc
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper.h
M chrome/browser/performance_manager/policies/page_discarding_helper_browsertest.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/performance_manager/test_support/page_discarding_utils.cc
M chrome/browser/performance_manager/test_support/page_discarding_utils.h
M chrome/browser/resource_coordinator/BUILD.gn
A chrome/browser/resource_coordinator/decision_details.cc
A chrome/browser/resource_coordinator/decision_details.h
A chrome/browser/resource_coordinator/decision_details_unittest.cc
M chrome/browser/resource_coordinator/lifecycle_unit.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M chrome/browser/resource_coordinator/tab_lifecycle_unit.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit_source_unittest.cc
M chrome/browser/resource_coordinator/tab_lifecycle_unit_unittest.cc
M chrome/browser/resource_coordinator/tab_manager.cc
M chrome/browser/resource_coordinator/tab_manager.h
M chrome/browser/resource_coordinator/tab_manager_browsertest.cc
M chrome/browser/resource_coordinator/tab_manager_unittest.cc
M chrome/browser/resource_coordinator/test_lifecycle_unit.cc
M chrome/browser/resource_coordinator/test_lifecycle_unit.h
M chrome/test/BUILD.gn

Change size: XL

Delta: 28 files changed, 1989 insertions(+), 765 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: Bot-Commit+1 by Rubber Stamper

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Lingqi Chi (Gerrit)

unread,

Nov 26, 2025, 3:37:31 AM (4 days ago) Nov 26

to Chromium LUCI CQ, Rubber Stamper, AyeAye, Vovo Yang, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org

Lingqi Chi added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Lingqi Chi . resolved

Hey Vovo (and the next gardener), I realized that after reverting your CL, TabManagerTest.DiscardTabsWithOccludedWindow started to fail..

but without reverting your CL, DiscardEligibilityPolicyBrowserTest.CannotDiscardVisibleInSplit will fail.

Sorry for leaving this situation to you. I feel like maybe we have to disable one of the tests?

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Vovo Yang (Gerrit)

unread,

Nov 26, 2025, 12:15:51 PM (4 days ago) Nov 26

to Chromium LUCI CQ, Lingqi Chi, Rubber Stamper, AyeAye, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org

Vovo Yang added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Lingqi Chi . resolved

Hey Vovo (and the next gardener), I realized that after reverting your CL, TabManagerTest.DiscardTabsWithOccludedWindow started to fail..
but without reverting your CL, DiscardEligibilityPolicyBrowserTest.CannotDiscardVisibleInSplit will fail.
Sorry for leaving this situation to you. I feel like maybe we have to disable one of the tests?

Vovo Yang

I am working on relanding [resource coordinator] TabManager discards via PageDiscardingHelper (http://crrev.com/c/7206651) with DiscardEligibilityPolicyBrowserTest.CannotDiscardVisibleInSplit disabled on Chrome OS. I can re-enable DiscardTabsWithOccludedWindow after the reland.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Revert "[resource coordinator] TabManager discards via PageDiscardingHelper" [chromium/src : main]

Lingqi Chi (Gerrit)

Lingqi Chi voted

Related details

Rubber Stamper (Gerrit)

Rubber Stamper voted

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Lingqi Chi (Gerrit)

Lingqi Chi added 1 comment

Related details

Vovo Yang (Gerrit)

Vovo Yang added 1 comment

Related details