Joe Mason (Gerrit)

unread,

Dec 3, 2025, 2:58:29 PMDec 3

to Etienne Pierre-Doray, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Etienne Pierre-Doray

Joe Mason voted and added 1 comment

Votes added by Joe Mason

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4:

Joe Mason . unresolved

Review note: the only significant changes are in
base/
media/base/
tools/metrics/
services/resource_coordinator/public/cpp/memory_instrumentation/tracing_integration_unittest.cc
third_party/blink/renderer/platform/media/web_media_player_impl.cc
The others are all just changing strings to fit the histogram format.
etiennep, can you review the above files? I'll send the rest of the changes to individual OWNER's to make sure the specific new names for MemoryDumpProviders don't have side effects (eg. if there are analysis scripts that use the old names).

Joe Mason

Actually, I'm going to simplify this by removing the name changes.

Open in Gerrit

Related details

Attention is currently required from:

Etienne Pierre-Doray

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Joe Mason (Gerrit)

unread,

Dec 3, 2025, 4:00:54 PMDec 3

to Etienne Pierre-Doray, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Etienne Pierre-Doray

Joe Mason voted and added 1 comment

Votes added by Joe Mason

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4:

Joe Mason . resolved

Review note: the only significant changes are in
base/
media/base/
tools/metrics/
services/resource_coordinator/public/cpp/memory_instrumentation/tracing_integration_unittest.cc
third_party/blink/renderer/platform/media/web_media_player_impl.cc
The others are all just changing strings to fit the histogram format.
etiennep, can you review the above files? I'll send the rest of the changes to individual OWNER's to make sure the specific new names for MemoryDumpProviders don't have side effects (eg. if there are analysis scripts that use the old names).

Joe Mason

Actually, I'm going to simplify this by removing the name changes.

Joe Mason

Done

Open in Gerrit

Related details

Attention is currently required from:

Etienne Pierre-Doray

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

Etienne Pierre-Doray (Gerrit)

unread,

Dec 4, 2025, 5:10:51 AMDec 4

to AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Joe Mason

Etienne Pierre-Doray added 2 comments

File base/trace_event/memory_infra_background_allowlist.cc

Line 26, Patchset 15 (Latest):// The names of dump providers allowed to perform background tracing. Dump

Etienne Pierre-Doray . unresolved

It sounds like you will be using this allow list to also report UMA metrics, so update the description to reflect that?

Line 31, Patchset 15 (Latest):constexpr auto kDumpProviderAllowlist =

Etienne Pierre-Doray . unresolved

kMemoryDumpProviderNameAllowList is now generated by generate_allowlist_from_histograms_file, can that replace kDumpProviderAllowlist entirely (or at list auto generate a flat_set from kMemoryDumpProviderNameAllowList list)?

Open in Gerrit

Related details

Attention is currently required from:

Joe Mason

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Joe Mason (Gerrit)

unread,

Dec 4, 2025, 3:32:50 PMDec 4

to Etienne Pierre-Doray, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Etienne Pierre-Doray

Joe Mason added 2 comments

File base/trace_event/memory_infra_background_allowlist.cc

Line 26, Patchset 15 (Latest):// The names of dump providers allowed to perform background tracing. Dump

Etienne Pierre-Doray . resolved

It sounds like you will be using this allow list to also report UMA metrics, so update the description to reflect that?

Joe Mason

No, I don't plan to change the usage of this. I only changed it to take `Name` instead of `string` to provide better compile-time safety for the existing code, now that it's possible.

Line 31, Patchset 15 (Latest):constexpr auto kDumpProviderAllowlist =

Etienne Pierre-Doray . resolved

kMemoryDumpProviderNameAllowList is now generated by generate_allowlist_from_histograms_file, can that replace kDumpProviderAllowlist entirely (or at list auto generate a flat_set from kMemoryDumpProviderNameAllowList list)?

Joe Mason

No, this is a subset of providers that are allowed to run in the background, so it still needs to be a manually maintained list.

`kMemoryDumpProviderNameAllowList` is a list of strings that are allowed to be used as MemoryDumpProvider names. It's a coincidence that both vars are named "AllowList".

Open in Gerrit

Related details

Attention is currently required from:

Etienne Pierre-Doray

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

Joe Mason (Gerrit)

unread,

Dec 4, 2025, 3:47:43 PMDec 4

to Etienne Pierre-Doray, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Etienne Pierre-Doray

Joe Mason added 1 comment

File base/trace_event/memory_infra_background_allowlist.cc

Line 31, Patchset 15:constexpr auto kDumpProviderAllowlist =

Etienne Pierre-Doray . resolved

kMemoryDumpProviderNameAllowList is now generated by generate_allowlist_from_histograms_file, can that replace kDumpProviderAllowlist entirely (or at list auto generate a flat_set from kMemoryDumpProviderNameAllowList list)?

Joe Mason

No, this is a subset of providers that are allowed to run in the background, so it still needs to be a manually maintained list.
`kMemoryDumpProviderNameAllowList` is a list of strings that are allowed to be used as MemoryDumpProvider names. It's a coincidence that both vars are named "AllowList".

Joe Mason

It looks like this is dead code. I'll double-check on that and remove it in a followup. In the meantime I reverted the changes in this file to avoid churn.

Open in Gerrit

Related details

Attention is currently required from:

Etienne Pierre-Doray

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

Etienne Pierre-Doray (Gerrit)

unread,

Dec 8, 2025, 1:59:21 PM (12 days ago) Dec 8

to AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Joe Mason

Etienne Pierre-Doray voted and added 1 comment

Votes added by Etienne Pierre-Doray

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 19 (Latest):

