Stop sending performance timing mark with dedicated IPC [chromium/src : main]

0 views
Skip to first unread message

Shunya Shishido (Gerrit)

unread,
Feb 17, 2026, 7:03:55 AM (3 days ago) Feb 17
to Takashi Toyoshima, Keita Suzuki, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Keita Suzuki and Takashi Toyoshima

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:
  • Keita Suzuki
  • Takashi Toyoshima
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: I07313e4561fd3c5cf0fff660f226bc107eaf4daf
Gerrit-Change-Number: 7239589
Gerrit-PatchSet: 9
Gerrit-Owner: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Reviewer: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
Gerrit-Attention: Takashi Toyoshima <toyo...@chromium.org>
Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Comment-Date: Tue, 17 Feb 2026 12:03:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Shunya Shishido (Gerrit)

unread,
Feb 19, 2026, 10:57:15 PM (15 hours ago) Feb 19
to Takashi Toyoshima, Keita Suzuki, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Keita Suzuki

Shunya Shishido added 3 comments

File components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
Line 1106, Patchset 10: feature_list.InitWithFeatures({},
Takashi Toyoshima . resolved

nit: InitAndDisableFeature(features::kThrottleSendingCustomUserTimings) is more common for such a simple case?

Shunya Shishido

Done

Line 1122, Patchset 10: base::test::ScopedFeatureList feature_list;
Takashi Toyoshima . resolved

ditto; InitAndEnableFeature

Or, can we just make the existing test parameterized?

Shunya Shishido

I parameterized existing tests.

File components/page_load_metrics/renderer/page_timing_metrics_sender.cc
Line 368, Patchset 10:
Takashi Toyoshima . resolved

nit: We don't need to call clear()s for moved members?

Shunya Shishido

Done. I removed other moved members calling clear().

Open in Gerrit

Related details

Attention is currently required from:
  • Keita Suzuki
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement 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: I07313e4561fd3c5cf0fff660f226bc107eaf4daf
Gerrit-Change-Number: 7239589
Gerrit-PatchSet: 12
Gerrit-Owner: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Reviewer: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Comment-Date: Fri, 20 Feb 2026 03:56:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Takashi Toyoshima <toyo...@chromium.org>
satisfied_requirement
open
diffy

Shunya Shishido (Gerrit)

unread,
1:04 AM (12 hours ago) 1:04 AM
to Takashi Toyoshima, Keita Suzuki, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Keita Suzuki

Shunya Shishido voted Commit-Queue+2

Commit-Queue+2
Open in Gerrit

Related details

Attention is currently required from:
  • Keita Suzuki
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement 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: I07313e4561fd3c5cf0fff660f226bc107eaf4daf
Gerrit-Change-Number: 7239589
Gerrit-PatchSet: 14
Gerrit-Owner: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Reviewer: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
Gerrit-Attention: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Comment-Date: Fri, 20 Feb 2026 06:04:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Chromium LUCI CQ (Gerrit)

unread,
1:07 AM (12 hours ago) 1:07 AM
to Shunya Shishido, Takashi Toyoshima, Keita Suzuki, AyeAye, chromium...@chromium.org, bmcquad...@chromium.org, csharris...@chromium.org, ipc-securi...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

10 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/renderer/page_timing_metrics_sender.cc
Insertions: 0, Deletions: 1.

@@ -374,7 +374,6 @@
last_cpu_timing_->task_time = base::TimeDelta();
modified_resources_.clear();
render_data_.new_layout_shifts.clear();
- custom_user_timings_.clear();
// As PageTimingMetricsSender is owned by MetricsRenderFrameObserver, which is
// instantiated for each frame, there's no need to make soft_navigation_count_
// zero here, as its value only increments through the lifetime of the frame.
```
```
The name of the file: components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
Insertions: 31, Deletions: 49.

@@ -82,7 +82,8 @@
} // namespace

