Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[cr149] PageActionPerActionMetricsRecorder renamed

Browse Source 
This PR fixes the plaster for `PageActionPerActionMetricsRecorder`,
which has to use the new `PageActionMetricsRecorder`. This PR also
improves the plaster to disable any `Record` functions in this class, so
we can notified when new ones are added.

Chromium changes:
https://chromium.googlesource.com/chromium/src/+/42d106dd97f5729ff8ccf50171f3dc87798d76d8

commit 42d106dd97f5729ff8ccf50171f3dc87798d76d8
Author: Kaan Alsan <alsan@chromium.org>
Date:   Fri Apr 24 14:29:50 2026 -0700

    Consolidate PageAction Metrics Recording using ScopedMultiSourceObservation

    This CL transitions the PageAction metrics recording system from
    multiple ScopedObservation-based recorders to a single
    PageActionMetricsRecorder using ScopedMultiSourceObservation.

    Previously, we had count(page actions) * count(tabs) metrics recorder
    objects, which duplicated GURLs and created many individual scoped
    observations. This CL improves efficiency by sharing navigation state
    across all actions in a tab and reducing the total number of
    observations. With local profiling (with debug parameters on), this
    saves ~100KB per tab, with 31 page actions enabled.

    Key changes:
    - Merged per-action and page-level metrics logic into a unified
      PageActionMetricsRecorder.
    - Updated PageActionModelInterface to include GetActionId() for
      identification in the consolidated recorder.
    - Refactored PageActionControllerImpl to manage the unified recorder.
    - Deleted the redundant PageActionPageMetricsRecorder.
    - Updated unit tests and test support classes to align with the new
      architecture.

    Bug: 384074251
    Change-Id: I1aeb2205b4d36de3af68d70404ce459babf1dbef
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7790756
    Commit-Queue: Kaan Alsan <alsan@chromium.org>
    Reviewed-by: Foromo Daniel Soromou <koretadaniel@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#1620471}
