Francois Pierre Doray (Gerrit)

unread,

Jul 10, 2025, 3:20:58 PMJul 10

to Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Francois Pierre Doray voted and added 1 comment

Votes added by Francois Pierre Doray

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Francois Pierre Doray . resolved

gab@: Please take a look.

Open in Gerrit

Related details

Attention set is empty

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Jul 11, 2025, 11:28:21 AMJul 11

to Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Gabriel Charette

Francois Pierre Doray added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Francois Pierre Doray . resolved

gab: Please take a look. Thanks.

Open in Gerrit

Related details

Attention is currently required from:

Gabriel Charette

satisfied_requirement

unsatisfied_requirement

open

diffy

Gabriel Charette (Gerrit)

unread,

Jul 14, 2025, 3:52:54 PMJul 14

to Francois Pierre Doray, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Gabriel Charette voted and added 10 comments

Votes added by Gabriel Charette

Code-Review	+1
Owners-Override	+1

10 comments

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Gabriel Charette . resolved

LGTM % comments; owners-override for side-effects

File base/process/process_metrics_apple.mm

Line 240, Patchset 9 (Latest):// ByteCount committed by the system.

Gabriel Charette . unresolved

The type of this method's return value didn't change to ByteCount? Should it or should we restore the old comment?

File base/system/sys_info.h

Line 90, Patchset 9 (Latest): static uint64_t AmountOfVirtualMemory();

Gabriel Charette . unresolved

Why not this one as well? Follow-up? Seems there are more we should update in this file too (disk space bytes, VM bytes, etc.). Submit a go/code-health-rotation if too onerous?

File base/system/sys_info.cc

Line 97, Patchset 9 (Latest): constexpr int64_t kUpperBound2GB = 2 * 1024; // inclusive

Gabriel Charette . unresolved

convert as well? here and below?

File base/win/hardware_check.cc

Line 115, Patchset 1: static constexpr int64_t kMinTotalDiskSpace = 64 * 1024 * 1024;

Francois Pierre Doray . unresolved

Was this supposed to be 4 GiB, instead of 4 MiB?

Gabriel Charette

!! Let's add a "TODO(429140103): This was migrated as-is to 4MiB in ByteCount but the legacy code potentially intended 4GiB, needs investigation." ? And follow-up (i.e. I wouldn't do it in this migration-only CL)?

File chrome/browser/ash/bruschetta/bruschetta_installer_impl_unittest.cc

Line 98, Patchset 9 (Parent): BruschettaInstallerTest() : fake_20gb_memory(20ULL * 1024 * 1024) {}

Gabriel Charette . unresolved

This constructor used to take MiB, was this incorrectly passing 20TiB? Potentially worth fixing separately if I'm reading correctly so the migration remains a no-op (and avoids a revert)?

File chrome/browser/policy/site_isolation_policy_browsertest.cc