Etienne Pierre-Doray . resolved

LGTM

Open in Gerrit

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Joe Mason (Gerrit)

unread,

Dec 8, 2025, 4:03:39 PM (12 days ago) Dec 8

to Colin Blundell, Etienne Pierre-Doray, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Colin Blundell

Joe Mason added 1 comment

Patchset-level comments

File-level comment, Patchset 19 (Latest):

Joe Mason . resolved

+blundell for media/

Open in Gerrit

Related details

Attention is currently required from:

Colin Blundell

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

Colin Blundell (Gerrit)

unread,

Dec 9, 2025, 3:13:44 AM (11 days ago) Dec 9

to Colin Blundell, Etienne Pierre-Doray, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Joe Mason

Colin Blundell voted and added 1 comment

Votes added by Colin Blundell

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 20 (Latest):

Colin Blundell . resolved

Thanks!

Open in Gerrit

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.

Gerrit

satisfied_requirement

open

diffy

Joe Mason (Gerrit)

unread,

Dec 9, 2025, 2:56:14 PM (11 days ago) Dec 9

to Etienne Pierre-Doray, Colin Blundell, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, Chromium LUCI CQ, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Attention needed from Etienne Pierre-Doray

Joe Mason voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Etienne Pierre-Doray

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,

Dec 9, 2025, 3:11:25 PM (11 days ago) Dec 9

to Etienne Pierre-Doray, Colin Blundell, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Chromium LUCI CQ submitted the change

Unreviewed changes

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

Change information

Commit message:

Enforce that MemoryDumpProvider names are histogram variants

To prepare for per-MemoryDumpProvider metrics, use
generate_allowlist_from_histograms_file to enforce that the name string
passed to a MemoryDumpProvider matches a variant tag defined in
histograms.xml. The variant tag check is done at compile-time by a new
MemoryDumpProvider::Name class with a consteval constructor, inspired by
sql::Database::Tag.

Adds variants for all existing names to histograms.xml, with ":"
characters changed to "_". Follow-up patches will add metrics that use
these variants.

Bug: 449150643

Skip-Clang-Tidy-Checks: google-explicit-constructor

Change-Id: Id39c782f1730939bd333f3421168d5d093309a80

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

Reviewed-by: Etienne Pierre-Doray <etie...@chromium.org>

Reviewed-by: Colin Blundell <blun...@chromium.org>

Commit-Queue: Joe Mason <joenot...@google.com>

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

Files:

M base/BUILD.gn
M base/trace_event/memory_dump_manager.cc
M base/trace_event/memory_dump_manager.h
M base/trace_event/memory_dump_manager_unittest.cc
M base/trace_event/memory_dump_provider.h
M content/browser/tracing/memory_tracing_browsertest.cc
M media/base/memory_dump_provider_proxy.cc
M media/base/memory_dump_provider_proxy.h
M services/resource_coordinator/public/cpp/memory_instrumentation/tracing_integration_unittest.cc
M third_party/blink/renderer/platform/media/web_media_player_impl.cc
M tools/metrics/histograms/metadata/memory/histograms.xml

Change size: M

Delta: 11 files changed, 209 insertions(+), 35 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Colin Blundell, +1 by Etienne Pierre-Doray

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Peter Wen (Gerrit)

unread,

Dec 18, 2025, 3:10:53 PM (2 days ago) Dec 18

to Chromium LUCI CQ, Etienne Pierre-Doray, Colin Blundell, AyeAye, Chromium Metrics Reviews, chromium...@chromium.org, Dirk Schulze, Stephen Chenney, chrome-gr...@chromium.org, agriev...@chromium.org, asvitkine...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cblume...@chromium.org, cc-...@chromium.org, chromeos-gfx-...@google.com, drott+bl...@chromium.org, erickun...@chromium.org, feature-me...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, lizeb...@chromium.org, media-cro...@chromium.org, mfoltz+wa...@chromium.org, nyquis...@chromium.org, pasko...@chromium.org, penghu...@chromium.org, penghuan...@chromium.org, spang...@chromium.org, torne...@chromium.org, tracing...@chromium.org, vaapi-...@chromium.org, wfh+...@chromium.org, yfriedm...@chromium.org

Peter Wen added 1 comment

Patchset-level comments

File-level comment, Patchset 22 (Latest):

Peter Wen . resolved

This caused orderfile bot failures (see http://b/470070622), adding Orderfile here: https://crrev.com/c/7277544

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

Enforce that MemoryDumpProvider names are histogram variants [chromium/src : main]

Joe Mason (Gerrit)

Joe Mason voted and added 1 comment

Votes added by Joe Mason

1 comment

Related details

Joe Mason (Gerrit)

Joe Mason voted and added 1 comment

Votes added by Joe Mason

1 comment

Related details

Etienne Pierre-Doray (Gerrit)

Etienne Pierre-Doray added 2 comments

Related details

Joe Mason (Gerrit)

Joe Mason added 2 comments

Related details

Joe Mason (Gerrit)

Joe Mason added 1 comment

Related details

Etienne Pierre-Doray (Gerrit)

Etienne Pierre-Doray voted and added 1 comment

Votes added by Etienne Pierre-Doray

1 comment

Related details

Joe Mason (Gerrit)

Joe Mason added 1 comment

Related details

Colin Blundell (Gerrit)

Colin Blundell voted and added 1 comment

Votes added by Colin Blundell

1 comment

Related details

Joe Mason (Gerrit)

Joe Mason voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Unreviewed changes

Change information

Peter Wen (Gerrit)

Peter Wen added 1 comment

Related details