This commit is contained in:
Claudio DeSouza
2026-05-22 16:25:14 -04:00
committed by Max Karolinskiy
parent ded7386ce6
commit d38d733dde
3 changed files with 56 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
chromium_src/chrome/browser/ui/views/page_action/page_action_metrics_recorder.h
-30
View File
@@ -1,30 +0,0 @@
// Copyright (c) 2026 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.
#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_METRICS_RECORDER_H_
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_METRICS_RECORDER_H_
// Scrubs out histogramming calls
#define RecordIconShown() \
RecordIconShown() {} \
void RecordIconShown_Chromium()
#define RecordChipShown() \
RecordChipShown() {} \
void RecordChipShown_Chromium()
#define RecordIconClick() \
RecordIconClick() {} \
void RecordIconClick_Chromium()
#define RecordChipClick() \
RecordChipClick() {} \
void RecordChipClick_Chromium()
#include <chrome/browser/ui/views/page_action/page_action_metrics_recorder.h> // IWYU pragma: export
#undef RecordChipClick
#undef RecordIconClick
#undef RecordChipShown
#undef RecordIconShown
#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_METRICS_RECORDER_H_
patches/chrome-browser-ui-views-page_action-page_action_metrics_recorder.cc.patch
+52 -32
View File
@@ -1,40 +1,60 @@
diff --git a/chrome/browser/ui/views/page_action/page_action_metrics_recorder.cc b/chrome/browser/ui/views/page_action/page_action_metrics_recorder.cc
index f5cf8379758e5d7bbbb42761a452f7e279460851..2b6ca4d1c131babea6e43efd4c9ba118659650c8 100644
index 9752c9a18eee13e994695d36000e35de5f29999a..8e46749d6553cde220374a9b7f377672b1eda340 100644
--- a/chrome/browser/ui/views/page_action/page_action_metrics_recorder.cc
+++ b/chrome/browser/ui/views/page_action/page_action_metrics_recorder.cc
@@ -105,7 +105,7 @@ bool PageActionPerActionMetricsRecorder::IsNewNavigation() {
return false;
}
@@ -45,6 +45,7 @@ PageActionMetricsRecorder::~PageActionMetricsRecorder() {
-void PageActionPerActionMetricsRecorder::RecordIconShown() {
+void PageActionPerActionMetricsRecorder::RecordIconShown_Chromium() {
if (current_navigation_metrics_.icon_shown_recorded) {
void PageActionMetricsRecorder::RecordClick(actions::ActionId action_id,
PageActionTrigger trigger_source) {
+ if ((true)) return;
auto it = per_action_states_.find(action_id);
if (it == per_action_states_.end()) {
return;
}
@@ -121,7 +121,7 @@ void PageActionPerActionMetricsRecorder::RecordIconShown() {
page_action_type_);
@@ -203,6 +204,7 @@ void PageActionMetricsRecorder::CheckForNewNavigation() {
}
-void PageActionPerActionMetricsRecorder::RecordChipShown() {
+void PageActionPerActionMetricsRecorder::RecordChipShown_Chromium() {
if (current_navigation_metrics_.chip_shown_recorded) {
void PageActionMetricsRecorder::RecordPageShownMetrics() {
+ if ((true)) return;
if (!has_recorded_page_shown_for_navigation_) {
RecordPageEvent(PageActionPageEvent::kPageShown);
has_recorded_page_shown_for_navigation_ = true;
@@ -214,6 +216,7 @@ void PageActionMetricsRecorder::RecordPageShownMetrics() {
}
void PageActionMetricsRecorder::RecordIconShown(actions::ActionId action_id) {
+ if ((true)) return;
auto it = per_action_states_.find(action_id);
CHECK(it != per_action_states_.end());
auto& state = it->second;
@@ -235,6 +238,7 @@ void PageActionMetricsRecorder::RecordIconShown(actions::ActionId action_id) {
}
void PageActionMetricsRecorder::RecordChipShown(actions::ActionId action_id) {
+ if ((true)) return;
auto it = per_action_states_.find(action_id);
CHECK(it != per_action_states_.end());
auto& state = it->second;
@@ -256,6 +260,7 @@ void PageActionMetricsRecorder::RecordChipShown(actions::ActionId action_id) {
}
void PageActionMetricsRecorder::RecordIconClick(actions::ActionId action_id) {
+ if ((true)) return;
auto it = per_action_states_.find(action_id);
CHECK(it != per_action_states_.end());
auto& state = it->second;
@@ -272,6 +277,7 @@ void PageActionMetricsRecorder::RecordIconClick(actions::ActionId action_id) {
}
void PageActionMetricsRecorder::RecordChipClick(actions::ActionId action_id) {
+ if ((true)) return;
auto it = per_action_states_.find(action_id);
CHECK(it != per_action_states_.end());
auto& state = it->second;
@@ -289,6 +295,7 @@ void PageActionMetricsRecorder::RecordChipClick(actions::ActionId action_id) {
void PageActionMetricsRecorder::RecordShownPerNavigation(
actions::ActionId action_id) {
+ if ((true)) return;
auto it = per_action_states_.find(action_id);
if (it == per_action_states_.end()) {
return;
}
@@ -153,7 +153,7 @@ void PageActionPerActionMetricsRecorder::RecordClick(
}
}
-void PageActionPerActionMetricsRecorder::RecordIconClick() {
+void PageActionPerActionMetricsRecorder::RecordIconClick_Chromium() {
base::UmaHistogramEnumeration("PageActionController.Icon.CTR2",
PageActionCTREvent::kClicked);
base::UmaHistogramEnumeration(
@@ -164,7 +164,7 @@ void PageActionPerActionMetricsRecorder::RecordIconClick() {
visible_ephemeral_page_actions_count_callback_.Run(), 20);
}
-void PageActionPerActionMetricsRecorder::RecordChipClick() {
+void PageActionPerActionMetricsRecorder::RecordChipClick_Chromium() {
base::UmaHistogramEnumeration("PageActionController.Chip.CTR2",
PageActionCTREvent::kClicked);
base::UmaHistogramEnumeration(
rewrite/chrome/browser/ui/views/page_action/page_action_metrics_recorder.cc.toml
+4 -18
View File
@@ -4,21 +4,7 @@
# You can obtain one at https://mozilla.org/MPL/2.0/.
[[substitution]]
description = 'Replaces RecordChipShown with RecordChipShown_Chromium'
re_pattern = '(void PageActionPerActionMetricsRecorder::RecordChipShown)\(\)'
replace = '\1_Chromium()'
[[substitution]]
description = 'Replaces RecordIconShown with RecordIconShown_Chromium'
re_pattern = '(void PageActionPerActionMetricsRecorder::RecordIconShown)\(\)'
replace = '\1_Chromium()'
[[substitution]]
description = 'Replaces RecordIconClick with RecordIconClick_Chromium'
re_pattern = '(void PageActionPerActionMetricsRecorder::RecordIconClick)\(\)'
replace = '\1_Chromium()'
[[substitution]]
description = 'Replaces RecordChipClick with RecordChipClick_Chromium'
re_pattern = '(void PageActionPerActionMetricsRecorder::RecordChipClick)\(\)'
replace = '\1_Chromium()'
description = 'Early return in all Record* functions to suppress Chromium metrics'
re_pattern = '(void PageActionMetricsRecorder::Record\w+\([^)]*\) \{)'
replace = '\1\n if ((true)) return;'
count = 7