class MetricsWebContentsObserverTest
- : public content::RenderViewHostTestHarness {
+ : public content::RenderViewHostTestHarness,
+ public testing::WithParamInterface<bool> {
public:
MetricsWebContentsObserverTest() {
mojom::PageLoadTiming timing;
@@ -287,7 +288,9 @@
raw_ptr<content::ContentBrowserClient> original_browser_client_ = nullptr;
};

-TEST_F(MetricsWebContentsObserverTest, SuccessfulMainFrameNavigation) {
+INSTANTIATE_TEST_SUITE_P(All, MetricsWebContentsObserverTest, testing::Bool());
+
+TEST_P(MetricsWebContentsObserverTest, SuccessfulMainFrameNavigation) {
mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
timing.navigation_start = base::Time::FromSecondsSinceUnixEpoch(1);
@@ -320,7 +323,7 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest,
+TEST_P(MetricsWebContentsObserverTest,
DISABLED_MainFrameNavigationInternalAbort) {
auto navigation = content::NavigationSimulator::CreateBrowserInitiated(
GURL(kDefaultTestUrl), web_contents());
@@ -329,7 +332,7 @@
ASSERT_EQ(kDefaultTestUrl, observed_aborted_urls().front().spec());
}

-TEST_F(MetricsWebContentsObserverTest, SubFrame) {
+TEST_P(MetricsWebContentsObserverTest, SubFrame) {
mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
timing.navigation_start = base::Time::FromSecondsSinceUnixEpoch(1);
@@ -381,7 +384,7 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, SameDocumentNoTrigger) {
+TEST_P(MetricsWebContentsObserverTest, SameDocumentNoTrigger) {
mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
timing.navigation_start = base::Time::FromSecondsSinceUnixEpoch(1);
@@ -413,7 +416,7 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, DontLogIrrelevantNavigation) {
+TEST_P(MetricsWebContentsObserverTest, DontLogIrrelevantNavigation) {
mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
timing.navigation_start = base::Time::FromSecondsSinceUnixEpoch(10);
@@ -438,7 +441,7 @@
CheckTotalErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, EmptyTimingError) {
+TEST_P(MetricsWebContentsObserverTest, EmptyTimingError) {
// Page load timing errors are not being reported when the error occurs for a
// page that gets preserved in the back/forward cache.
// TODO(crbug.com/40213776): Fix this.
@@ -466,7 +469,7 @@
page_load_metrics::internal::INVALID_EMPTY_TIMING, 1);
}

-TEST_F(MetricsWebContentsObserverTest, NullNavigationStartError) {
+TEST_P(MetricsWebContentsObserverTest, NullNavigationStartError) {
// Page load timing errors are not being reported when the error occurs for a
// page that gets preserved in the back/forward cache.
// TODO(crbug.com/40213776): Fix this.
@@ -495,7 +498,7 @@
page_load_metrics::internal::INVALID_NULL_NAVIGATION_START, 1);
}

-TEST_F(MetricsWebContentsObserverTest, TimingOrderError) {
+TEST_P(MetricsWebContentsObserverTest, TimingOrderError) {
// Page load timing errors are not being reported when the error occurs for a
// page that gets preserved in the back/forward cache.
// TODO(crbug.com/40213776): Fix this.
@@ -525,7 +528,7 @@
page_load_metrics::internal::INVALID_ORDER_PARSE_START_PARSE_STOP, 1);
}

-TEST_F(MetricsWebContentsObserverTest, BadIPC) {
+TEST_P(MetricsWebContentsObserverTest, BadIPC) {
mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
timing.navigation_start = base::Time::FromSecondsSinceUnixEpoch(10);
@@ -545,7 +548,7 @@
CheckTotalErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, ObservePartialNavigation) {
+TEST_P(MetricsWebContentsObserverTest, ObservePartialNavigation) {
// Reset the state of the tests, and attach the MetricsWebContentsObserver in
// the middle of a navigation. This tests that the class is robust to only
// observing some of a navigation.
@@ -576,7 +579,7 @@
CheckTotalErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, StopObservingOnCommit) {
+TEST_P(MetricsWebContentsObserverTest, StopObservingOnCommit) {
ASSERT_TRUE(completed_filtered_urls().empty());

content::NavigationSimulator::NavigateAndCommitFromBrowser(
@@ -601,7 +604,7 @@
completed_filtered_urls());
}

-TEST_F(MetricsWebContentsObserverTest, StopObservingOnStart) {
+TEST_P(MetricsWebContentsObserverTest, StopObservingOnStart) {
ASSERT_TRUE(completed_filtered_urls().empty());

content::NavigationSimulator::NavigateAndCommitFromBrowser(
@@ -628,7 +631,7 @@

// We buffer cross frame timings in order to provide a consistent view of
// timing data to observers. See crbug.com/722860 for more.
-TEST_F(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming) {
+TEST_P(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming) {
mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
timing.navigation_start = base::Time::FromSecondsSinceUnixEpoch(1);
@@ -692,7 +695,7 @@

// We buffer cross-frame paint updates to account for paint timings from
// different frames arriving out of order.
-TEST_F(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming2) {
+TEST_P(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming2) {
// Dispatch a timing update for the main frame that includes a first
// paint. This should be buffered, with the dispatch timer running.
mojom::PageLoadTiming timing;
@@ -771,7 +774,7 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, FlushBufferOnAppBackground) {
+TEST_P(MetricsWebContentsObserverTest, FlushBufferOnAppBackground) {
mojom::PageLoadTiming timing;
PopulatePageLoadTiming(&timing);
timing.paint_timing->first_paint = base::Milliseconds(100000);
@@ -784,7 +787,7 @@
ASSERT_EQ(1, CountUpdatedTimingReported());
}

-TEST_F(MetricsWebContentsObserverTest,
+TEST_P(MetricsWebContentsObserverTest,
FirstInputDelayMissingFirstInputTimestamp) {
mojom::PageLoadTiming timing;
PopulatePageLoadTiming(&timing);
@@ -813,7 +816,7 @@
CheckTotalErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest,
+TEST_P(MetricsWebContentsObserverTest,
FirstInputTimestampMissingFirstInputDelay) {
mojom::PageLoadTiming timing;
PopulatePageLoadTiming(&timing);
@@ -845,7 +848,7 @@
// Main frame delivers an input notification. Subsequently, a subframe delivers
// an input notification, where the input occurred first. Verify that
// FirstInputDelay and FirstInputTimestamp come from the subframe.
-TEST_F(MetricsWebContentsObserverTest,
+TEST_P(MetricsWebContentsObserverTest,
FirstInputDelayAndTimingSubframeFirstDeliveredSecond) {
mojom::PageLoadTiming timing;
PopulatePageLoadTiming(&timing);
@@ -894,7 +897,7 @@
// A subframe delivers an input notification. Subsequently, the mainframe
// delivers an input notification, where the input occurred first. Verify that
// FirstInputDelay and FirstInputTimestamp come from the main frame.
-TEST_F(MetricsWebContentsObserverTest,
+TEST_P(MetricsWebContentsObserverTest,
FirstInputDelayAndTimingMainframeFirstDeliveredSecond) {
// We need to navigate before we can navigate the subframe.
content::NavigationSimulator::NavigateAndCommitFromBrowser(
@@ -945,7 +948,7 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, DispatchDelayedMetricsOnPageClose) {
+TEST_P(MetricsWebContentsObserverTest, DispatchDelayedMetricsOnPageClose) {
mojom::PageLoadTiming timing;
PopulatePageLoadTiming(&timing);
timing.paint_timing->first_paint = base::Milliseconds(1000);
@@ -977,7 +980,7 @@
}

// Make sure the dispatch of CPU occurs immediately.
-TEST_F(MetricsWebContentsObserverTest, DispatchCpuMetricsImmediately) {
+TEST_P(MetricsWebContentsObserverTest, DispatchCpuMetricsImmediately) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL(kDefaultTestUrl));

@@ -997,7 +1000,7 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, OnLoadedResource_MainFrame) {
+TEST_P(MetricsWebContentsObserverTest, OnLoadedResource_MainFrame) {
GURL main_resource_url(kDefaultTestUrl);
content::NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(),
main_resource_url);
@@ -1031,7 +1034,7 @@
loaded_resources().back().final_url);
}

-TEST_F(MetricsWebContentsObserverTest, OnLoadedResource_Subresource) {
+TEST_P(MetricsWebContentsObserverTest, OnLoadedResource_Subresource) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL(kDefaultTestUrl));
GURL loaded_resource_url("http://www.other.com/");
@@ -1045,7 +1048,7 @@
loaded_resources().back().final_url);
}

-TEST_F(MetricsWebContentsObserverTest,
+TEST_P(MetricsWebContentsObserverTest,
OnLoadedResource_ResourceFromOtherRFHIgnored) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL(kDefaultTestUrl));
@@ -1065,7 +1068,7 @@
EXPECT_TRUE(loaded_resources().empty());
}

-TEST_F(MetricsWebContentsObserverTest, RecordFeatureUsage) {
+TEST_P(MetricsWebContentsObserverTest, RecordFeatureUsage) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL(kDefaultTestUrl));
ASSERT_EQ(main_rfh()->GetLastCommittedURL().spec(), GURL(kDefaultTestUrl));
@@ -1089,7 +1092,7 @@
EXPECT_EQ(observed_features()[1], feature2);
}

-TEST_F(MetricsWebContentsObserverTest, RecordFeatureUsageNoObserver) {
+TEST_P(MetricsWebContentsObserverTest, RecordFeatureUsageNoObserver) {
// Reset the state of the tests, and don't add an observer.
DeleteContents();
SetContents(CreateTestWebContents());
@@ -1101,10 +1104,7 @@
blink::mojom::WebFeature::kFormAttribute});
}

-TEST_F(MetricsWebContentsObserverTest, CustomUserTiming) {
- base::test::ScopedFeatureList feature_list;
- feature_list.InitWithFeatures({},
- {features::kThrottleSendingCustomUserTimings});
+TEST_P(MetricsWebContentsObserverTest, CustomUserTiming) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL(kDefaultTestUrl));
content::RenderFrameHost* rfh = web_contents()->GetPrimaryMainFrame();
@@ -1118,24 +1118,6 @@
CheckNoErrorEvents();
}

-TEST_F(MetricsWebContentsObserverTest, CustomUserTimingWithThrottling) {
- base::test::ScopedFeatureList feature_list;
- feature_list.InitWithFeatures({features::kThrottleSendingCustomUserTimings},
- {});
- content::NavigationSimulator::NavigateAndCommitFromBrowser(
- web_contents(), GURL(kDefaultTestUrl));
- content::RenderFrameHost* rfh = web_contents()->GetPrimaryMainFrame();
-
- mojom::CustomUserTimingMark custom_timing;
- custom_timing.mark_name = "fake_custom_mark";
- custom_timing.start_time = base::Milliseconds(1000);
-
- SimulateCustomUserTimingUpdate(custom_timing, rfh);
- ASSERT_EQ(1, CountUpdatedCustomUserTimingReported());
- EXPECT_TRUE(custom_timing.Equals(*updated_custom_user_timings().back()));
- CheckNoErrorEvents();
-}
-
class MetricsWebContentsObserverBackForwardCacheTest
: public MetricsWebContentsObserverTest,
public content::WebContentsDelegate {
```

Change information

Commit message:
Stop sending performance timing mark with dedicated IPC

crrev.com/c/5626378 introduced `AddCustomUserTiming()` mojo call so that
the renderer immediately sends performance mark event to the browser. We
thought this is OK, as we limit sending events from the main frame only.

But now we want to send events from subframes as well. This may increase
the number of events, which directly increases the number of IPCs. To
prevent that, this CL updates the existing message to include
performance mark events, and make the IPC send timing throttled.

As a tradeoff, this may move the metrics a bit (as the send timing is
changed), but instead we can keep the performance impact minimal.


Bug: 467177770

Bundle custom user timings into existing throttled IPC

Previously, custom user timings were sent from the renderer to the
browser immediately using a dedicated Mojo call. This was acceptable
when limited only to the main frame, but extending support to subframes
risks a significant increase in IPC volume.

To address this, update the existing page load timing message to include
custom user timing marks. When the throttling feature is enabled, these
events are bundled and sent via the existing throttled IPC mechanism
instead of using the dedicated call.

While this change may result in slight shifts in reported metrics due to
the delay in transmission, it ensures that the performance impact of
reporting these events remains minimal as subframe support is
introduced.
Bug: 467177770
Change-Id: I07313e4561fd3c5cf0fff660f226bc107eaf4daf
Commit-Queue: Shunya Shishido <sisid...@chromium.org>
Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1587627}
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
  • M components/page_load_metrics/browser/observers/ad_metrics/ads_page_load_metrics_observer_unittest.cc
  • M components/page_load_metrics/browser/observers/page_load_metrics_observer_tester.cc
  • M components/page_load_metrics/common/page_load_metrics.mojom
  • M components/page_load_metrics/renderer/fake_page_timing_sender.cc
  • M components/page_load_metrics/renderer/fake_page_timing_sender.h
  • M components/page_load_metrics/renderer/metrics_render_frame_observer.cc
  • M components/page_load_metrics/renderer/page_timing_metrics_sender.cc
  • M components/page_load_metrics/renderer/page_timing_metrics_sender.h
  • M components/page_load_metrics/renderer/page_timing_sender.h
Change size: M
Delta: 12 files changed, 87 insertions(+), 49 deletions(-)
Branch: refs/heads/main
Submit Requirements:
  • requirement satisfiedCode-Review: +1 by Takashi Toyoshima
Open in Gerrit
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: merged
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I07313e4561fd3c5cf0fff660f226bc107eaf4daf
Gerrit-Change-Number: 7239589
Gerrit-PatchSet: 15
Gerrit-Owner: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
Gerrit-Reviewer: Keita Suzuki <suzuk...@chromium.org>
Gerrit-Reviewer: Shunya Shishido <sisid...@chromium.org>
Gerrit-Reviewer: Takashi Toyoshima <toyo...@chromium.org>
open
diffy
satisfied_requirement
Reply all
Reply to author
Forward
0 new messages