Shunya Shishido (Gerrit)

unread,

Jun 9, 2026, 8:54:01 PM (2 days ago) Jun 9

to Etienne Bergeron, Minoru Chikamune, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, csharris...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org, loading-rev...@chromium.org, bmcquad...@chromium.org

Attention needed from Minoru Chikamune

Shunya Shishido voted and added 1 comment

Votes added by Shunya Shishido

Commit-Queue

+1

1 comment

File components/page_load_metrics/browser/metrics_web_contents_observer.cc

Line 1200, Patchset 3: page_load_custom_timings_.push_back(std::move(custom_timing));

Etienne Bergeron . resolved

shall we not even call push_back(...) when GetPageLoadTrackerIfValid(rfh) is returning null ?

Shunya Shishido

Thank you! Updated not to push_back when the tracker is null. This resulted in removing `page_load_custom_timings_` entirely. We decided not to store the events when the tracker is null. This behavior is consistent with existing page load metrics function e.g., `OnTimingUpdated()`.

Open in Gerrit

Related details

Attention is currently required from:

Minoru Chikamune

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

Shunya Shishido (Gerrit)

unread,

Jun 10, 2026, 4:25:43 AM (yesterday) Jun 10

to Etienne Bergeron, Minoru Chikamune, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, csharris...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org, loading-rev...@chromium.org, bmcquad...@chromium.org

Attention needed from Minoru Chikamune

Shunya Shishido voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Minoru Chikamune

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,

Jun 10, 2026, 4:29:06 AM (yesterday) Jun 10

to Shunya Shishido, Etienne Bergeron, Minoru Chikamune, android-bu...@system.gserviceaccount.com, csharris...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org, loading-rev...@chromium.org, bmcquad...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
Insertions: 12, Deletions: 6.

@@ -1123,17 +1123,23 @@
   CheckNoErrorEvents();
 }
 
-TEST_P(MetricsWebContentsObserverTest, CustomUserTimingBufferCap) {
+TEST_P(MetricsWebContentsObserverTest, CustomUserTimingNoTracker) {
   NavigateToUntrackedUrl();
   content::RenderFrameHost* rfh = web_contents()->GetPrimaryMainFrame();
   mojom::CustomUserTimingMark custom_timing;
   custom_timing.mark_name = "fake_custom_mark";
   custom_timing.start_time = base::Milliseconds(1000);
-  // Enforce 500 mark updates to simulate flooding/bursts in an untracked frame.
-  for (int i = 0; i < 500; ++i) {
-    SimulateCustomUserTimingUpdate(custom_timing, rfh);
-  }
-  EXPECT_EQ(100u, observer()->GetPageLoadCustomTimingsSizeForTesting());
+
+  // When there is no tracker, updates should be dropped immediately rather than
+  // buffered.
+  SimulateCustomUserTimingUpdate(custom_timing, rfh);
+  EXPECT_EQ(0, CountUpdatedCustomUserTimingReported());
+
+  // Navigate to a tracked URL and verify that the dropped timing is NOT
+  // flushed.
+  content::NavigationSimulator::NavigateAndCommitFromBrowser(
+      web_contents(), GURL(kDefaultTestUrl));
+  EXPECT_EQ(0, CountUpdatedCustomUserTimingReported());
 }
 
 class MetricsWebContentsObserverBackForwardCacheTest
```
```
The name of the file: components/page_load_metrics/browser/metrics_web_contents_observer.cc
Insertions: 4, Deletions: 10.

@@ -1193,18 +1193,12 @@
 void MetricsWebContentsObserver::OnCustomUserTimingUpdated(
     content::RenderFrameHost* rfh,
     mojom::CustomUserTimingMarkPtr custom_timing) {
-  // Enforce a ceiling on pre-commit buffering to prevent permanent memory leaks
-  // when custom timings arrive for untracked frames (e.g. blob/about:blank).
-  constexpr size_t kMaxBufferedCustomTimings = 100;
-  if (page_load_custom_timings_.size() < kMaxBufferedCustomTimings) {
-    page_load_custom_timings_.push_back(std::move(custom_timing));
-  }
   TRACE_EVENT("loading",
-              "MetricsWebContentsObserver::OnCustomUserTimingUpdated",
-              "buffered_timings_count", page_load_custom_timings_.size());
+              "MetricsWebContentsObserver::OnCustomUserTimingUpdated");
   if (PageLoadTracker* tracker = GetPageLoadTrackerIfValid(rfh)) {
-    tracker->AddCustomUserTimings(std::move(page_load_custom_timings_));
-    page_load_custom_timings_.clear();
+    std::vector<mojom::CustomUserTimingMarkPtr> custom_timings;
+    custom_timings.push_back(std::move(custom_timing));
+    tracker->AddCustomUserTimings(std::move(custom_timings));
   }
 }
 
```
```
The name of the file: components/page_load_metrics/browser/metrics_web_contents_observer.h
Insertions: 0, Deletions: 6.

@@ -224,10 +224,6 @@
     return weak_ptr_factory_.GetWeakPtr();
   }
 
-  size_t GetPageLoadCustomTimingsSizeForTesting() const {  // IN-TEST
-    return page_load_custom_timings_.size();
-  }
-
  protected:
   // Protected rather than private so that derived test classes can call
   // constructor.
@@ -413,8 +409,6 @@
   base::flat_map<content::RenderFrameHost*, std::unique_ptr<PageLoadTracker>>
       inactive_pages_;
 
-  std::vector<mojom::CustomUserTimingMarkPtr> page_load_custom_timings_;
-
   // Has the MWCO observed at least one navigation?
   bool has_navigated_;
 
```

Change information

Commit message:

Do not buffer custom user timings when PageLoadTracker is null

Previously, MetricsWebContentsObserver buffered custom user timings in
`page_load_custom_timings_` when received for frames without an active
PageLoadTracker. This could potentially lead to unbounded memory growth
or unnecessary complexity in managing buffer caps.

This CL simplifies the design by removing the
`page_load_custom_timings_` buffer and member variable entirely, and
dropping custom user timing updates in `OnCustomUserTimingUpdated()` if
a valid PageLoadTracker does not exist for the given RenderFrameHost.

We don't expect this code will be used for a long time, as we'll soon
launch `ThrottleSendingCustomUserTimings` across platforms. After the
launch, we can remove `OnCustomUserTimingUpdated()` entirely from the
code base.

Bug: 467177770, 511200987

Change-Id: I8841cbd2bbe985cf94f9ca200edc6133fa40d03f

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

Commit-Queue: Shunya Shishido <sisid...@chromium.org>

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

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

Files:

M components/page_load_metrics/browser/metrics_web_contents_observer.cc
M components/page_load_metrics/browser/metrics_web_contents_observer.h
M components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc

Change size: S

Delta: 3 files changed, 23 insertions(+), 8 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Etienne Bergeron

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Do not buffer custom user timings when PageLoadTracker is null [chromium/src : main]

Shunya Shishido (Gerrit)

Shunya Shishido voted and added 1 comment

Votes added by Shunya Shishido

1 comment

Related details

Shunya Shishido (Gerrit)

Shunya Shishido voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information