Skip to content

Commit

Permalink
Remove suffixes OnBattery, NotOnBattery from FCP
Browse files Browse the repository at this point in the history
This change removes histogram suffixes
PageLoad.PaintTiming.NavigationToFirstContentfulPaint.OnBattery and
PageLoad.PaintTiming.NavigationToFirstContentfulPaint.NotOnBattery,
which allowed the filtering of FirstContentfulPaint measurements by
whether the system was on battery or wall power during page load.

This was used to filter FirstContentfulPaint regressions for
battery-specific Finch experiment SlowDCTimerInterruptsWin, which is
now on Stable. Because the experiment is finished and I'm not aware
of any other users, this histogram can be removed.

Change-Id: I78a6a2026596cb9aaa0783129ee3f4d8034fc039
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468337
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Cr-Commit-Position: refs/heads/master@{#817559}
  • Loading branch information
jessemckenna authored and Commit Bot committed Oct 15, 2020
1 parent 0d9ca02 commit 0300ce5
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include "base/feature_list.h"
#include "base/metrics/histogram_macros.h"
#include "base/power_monitor/power_monitor.h"
#include "components/page_load_metrics/browser/observers/core/largest_contentful_paint_handler.h"
#include "components/page_load_metrics/browser/page_load_metrics_util.h"
#include "content/public/common/process_type.h"
Expand Down Expand Up @@ -187,11 +186,6 @@ const char kHistogramParseBlockedOnScriptExecutionDocumentWrite[] =
const char kHistogramFirstContentfulPaintNoStore[] =
"PageLoad.PaintTiming.NavigationToFirstContentfulPaint.NoStore";

const char kHistogramFirstContentfulPaintOnBattery[] =
"PageLoad.PaintTiming.NavigationToFirstContentfulPaint.OnBattery";
const char kHistogramFirstContentfulPaintNotOnBattery[] =
"PageLoad.PaintTiming.NavigationToFirstContentfulPaint.NotOnBattery";

const char kHistogramFirstContentfulPaintHiddenWhileFlushing[] =
"PageLoad.PaintTiming.NavigationToFirstContentfulPaint.HiddenWhileFlushing";

Expand Down Expand Up @@ -522,14 +516,6 @@ void UmaPageLoadMetricsObserver::OnFirstContentfulPaintInPage(
timing.paint_timing->first_contentful_paint.value());
}

if (base::PowerMonitor::IsOnBatteryPower()) {
PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstContentfulPaintOnBattery,
timing.paint_timing->first_contentful_paint.value());
} else {
PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstContentfulPaintNotOnBattery,
timing.paint_timing->first_contentful_paint.value());
}

// TODO(bmcquade): consider adding a histogram that uses
// UserInputInfo.user_input_event.
if (GetDelegate().GetUserInitiatedInfo().browser_initiated ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@ class UmaPageLoadMetricsObserverTest
void SetUp() override {
page_load_metrics::PageLoadMetricsObserverContentTestHarness::SetUp();
page_load_metrics::LargestContentfulPaintHandler::SetTestMode(true);

base::PowerMonitor::Initialize(
std::make_unique<base::PowerMonitorTestSource>());
}

void TearDown() override {
base::PowerMonitor::ShutdownForTesting();
page_load_metrics::PageLoadMetricsObserverContentTestHarness::TearDown();
}

void OnCpuTimingUpdate(RenderFrameHost* render_frame_host,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7902,6 +7902,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</histogram_suffixes>

<histogram_suffixes name="IsOnBattery" separator=".">
<obsolete>
Removed 2020-10.
</obsolete>
<suffix name="NotOnBattery" label="System is on external power."/>
<suffix name="OnBattery" label="System is on battery power."/>
<affected-histogram
Expand Down

0 comments on commit 0300ce5

Please sign in to comment.