Line 287, Patchset 9 (Parent): uint64_t ram_kb) {

Gabriel Charette . unresolved

This was incorrectly named `_kb` but specifying MB (per ScopedAmountOfPhysicalMemoryOverride's constructor)?

Worth fixing the tests and specifying the change in the CL description assuming they pass?

File content/browser/network/network_service_util_internal.cc

Line 25, Patchset 9 (Parent):// Using 1077 rather than 1024 because it helps ensure that devices with

Gabriel Charette . resolved

Drive-by: wow, second place with this magic constant, I'm puzzled as to why exactly 1077, it's just slightly bigger than 105% of 1024.. 🤷🏼‍♂️

File content/browser/renderer_host/navigation_transitions/navigation_transition_config.cc

Line 75, Patchset 9 (Latest):size_t NavigationTransitionConfig::ComputeCacheSizeInBytes() {

Gabriel Charette . unresolved

There's room to keep expanding on some of these methods. Of course, we need to stop at some point on each CL, should we keep a list of methods to refactor in follow-up CLs (for a code health rotation)?

File sandbox/policy/win/sandbox_win_unittest.cc

Line 452, Patchset 9 (Latest):

  constexpr base::ByteCount k65GB = base::MiB(66560);
  constexpr base::ByteCount k33GB = base::MiB(33792);
  constexpr base::ByteCount k17GB = base::MiB(17408);

Gabriel Charette . unresolved

These are actually precise base::GiB multiples of 1024

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Jul 15, 2025, 3:48:48 PMJul 15

to Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Joe Mason

Francois Pierre Doray added 10 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Francois Pierre Doray . resolved

joenotcharles: I trust your code reviews a lot. Could you take a look at this large change before it's landed?

File base/process/process_metrics_apple.mm

Line 240, Patchset 9:// ByteCount committed by the system.

Gabriel Charette . resolved

The type of this method's return value didn't change to ByteCount? Should it or should we restore the old comment?

Francois Pierre Doray

Done

Line 240, Patchset 9:// ByteCount committed by the system.

Gabriel Charette . resolved

The type of this method's return value didn't change to ByteCount? Should it or should we restore the old comment?

Francois Pierre Doray

It's probably due to an incorrect search+replace. We should change the function's return type, but not in this CL.

File base/system/sys_info.h

Line 90, Patchset 9: static uint64_t AmountOfVirtualMemory();

Gabriel Charette . resolved

Why not this one as well? Follow-up? Seems there are more we should update in this file too (disk space bytes, VM bytes, etc.). Submit a go/code-health-rotation if too onerous?

Francois Pierre Doray

We should update it, but I had to stop at some point to keep this CL small-ish. go/code-health-rotation is a good idea.

File base/system/sys_info.cc

Line 97, Patchset 9: constexpr int64_t kUpperBound2GB = 2 * 1024; // inclusive

Gabriel Charette . resolved

convert as well? here and below?

Francois Pierre Doray

Done. Note that I used MiB instead of GiB in some places below, because ByteCount doesn't support multiplication by floating point number (I'm unsure whether it should, but we can discuss that separately).

File base/win/hardware_check.cc

Line 115, Patchset 1: static constexpr int64_t kMinTotalDiskSpace = 64 * 1024 * 1024;

Francois Pierre Doray . resolved

Was this supposed to be 4 GiB, instead of 4 MiB?

Gabriel Charette

!! Let's add a "TODO(429140103): This was migrated as-is to 4MiB in ByteCount but the legacy code potentially intended 4GiB, needs investigation." ? And follow-up (i.e. I wouldn't do it in this migration-only CL)?

Francois Pierre Doray

Done

File chrome/browser/ash/bruschetta/bruschetta_installer_impl_unittest.cc

Line 98, Patchset 9 (Parent): BruschettaInstallerTest() : fake_20gb_memory(20ULL * 1024 * 1024) {}

Gabriel Charette . resolved

This constructor used to take MiB, was this incorrectly passing 20TiB? Potentially worth fixing separately if I'm reading correctly so the migration remains a no-op (and avoids a revert)?

Francois Pierre Doray

Ack, added a TODO and preserved the 20 TiB for this CL.

File chrome/browser/policy/site_isolation_policy_browsertest.cc

Line 287, Patchset 9 (Parent): uint64_t ram_kb) {

Gabriel Charette . resolved

This was incorrectly named `_kb` but specifying MB (per ScopedAmountOfPhysicalMemoryOverride's constructor)?
Worth fixing the tests and specifying the change in the CL description assuming they pass?

Francois Pierre Doray

Yes, the value was in MiB despite the variable name suggesting that it was in KiB. Given the test names (high RAM, low RAM), I suspect that the intent was to use MiB... (tests don't pass if we use KiB), it's just the variable name that was incorrect.

File content/browser/renderer_host/navigation_transitions/navigation_transition_config.cc

Line 75, Patchset 9:size_t NavigationTransitionConfig::ComputeCacheSizeInBytes() {

Gabriel Charette . resolved

There's room to keep expanding on some of these methods. Of course, we need to stop at some point on each CL, should we keep a list of methods to refactor in follow-up CLs (for a code health rotation)?

Francois Pierre Doray

Yes, I'll submit a code health rotation proposal after my vacation. This CL is only intended to be a proof of concept, and I need to stop somewhere.

File sandbox/policy/win/sandbox_win_unittest.cc

Line 452, Patchset 9: constexpr base::ByteCount k65GB = base::MiB(66560);


  constexpr base::ByteCount k33GB = base::MiB(33792);
  constexpr base::ByteCount k17GB = base::MiB(17408);

Gabriel Charette . resolved

These are actually precise base::GiB multiples of 1024

Francois Pierre Doray

Done

Open in Gerrit

Related details

Attention is currently required from:

Joe Mason

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Gabriel Charette (Gerrit)

unread,

Jul 16, 2025, 10:32:04 AMJul 16

to Francois Pierre Doray, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray and Joe Mason

Gabriel Charette voted and added 2 comments

Votes added by Gabriel Charette

Code-Review	+1
Owners-Override	+1

2 comments

File base/system/sys_info.cc

Line 97, Patchset 9: constexpr int64_t kUpperBound2GB = 2 * 1024; // inclusive

Gabriel Charette . resolved

convert as well? here and below?

Francois Pierre Doray

Done. Note that I used MiB instead of GiB in some places below, because ByteCount doesn't support multiplication by floating point number (I'm unsure whether it should, but we can discuss that separately).

Gabriel Charette

Agreed that maybe it should, okay to park that for later if it comes up again.

File content/browser/renderer_host/navigation_transitions/navigation_transition_config.cc

Line 75, Patchset 9:size_t NavigationTransitionConfig::ComputeCacheSizeInBytes() {

Gabriel Charette . unresolved

There's room to keep expanding on some of these methods. Of course, we need to stop at some point on each CL, should we keep a list of methods to refactor in follow-up CLs (for a code health rotation)?

Francois Pierre Doray

Yes, I'll submit a code health rotation proposal after my vacation. This CL is only intended to be a proof of concept, and I need to stop somewhere.

Gabriel Charette

Right but I meant that we should keep track of methods where we stopped and should resume? I suspect the code-health-rotation will need multiple iterations of "fix a method, stop at some depth, make a list of follow-ups, repeat"?

In other words, if we don't make a list now, how do we find these candidates again in the future?

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray
Joe Mason

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Will Harris (Gerrit)

unread,

Jul 16, 2025, 10:52:03 AMJul 16

to Francois Pierre Doray, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray and Joe Mason

Will Harris added 1 comment

File sandbox/policy/win/sandbox_win_unittest.cc

Line 449, Patchset 11 (Latest): constexpr base::ByteCount k8GB = base::MiB(8192);

Will Harris . unresolved

Curious, why can't this be GiB(8)?

satisfied_requirement

unsatisfied_requirement

open

diffy

Joe Mason (Gerrit)

unread,

Jul 16, 2025, 6:42:30 PMJul 16

to Francois Pierre Doray, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Joe Mason voted and added 15 comments

Votes added by Joe Mason

Code-Review

+1

15 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Joe Mason . resolved

LGTM with nits. Nice cleanup!

File base/system/sys_info.cc

Line 103, Patchset 11 (Latest): constexpr ByteCount kUpperBound3GB = MiB(3.2 * 1024); // inclusive

Joe Mason . unresolved

ByteCount now has a templated `GiB` function, but it looks like `GiB(3.2)` would convert to an int first, so it's a footgun.

As a followup, how about splitting the ByteCount templates into one with `requires std::is_integral_v<T>` and one with `requires std::is_floating_point_v<T>` that uses `CheckedNumeric<double>(gib) * 1024 * 1024 * 1024` and then coerces the result to `int64_t`? Then this can change to `GiB(3.2)`.

Line 115, Patchset 11 (Latest): constexpr ByteCount kUpperBound6GB = MiB(6.5 * 1024 - 1); // inclusive

Joe Mason . unresolved

Nit: should be `MiB(6.5 * 1024) - 1`, I think, in case `AmountOfPhysicalMemory()` ever returns incremements smaller than 1 MB.

File base/system/sys_info_freebsd.cc

Line 23, Patchset 11 (Latest): return ByteCount(pages * page_size);

Joe Mason . unresolved

Nit: how about `ByteCount(pages) * page_size` which will CHECK on overflow?

File base/system/sys_info_ios.mm

Line 116, Patchset 11 (Latest): return KiB(info.free + info.speculative);

Joe Mason . unresolved

Nit: `KiB(info.free) + KiB(info.speculative)` will CHECK on overflow.

File base/system/sys_info_linux.cc

Line 39, Patchset 11 (Latest): return base::ByteCount(pages * page_size);

Joe Mason . unresolved

Nit: `ByteCount(pages) * page_size`?

File base/system/sys_info_mac.mm

Line 110, Patchset 11 (Latest): return KiB(info.free + info.speculative);

Joe Mason . unresolved

Nit: `KiB(info.free) + KiB(info.speculative)`?

File base/system/sys_info_openbsd.cc

Line 26, Patchset 11 (Latest): return ByteCount(pages * page_size);

Joe Mason . unresolved

Nit: `ByteCount(pages) * page_size`?

File chrome/browser/performance_manager/metrics/metrics_provider_common.cc

Line 83, Patchset 11 (Latest): // `info.file_backed` is in kb, so multiply it by 1024 to get the amount of

Joe Mason . unresolved

Nit: this comment's not useful anymore.

Line 88, Patchset 11 (Latest): total_bytes.InBytes());

Joe Mason . unresolved

Nit: I think you could just do one `.InBytes()` at the end of the calculation. Less back and forth and risk of overflow.

`((available_bytes + base::KiB(info.file_backed)) * 100 / total_bytes).InBytes()`

File chrome/browser/policy/site_isolation_policy_browsertest.cc

Line 311, Patchset 11 (Latest): /*ram=*/base::MiB(8000)),

Joe Mason . unresolved

Nit: these could use comments that the original code used MB but said KB, needs investigation.

File components/miracle_parameter/common/public/miracle_parameter_unittest.cc

Line 125, Patchset 11 (Latest): kMiracleParameterMemory512MB - base::MiB(1));

Joe Mason . unresolved

Nit: I think these should all be `- 1` since the measurement used to be in 1 MB granularity and now is in 1 byte, and it's testing 1 less than the boundary condition.

File extensions/browser/api/system_memory/memory_info_provider.cc

Line 25, Patchset 11 (Latest): static_cast<double>(base::SysInfo::AmountOfPhysicalMemory().InBytes());

Joe Mason . unresolved

Nit: as a followup, how about adding `InBytesF` to ByteCount to avoid the cast?

File ios/chrome/app/memory_monitor.mm

Line 35, Patchset 11 (Latest): const int free_memory = static_cast<int>(

Joe Mason . unresolved

Nit: putting this inline in the function call should avoid the cast. `SetCurrentFreeMemoryInKB(AmountOfEtc().InKb())`

File sandbox/policy/win/sandbox_win_unittest.cc

Line 449, Patchset 11 (Latest): constexpr base::ByteCount k8GB = base::MiB(8192);

Will Harris . unresolved

Curious, why can't this be GiB(8)?

Joe Mason

Nit: please use `GiB(8)`. :-)

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

satisfied_requirement

unsatisfied_requirement

open

diffy

Thiabaud Engelbrecht (Gerrit)

unread,

Jul 16, 2025, 11:07:51 PMJul 16

to Francois Pierre Doray, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Thiabaud Engelbrecht added 1 comment

File base/system/sys_info.cc

Line 103, Patchset 11 (Latest): constexpr ByteCount kUpperBound3GB = MiB(3.2 * 1024); // inclusive

Joe Mason . unresolved

ByteCount now has a templated `GiB` function, but it looks like `GiB(3.2)` would convert to an int first, so it's a footgun.
As a followup, how about splitting the ByteCount templates into one with `requires std::is_integral_v<T>` and one with `requires std::is_floating_point_v<T>` that uses `CheckedNumeric<double>(gib) * 1024 * 1024 * 1024` and then coerces the result to `int64_t`? Then this can change to `GiB(3.2)`.

Thiabaud Engelbrecht

https://chromium-review.googlesource.com/c/chromium/src/+/6759641

satisfied_requirement

unsatisfied_requirement

open

diffy

Gabriel Charette (Gerrit)

unread,

Jul 17, 2025, 8:00:50 AMJul 17

to Francois Pierre Doray, Thiabaud Engelbrecht, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Gabriel Charette added 2 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Gabriel Charette . resolved

Nice review Joe!

File base/system/sys_info.cc

Line 103, Patchset 11 (Latest): constexpr ByteCount kUpperBound3GB = MiB(3.2 * 1024); // inclusive

Joe Mason . unresolved

ByteCount now has a templated `GiB` function, but it looks like `GiB(3.2)` would convert to an int first, so it's a footgun.
As a followup, how about splitting the ByteCount templates into one with `requires std::is_integral_v<T>` and one with `requires std::is_floating_point_v<T>` that uses `CheckedNumeric<double>(gib) * 1024 * 1024 * 1024` and then coerces the result to `int64_t`? Then this can change to `GiB(3.2)`.

Thiabaud Engelbrecht

https://chromium-review.googlesource.com/c/chromium/src/+/6759641

Gabriel Charette

+1 to Joe's suggestion for a floating point constructor to avoid explicit "1024" everywhere.

(I initially wasn't sure but the solution is quite clean with the template `requires` keyword 😊)

satisfied_requirement

unsatisfied_requirement

open

diffy

Joe Mason (Gerrit)

unread,

Jul 17, 2025, 3:57:58 PMJul 17

to Francois Pierre Doray, Thiabaud Engelbrecht, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Joe Mason added 2 comments

File base/system/sys_info.cc

Line 103, Patchset 11 (Latest): constexpr ByteCount kUpperBound3GB = MiB(3.2 * 1024); // inclusive

Joe Mason . unresolved

ByteCount now has a templated `GiB` function, but it looks like `GiB(3.2)` would convert to an int first, so it's a footgun.
As a followup, how about splitting the ByteCount templates into one with `requires std::is_integral_v<T>` and one with `requires std::is_floating_point_v<T>` that uses `CheckedNumeric<double>(gib) * 1024 * 1024 * 1024` and then coerces the result to `int64_t`? Then this can change to `GiB(3.2)`.

Thiabaud Engelbrecht

https://chromium-review.googlesource.com/c/chromium/src/+/6759641

Gabriel Charette

+1 to Joe's suggestion for a floating point constructor to avoid explicit "1024" everywhere.
(I initially wasn't sure but the solution is quite clean with the template `requires` keyword 😊)

Joe Mason

The floating point constructor is up for review in https://crrev.com/c/6764394.

File extensions/browser/api/system_memory/memory_info_provider.cc

Line 25, Patchset 11 (Latest): static_cast<double>(base::SysInfo::AmountOfPhysicalMemory().InBytes());

Joe Mason . unresolved

Nit: as a followup, how about adding `InBytesF` to ByteCount to avoid the cast?

Joe Mason

Added an InBytesF in https://crrev.com/c/6764394

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Aug 7, 2025, 4:35:08 PMAug 7

to Thiabaud Engelbrecht, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Gabriel Charette, Joe Mason, Thiabaud Engelbrecht and Will Harris

Francois Pierre Doray added 15 comments

File base/system/sys_info.cc

Line 103, Patchset 11: constexpr ByteCount kUpperBound3GB = MiB(3.2 * 1024); // inclusive

Joe Mason . resolved

ByteCount now has a templated `GiB` function, but it looks like `GiB(3.2)` would convert to an int first, so it's a footgun.
As a followup, how about splitting the ByteCount templates into one with `requires std::is_integral_v<T>` and one with `requires std::is_floating_point_v<T>` that uses `CheckedNumeric<double>(gib) * 1024 * 1024 * 1024` and then coerces the result to `int64_t`? Then this can change to `GiB(3.2)`.

Thiabaud Engelbrecht

https://chromium-review.googlesource.com/c/chromium/src/+/6759641

Gabriel Charette

+1 to Joe's suggestion for a floating point constructor to avoid explicit "1024" everywhere.
(I initially wasn't sure but the solution is quite clean with the template `requires` keyword 😊)

Joe Mason

The floating point constructor is up for review in https://crrev.com/c/6764394.

Francois Pierre Doray

Done - Now using the floating point API

Line 115, Patchset 11: constexpr ByteCount kUpperBound6GB = MiB(6.5 * 1024 - 1); // inclusive

Joe Mason . resolved

Nit: should be `MiB(6.5 * 1024) - 1`, I think, in case `AmountOfPhysicalMemory()` ever returns incremements smaller than 1 MB.

Francois Pierre Doray

I prefer to preserve the existing code behavior.

File base/system/sys_info_freebsd.cc

Line 23, Patchset 11: return ByteCount(pages * page_size);

Joe Mason . resolved

Nit: how about `ByteCount(pages) * page_size` which will CHECK on overflow?

Francois Pierre Doray

Done

File base/system/sys_info_ios.mm

Line 116, Patchset 11: return KiB(info.free + info.speculative);

Joe Mason . resolved

Nit: `KiB(info.free) + KiB(info.speculative)` will CHECK on overflow.

Francois Pierre Doray

Done

File base/system/sys_info_linux.cc

Line 39, Patchset 11: return base::ByteCount(pages * page_size);

Joe Mason . resolved

Nit: `ByteCount(pages) * page_size`?

Francois Pierre Doray

Done

File base/system/sys_info_mac.mm

Line 110, Patchset 11: return KiB(info.free + info.speculative);

Joe Mason . resolved

Nit: `KiB(info.free) + KiB(info.speculative)`?

Francois Pierre Doray

Done

File base/system/sys_info_openbsd.cc

Line 26, Patchset 11: return ByteCount(pages * page_size);

Joe Mason . resolved

Nit: `ByteCount(pages) * page_size`?

Francois Pierre Doray

Done

File chrome/browser/performance_manager/metrics/metrics_provider_common.cc

Line 83, Patchset 11: // `info.file_backed` is in kb, so multiply it by 1024 to get the amount of

Joe Mason . resolved

Nit: this comment's not useful anymore.

Francois Pierre Doray

Done

Line 88, Patchset 11: total_bytes.InBytes());

Joe Mason . resolved

Nit: I think you could just do one `.InBytes()` at the end of the calculation. Less back and forth and risk of overflow.
`((available_bytes + base::KiB(info.file_backed)) * 100 / total_bytes).InBytes()`

Francois Pierre Doray

It's weird to call InBytes() on the result of (bytes / bytes), no? Bytes cancel out and the resulting value of bytes / bytes is not in bytes?

File chrome/browser/policy/site_isolation_policy_browsertest.cc

Line 311, Patchset 11: /*ram=*/base::MiB(8000)),

Joe Mason . resolved

Nit: these could use comments that the original code used MB but said KB, needs investigation.

Francois Pierre Doray

Done

File components/miracle_parameter/common/public/miracle_parameter_unittest.cc

Line 125, Patchset 11: kMiracleParameterMemory512MB - base::MiB(1));

Joe Mason . resolved

Nit: I think these should all be `- 1` since the measurement used to be in 1 MB granularity and now is in 1 byte, and it's testing 1 less than the boundary condition.

Francois Pierre Doray

Done

File content/browser/renderer_host/navigation_transitions/navigation_transition_config.cc

Line 75, Patchset 9:size_t NavigationTransitionConfig::ComputeCacheSizeInBytes() {

Gabriel Charette . resolved

There's room to keep expanding on some of these methods. Of course, we need to stop at some point on each CL, should we keep a list of methods to refactor in follow-up CLs (for a code health rotation)?

Francois Pierre Doray

Yes, I'll submit a code health rotation proposal after my vacation. This CL is only intended to be a proof of concept, and I need to stop somewhere.

Gabriel Charette

Right but I meant that we should keep track of methods where we stopped and should resume? I suspect the code-health-rotation will need multiple iterations of "fix a method, stop at some depth, make a list of follow-ups, repeat"?
In other words, if we don't make a list now, how do we find these candidates again in the future?

Francois Pierre Doray

Good point. I think that TODOs in code are the most effective way to track this. They can be updated whenever the need to convert to ByteCount change (e.g. in changes unrelated to the rotation) and they're easy to add as the code is modified. Added a comment here. Note that we can also use AI to identify methods that return bytes 😊

File extensions/browser/api/system_memory/memory_info_provider.cc

Line 25, Patchset 11: static_cast<double>(base::SysInfo::AmountOfPhysicalMemory().InBytes());

Joe Mason . resolved

Nit: as a followup, how about adding `InBytesF` to ByteCount to avoid the cast?

Joe Mason

Added an InBytesF in https://crrev.com/c/6764394

Francois Pierre Doray

Done

File ios/chrome/app/memory_monitor.mm

Line 35, Patchset 11: const int free_memory = static_cast<int>(

Joe Mason . resolved

Nit: putting this inline in the function call should avoid the cast. `SetCurrentFreeMemoryInKB(AmountOfEtc().InKb())`

Francois Pierre Doray

Done

File sandbox/policy/win/sandbox_win_unittest.cc

Line 449, Patchset 11: constexpr base::ByteCount k8GB = base::MiB(8192);

Will Harris . resolved

Curious, why can't this be GiB(8)?

Joe Mason

Nit: please use `GiB(8)`. :-)

Francois Pierre Doray

Done

Open in Gerrit

Related details

Attention is currently required from:

Gabriel Charette
Joe Mason
Thiabaud Engelbrecht
Will Harris

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Aug 7, 2025, 8:44:44 PMAug 7

to Thiabaud Engelbrecht, Will Harris, Gabriel Charette, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Gabriel Charette

Francois Pierre Doray added 1 comment

Patchset-level comments

File-level comment, Patchset 16 (Latest):

Francois Pierre Doray . resolved

gab: This CL was rebased and I lost review votes. Can you take another look an CR+1 and OO+1? Thanks.

Open in Gerrit

Related details

Attention is currently required from:

Gabriel Charette

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Gabriel Charette (Gerrit)

unread,

Aug 11, 2025, 2:54:14 PMAug 11

to Francois Pierre Doray, Gabriel Charette, Thiabaud Engelbrecht, Will Harris, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Gabriel Charette voted and added 1 comment

Votes added by Gabriel Charette

Code-Review	+1
Owners-Override	+1

1 comment

Patchset-level comments

File-level comment, Patchset 16 (Latest):

Francois Pierre Doray . resolved

gab: This CL was rebased and I lost review votes. Can you take another look an CR+1 and OO+1? Thanks.

Gabriel Charette

This is great, thanks!

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

satisfied_requirement

open

diffy

Gabriel Charette (Gerrit)

unread,

Aug 12, 2025, 1:31:57 PMAug 12

to Francois Pierre Doray, Gabriel Charette, Thiabaud Engelbrecht, Will Harris, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Attention needed from Francois Pierre Doray

Gabriel Charette voted and added 1 comment

Votes added by Gabriel Charette

Code-Review	+1
Owners-Override	+1

1 comment

Patchset-level comments

File-level comment, Patchset 18 (Latest):

Gabriel Charette . resolved

Rebase stamp

Open in Gerrit

Related details

Attention is currently required from:

Francois Pierre Doray

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Aug 12, 2025, 1:42:35 PMAug 12

to Gabriel Charette, Thiabaud Engelbrecht, Will Harris, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Francois Pierre Doray voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

satisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Aug 13, 2025, 9:31:24 AMAug 13

to Gabriel Charette, Thiabaud Engelbrecht, Will Harris, Chromium LUCI CQ, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Francois Pierre Doray voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Aug 13, 2025, 8:21:25 PMAug 13

to Francois Pierre Doray, Gabriel Charette, Thiabaud Engelbrecht, Will Harris, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Chromium LUCI CQ submitted the change

Unreviewed changes

18 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:

Use base::ByteCount in base::SysInfo.

Use base::ByteCount as the return value of
SysInfo::AmountOfPhysicalMemory() and
SysInfo::AmountOfAvailablePhysicalMemory() and adjust call sites across
the codebase. Note that other methods in base::SysInfo could also return
base::Bytes, but they're not upgraded in this CL to avoid making it
bigger.

base::ByteCount is a class to make bytes manipulations type safe, easier
to read and less error-prone.

NO_IFTTT=Code style change in base/system/sys_info.cc, does not affect code behavior.

AX-Relnotes: n/a

Bug: 429140103

Change-Id: I540a58b843b1765a61d03bb5d7867b3bf8a2ddd1

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

Reviewed-by: Gabriel Charette <g...@chromium.org>

Owners-Override: Gabriel Charette <g...@chromium.org>

Commit-Queue: Francois Pierre Doray <fdo...@chromium.org>

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

Files:

M android_webview/browser/aw_browser_main_parts.cc
M ash/display/window_tree_host_manager.cc
M ash/test/pixel/ash_pixel_test_helper.cc
M base/allocator/miracle_parameter.cc
M base/allocator/miracle_parameter.h
M base/allocator/partition_alloc_support.cc
M base/android/sys_utils_unittest.cc
M base/features.cc
M base/features.h
M base/process/process_metrics_apple.mm
M base/process/process_metrics_unittest.cc
M base/system/sys_info.cc
M base/system/sys_info.h
M base/system/sys_info_apple.mm
M base/system/sys_info_freebsd.cc
M base/system/sys_info_fuchsia.cc
M base/system/sys_info_ios.mm
M base/system/sys_info_linux.cc
M base/system/sys_info_mac.mm
M base/system/sys_info_openbsd.cc
M base/system/sys_info_unittest.cc
M base/system/sys_info_win.cc
M base/test/scoped_amount_of_physical_memory_override.cc
M base/test/scoped_amount_of_physical_memory_override.h
M base/win/hardware_check.cc
M cc/tiles/image_decode_cache_utils.cc
M chrome/browser/android/oom_intervention/oom_intervention_config.cc
M chrome/browser/ash/app_mode/metrics/periodic_metrics_service.cc
M chrome/browser/ash/borealis/borealis_hardware_checker.cc
M chrome/browser/ash/borealis/borealis_hardware_checker_unittest.cc
M chrome/browser/ash/borealis/borealis_survey_handler.cc
M chrome/browser/ash/borealis/borealis_survey_handler_unittest.cc
M chrome/browser/ash/bruschetta/bruschetta_installer_impl.cc
M chrome/browser/ash/bruschetta/bruschetta_installer_impl_unittest.cc
M chrome/browser/ash/policy/status_collector/device_status_collector.cc
M chrome/browser/ash/policy/status_collector/device_status_collector.h
M chrome/browser/feedback/android/system_info_feedback_source.cc
M chrome/browser/media/webrtc/webrtc_text_log_handler.cc
M chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
M chrome/browser/metrics/structured/storage_manager_impl.cc
M chrome/browser/metrics/usage_scenario/chrome_responsiveness_calculator_delegate.cc
M chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc
M chrome/browser/performance_manager/metrics/metrics_provider_common.cc
M chrome/browser/performance_manager/policies/background_tab_loading_policy.cc
M chrome/browser/policy/site_isolation_policy_browsertest.cc
M chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.cc
M chrome/browser/preloading/search_preload/search_preload_features.cc
M chrome/browser/resource_coordinator/session_restore_policy.cc
M chrome/browser/site_isolation/chrome_site_isolation_policy_unittest.cc
M chrome/browser/ui/browser_browsertest.cc
M chrome/browser/ui/lens/lens_overlay_entry_point_controller.cc
M chrome/browser/ui/performance_controls/memory_saver_opt_in_iph_controller.cc
M chrome/browser/ui/performance_controls/performance_controls_hats_service.cc
M chrome/browser/ui/views/performance_controls/memory_saver_iph_interactive_uitest.cc
M chromeos/ash/components/memory/swap_configuration.cc
M components/autofill/content/browser/risk/fingerprint.cc
M components/discardable_memory/service/discardable_shared_memory_manager.cc
M components/gwp_asan/client/gwp_asan.cc
M components/metrics/metrics_log.cc
M components/metrics/metrics_log_unittest.cc
M components/miracle_parameter/common/public/miracle_parameter.cc
M components/miracle_parameter/common/public/miracle_parameter.h
M components/miracle_parameter/common/public/miracle_parameter_unittest.cc
M components/segmentation_platform/internal/execution/processing/custom_input_processor.cc
M components/site_isolation/site_isolation_policy.cc
M components/site_isolation/site_isolation_policy_unittest.cc
M components/update_client/protocol_serializer.cc
M components/viz/client/frame_eviction_manager.cc
M content/browser/back_forward_cache_browsertest.cc
M content/browser/network/network_service_util_internal.cc
M content/browser/network/shared_dictionary_util.cc
M content/browser/preloading/prerender/prerender_browsertest.cc
M content/browser/preloading/prerender/prerender_host_registry.cc
M content/browser/renderer_host/navigation_transitions/navigation_entry_screenshot_manager_unittest.cc
M content/browser/renderer_host/navigation_transitions/navigation_transition_config.cc
M content/browser/renderer_host/render_process_host_impl.cc
M content/browser/renderer_host/spare_render_process_host_manager_browsertest.cc
M content/browser/renderer_host/spare_render_process_host_manager_impl.cc
M content/browser/tracing/tracing_controller_impl.cc
M content/child/blink_platform_impl.cc
M content/common/content_navigation_policy.cc
M extensions/browser/api/system_memory/memory_info_provider.cc
M gin/isolate_holder.cc
M gpu/command_buffer/client/shared_memory_limits.h
M gpu/command_buffer/service/service_discardable_manager.cc
M gpu/config/gpu_finch_features.cc
M gpu/config/gpu_util.cc
M gpu/config/skia_limits.cc
M ios/chrome/app/memory_monitor.mm
M media/base/demuxer_memory_limit_android.cc
M net/disk_cache/blockfile/backend_impl.cc
M net/disk_cache/memory/mem_backend_impl.cc
M sandbox/policy/linux/sandbox_linux.cc
M sandbox/policy/win/sandbox_win.cc
M sandbox/policy/win/sandbox_win_unittest.cc
M services/on_device_model/ml/performance_class.cc
M services/on_device_model/public/cpp/cpu.cc
M services/screen_ai/screen_ai_ocr_perf_test.cc
M services/screen_ai/screen_ai_service_impl.cc
M storage/browser/blob/blob_memory_controller.cc
M storage/browser/file_system/obfuscated_file_util_memory_delegate.cc
M storage/browser/quota/quota_device_info_helper.cc
M third_party/blink/common/device_memory/approximated_device_memory.cc
M third_party/blink/renderer/controller/memory_saver_controller.cc
M third_party/blink/renderer/platform/fonts/font_cache.cc
M third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc
M third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
M ui/gl/gl_surface_egl.cc

Change size: L

Delta: 108 files changed, 421 insertions(+), 392 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Gabriel Charette

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

luci-bisection@appspot.gserviceaccount.com (Gerrit)

unread,

Aug 13, 2025, 9:19:35 PMAug 13

to Francois Pierre Doray, Chromium LUCI CQ, Gabriel Charette, Thiabaud Engelbrecht, Will Harris, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Message from luci-bi...@appspot.gserviceaccount.com

LUCI Bisection has identified this change as the cause of a test failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/6525964558270464

Sample build with failed test: https://ci.chromium.org/b/8706605585641464801
Affected test(s):
[ninja://base:base_unittests/SysUtils.AmountOfPhysicalMemory](https://ci.chromium.org/ui/test/chromium/ninja:%2F%2Fbase:base_unittests%2FSysUtils.AmountOfPhysicalMemory?q=VHash%3A436bad0fdfd39741)
A revert for this change was not created because the builder that this CL broke is not watched by gardeners, therefore less important. You can consider revert this CL, fix forward or let builder owners resolve it themselves.

If this is a false positive, please report it at http://b.corp.google.com/createIssue?component=1199205&description=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Ftest-analysis%2Fb%2F6525964558270464&format=PLAIN&priority=P3&title=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F6698912&type=BUG

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Peter Pakkenberg (Gerrit)

unread,

Aug 14, 2025, 4:49:07 AMAug 14

to Francois Pierre Doray, Chromium LUCI CQ, luci-bi...@appspot.gserviceaccount.com, Gabriel Charette, Thiabaud Engelbrecht, Will Harris, bfcach...@chromium.org, chromium...@chromium.org, Dirk Schulze, Enterprise Policy Reviews, Andrew Rayskiy, prerendering-reviews, Rijubrata Bhaumik, Stephen Chenney, Simon Hangl, Zijie He, agriev...@chromium.org, alexmo...@chromium.org, android-web...@chromium.org, asvitki...@chromium.org, bartek...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, borealis-re...@google.com, browser-comp...@chromium.org, cc-...@chromium.org, chfreme...@chromium.org, chikamu...@chromium.org, chrome-gr...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, chromeos-kio...@google.com, chromium-a...@chromium.org, chromiumme...@microsoft.com, creis...@chromium.org, cros-report...@google.com, drott+bl...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, fuchsia...@chromium.org, gavin...@chromium.org, gavin...@chromium.org, glazuno...@chromium.org, grt+...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, jessemcke...@google.com, kinuko+...@chromium.org, kinuko...@chromium.org, lens-chrome...@google.com, lingqi...@chromium.org, lizeb...@chromium.org, mac-r...@chromium.org, marq+...@chromium.org, mercer...@google.com, mpdento...@chromium.org, navigation...@chromium.org, net-r...@chromium.org, nyquis...@chromium.org, oshima...@chromium.org, ozone-...@chromium.org, pasko...@chromium.org, performance-m...@chromium.org, rhalava...@chromium.org, rmcelra...@chromium.org, roblia...@chromium.org, rouslan+au...@chromium.org, rsesek...@chromium.org, spang...@chromium.org, stanfie...@google.com, tburkar...@chromium.org, thiabaud+watch-d...@google.com, torne...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org, zhangwen...@google.com

Peter Pakkenberg has created a revert of this change

Open original change

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Use base::ByteCount in base::SysInfo. [chromium/src : main]

Francois Pierre Doray (Gerrit)

Francois Pierre Doray voted and added 1 comment

Votes added by Francois Pierre Doray

1 comment

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 1 comment

Related details

Gabriel Charette (Gerrit)

Gabriel Charette voted and added 10 comments

Votes added by Gabriel Charette

10 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 10 comments

Related details

Gabriel Charette (Gerrit)

Gabriel Charette voted and added 2 comments

Votes added by Gabriel Charette

2 comments

Related details

Will Harris (Gerrit)

Will Harris added 1 comment

Joe Mason (Gerrit)

Joe Mason voted and added 15 comments

Votes added by Joe Mason

15 comments

Related details

Thiabaud Engelbrecht (Gerrit)

Thiabaud Engelbrecht added 1 comment

Gabriel Charette (Gerrit)

Gabriel Charette added 2 comments

Joe Mason (Gerrit)

Joe Mason added 2 comments

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 15 comments

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray added 1 comment

Related details

Gabriel Charette (Gerrit)

Gabriel Charette voted and added 1 comment

Votes added by Gabriel Charette

1 comment

Related details

Gabriel Charette (Gerrit)

Gabriel Charette voted and added 1 comment

Votes added by Gabriel Charette

1 comment

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray voted Commit-Queue+2

Related details

Francois Pierre Doray (Gerrit)

Francois Pierre Doray voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Unreviewed changes

Change information

luci-bisection@appspot.gserviceaccount.com (Gerrit)

Message from luci-bi...@appspot.gserviceaccount.com

Related details

Peter Pakkenberg (Gerrit)

Peter Pakkenberg has created a revert of this change

Related details