[PerformanceManager] Fix kDisableTabDiscarding feature interactions [chromium/src : main]
0 views
Skip to first unread message
Alex Attar (Gerrit)
Feb 23, 2026, 9:53:57 AMFeb 23
to Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org
Attention needed from Joe Mason
Alex Attar added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Joe Mason
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Mason (Gerrit)
Feb 23, 2026, 11:28:16 AMFeb 23
to Alex Attar, Olivier Li, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org
Attention needed from Alex Attar
Joe Mason voted and added 2 comments![]( "Open in Gerrit")
Votes added by Joe Mason
| Code-Review | +1 |
2 comments
Patchset-level comments
Joe Mason . resolved
lgtm with nits
File chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
Line 74, Patchset 2 (Latest):
default:Joe Mason . unresolved
Nit: better to list all reasons and without a `default`, so that the compiler will catch missing cases if anyone adds a LifecycleUnitDiscardReason. Optionally add `NOTREACHED()` after the switch to also catch run-time failures, like if a bad cast or cosmic ray calls this function with an int that's not actually an enum member.
Related details
Attention is currently required from:
- Alex Attar
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. |
Alex Attar (Gerrit)
Feb 24, 2026, 3:23:13 PMFeb 24
to Olivier Li, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org
Alex Attar added 1 comment![]( "Open in Gerrit")
File chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
Nit: better to list all reasons and without a `default`, so that the compiler will catch missing cases if anyone adds a LifecycleUnitDiscardReason. Optionally add `NOTREACHED()` after the switch to also catch run-time failures, like if a bad cast or cosmic ray calls this function with an int that's not actually an enum member.
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Attar (Gerrit)
Feb 24, 2026, 4:57:52 PMFeb 24
to Olivier Li, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org
Alex Attar voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
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. |
Chromium LUCI CQ (Gerrit)
Feb 24, 2026, 5:01:46 PMFeb 24
to Alex Attar, Olivier Li, chromium...@chromium.org, chrome-gr...@chromium.org, performance-m...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
Insertions: 9, Deletions: 1.
@@ -14,6 +14,7 @@
#include "base/logging.h"
#include "base/memory/raw_ptr.h"
#include "base/metrics/histogram_functions.h"
+#include "base/notreached.h"
#include "base/process/process_metrics.h"
#include "build/build_config.h"
#include "chrome/browser/devtools/devtools_window.h"
@@ -71,9 +72,16 @@
// The discard was explicitly requested by the Memory Saver feature,
// and we should respect the user's settings.
return false;
- default:
+ case LifecycleUnitDiscardReason::URGENT:
+ // Block urgent memory pressure discards during this experiment.
+ return true;
+ case LifecycleUnitDiscardReason::SUGGESTED:
+ // The discard is an optional suggestion to free up resources, which
+ // we block while the disable discarding experiment is active.
return true;
}
+
+ NOTREACHED();
}
} // namespace
```
Change information
Commit message:
[PerformanceManager] Fix kDisableTabDiscarding feature interactions
This CL fixes how the existing kDisableTabDiscarding experiment
interacts with memory policies:
* Refactors `TabLifecycleUnit::Discard` to allow `PROACTIVE` discards, ensuring the user's Memory Saver preference is respected.
* Guards the instantiation of `UrgentPageDiscardingPolicy` so it is not added to the graph when the disable flag is active.
Change-Id: Ie54b7cd0771afccc79a0c1fb10b87299e36da5d3
Reviewed-by: Joe Mason <joenot...@google.com>
Commit-Queue: Alex Attar <aat...@google.com>
Cr-Commit-Position: refs/heads/main@{#1589692}
Files:
- M chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc
- M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
Change size: M
Delta: 2 files changed, 45 insertions(+), 16 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joe Mason
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages