[histogram-cleanup] Remove expired histogram: Media.MediaDevices.GetAllScreensMedia.Latency [chromium/src : main]

0 views
Skip to first unread message

Mark Schillaci (Gerrit)

unread,
Apr 16, 2026, 3:46:46 PM (11 days ago) Apr 16
to Simon Hangl, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Raphael Kubo da Costa, asvitkine...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, tommyw+w...@chromium.org
Attention needed from Simon Hangl

Mark Schillaci added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Mark Schillaci . resolved

PTAL

Open in Gerrit

Related details

Attention is currently required from:
  • Simon Hangl
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I12773a1f9488c06d70a1c5112461dd187e93349d
Gerrit-Change-Number: 7765804
Gerrit-PatchSet: 1
Gerrit-Owner: Mark Schillaci <mschi...@google.com>
Gerrit-Reviewer: Mark Schillaci <mschi...@google.com>
Gerrit-Reviewer: Simon Hangl <sim...@google.com>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
Gerrit-Attention: Simon Hangl <sim...@google.com>
Gerrit-Comment-Date: Thu, 16 Apr 2026 19:46:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Simon Hangl (Gerrit)

unread,
Apr 20, 2026, 7:26:41 AM (7 days ago) Apr 20
to Mark Schillaci, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Raphael Kubo da Costa, asvitkine...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, tommyw+w...@chromium.org
Attention needed from Mark Schillaci

Simon Hangl voted and added 1 comment

Votes added by Simon Hangl

Code-Review+1

1 comment

Patchset-level comments
Simon Hangl . resolved

LGTM

Open in Gerrit

Related details

Attention is currently required from:
  • Mark Schillaci
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I12773a1f9488c06d70a1c5112461dd187e93349d
    Gerrit-Change-Number: 7765804
    Gerrit-PatchSet: 1
    Gerrit-Owner: Mark Schillaci <mschi...@google.com>
    Gerrit-Reviewer: Mark Schillaci <mschi...@google.com>
    Gerrit-Reviewer: Simon Hangl <sim...@google.com>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
    Gerrit-Attention: Mark Schillaci <mschi...@google.com>
    Gerrit-Comment-Date: Mon, 20 Apr 2026 11:26:25 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mark Schillaci (Gerrit)

    unread,
    Apr 20, 2026, 10:44:19 AM (7 days ago) Apr 20
    to Simon Hangl, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Raphael Kubo da Costa, asvitkine...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, tommyw+w...@chromium.org

    Mark Schillaci added 1 comment

    Patchset-level comments
    Mark Schillaci . resolved

    Thank you Simon for the review!

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I12773a1f9488c06d70a1c5112461dd187e93349d
    Gerrit-Change-Number: 7765804
    Gerrit-PatchSet: 1
    Gerrit-Owner: Mark Schillaci <mschi...@google.com>
    Gerrit-Reviewer: Mark Schillaci <mschi...@google.com>
    Gerrit-Reviewer: Simon Hangl <sim...@google.com>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
    Gerrit-Comment-Date: Mon, 20 Apr 2026 14:44:07 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mark Schillaci (Gerrit)

    unread,
    Apr 20, 2026, 10:44:54 AM (7 days ago) Apr 20
    to Takashi Toyoshima, Simon Hangl, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Raphael Kubo da Costa, asvitkine...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, tommyw+w...@chromium.org
    Attention needed from Takashi Toyoshima

    Mark Schillaci added 1 comment

    Patchset-level comments
    Mark Schillaci . resolved

    PTAL

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Takashi Toyoshima
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I12773a1f9488c06d70a1c5112461dd187e93349d
    Gerrit-Change-Number: 7765804
    Gerrit-PatchSet: 1
    Gerrit-Owner: Mark Schillaci <mschi...@google.com>
    Gerrit-Reviewer: Mark Schillaci <mschi...@google.com>
    Gerrit-Reviewer: Simon Hangl <sim...@google.com>
    Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
    Gerrit-Attention: Takashi Toyoshima <toyo...@chromium.org>
    Gerrit-Comment-Date: Mon, 20 Apr 2026 14:44:47 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Takashi Toyoshima (Gerrit)

    unread,
    Apr 21, 2026, 4:27:36 AM (6 days ago) Apr 21
    to Mark Schillaci, Simon Hangl, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Raphael Kubo da Costa, asvitkine...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, tommyw+w...@chromium.org
    Attention needed from Mark Schillaci

    Takashi Toyoshima added 1 comment

    Commit Message
    Line 9, Patchset 1 (Latest):This histogram expired on 2023-05-01.
    Takashi Toyoshima . unresolved

    Can you explain a bit about SuppressLatencyRecording()?
    This looks something more than just removing expired histograms.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mark Schillaci
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I12773a1f9488c06d70a1c5112461dd187e93349d
      Gerrit-Change-Number: 7765804
      Gerrit-PatchSet: 1
      Gerrit-Owner: Mark Schillaci <mschi...@google.com>
      Gerrit-Reviewer: Mark Schillaci <mschi...@google.com>
      Gerrit-Reviewer: Simon Hangl <sim...@google.com>
      Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
      Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
      Gerrit-CC: Kentaro Hara <har...@chromium.org>
      Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
      Gerrit-Attention: Mark Schillaci <mschi...@google.com>
      Gerrit-Comment-Date: Tue, 21 Apr 2026 08:27:07 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Mark Schillaci (Gerrit)

      unread,
      Apr 24, 2026, 5:17:37 PM (3 days ago) Apr 24
      to Takashi Toyoshima, Simon Hangl, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Raphael Kubo da Costa, asvitkine...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, tommyw+w...@chromium.org
      Attention needed from Takashi Toyoshima

      Mark Schillaci added 2 comments

      Patchset-level comments
      Mark Schillaci . resolved

      Thank you Takashi for the review!

      Commit Message
      Line 9, Patchset 1 (Latest):This histogram expired on 2023-05-01.
      Takashi Toyoshima . unresolved

      Can you explain a bit about SuppressLatencyRecording()?
      This looks something more than just removing expired histograms.

      Mark Schillaci

      Yeah we ended up adding a new method here because it is a shared class. ScriptPromiseResolverWithTracker is a helper used by multiple features to record both ".Result" and ".Latency" histograms (or ".Latency2" in one case).

      We only want to remove the ".Latency" histogram recording from this one instance (media_devices.cc), but the other clients of SPRWT are tracking ".Latency" histograms that are still active and won't be deleted. So we need clients to be able to turn on/off the recording of the ".Latency" histogram. (Usually these histogram removal CLs look like removing a RecordHistogram() line in the code, but here it's a guard clause to prevent going down a branch).

      An alternative might be to add a boolean to the ctor of SPRWT for `bool record_latency = true`. That way, we wouldn't need to update the other call sites at all, and the media_devices call site would just look like `..., /*record_latency=*/false);`. This avoids adding a new method.

      lmkwyt, thanks!

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Takashi Toyoshima
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I12773a1f9488c06d70a1c5112461dd187e93349d
      Gerrit-Change-Number: 7765804
      Gerrit-PatchSet: 1
      Gerrit-Owner: Mark Schillaci <mschi...@google.com>
      Gerrit-Reviewer: Mark Schillaci <mschi...@google.com>
      Gerrit-Reviewer: Simon Hangl <sim...@google.com>
      Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
      Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
      Gerrit-CC: Kentaro Hara <har...@chromium.org>
      Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
      Gerrit-Attention: Takashi Toyoshima <toyo...@chromium.org>
      Gerrit-Comment-Date: Fri, 24 Apr 2026 21:17:26 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Takashi Toyoshima <toyo...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages