Commit 3e2fac0a authored by Yaron Friedman's avatar Yaron Friedman Committed by Commit Bot

Deprecate PageLoad user action.

Turns out it's not very accurate at all and often gets misinterpeted as
representing user behaviour.  Deprecate it and point to
PageLoad.Timing2.NavigationToCommit which is more accurate for user
behaviour.

See also: https://docs.google.com/document/d/1TUvbPhbcuKGPrwx1iqUIisiV9YVWrz_SDTfdEz0_SDM/edit?ts=5ebc27cc#

Change-Id: Ic748fa3f3722237b5628353f680a13d704ff5840
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198684Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780076}
parent 5f27c4c2
......@@ -8,7 +8,6 @@
#include "base/macros.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h"
#include "components/metrics/stability_metrics_helper.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/testing_pref_service.h"
......@@ -61,7 +60,6 @@ class TestMetricsProvider : public AwStabilityMetricsProvider {
TEST_F(AwStabilityMetricsProviderTest, PageLoadCount) {
base::HistogramTester histogram_tester;
base::UserActionTester user_action_tester;
TestMetricsProvider provider(prefs());
metrics::SystemProfileProto system_profile;
......@@ -73,7 +71,6 @@ TEST_F(AwStabilityMetricsProviderTest, PageLoadCount) {
provider.ProvideStabilityMetrics(&system_profile);
EXPECT_EQ(1, system_profile.stability().page_load_count());
EXPECT_EQ(1, user_action_tester.GetActionCount("PageLoad"));
histogram_tester.ExpectUniqueSample("Stability.Experimental.Counts",
metrics::StabilityEventType::kPageLoad,
1);
......
......@@ -12,8 +12,6 @@
#include "base/check.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/notreached.h"
#include "base/system/sys_info.h"
#include "build/build_config.h"
#include "build/buildflag.h"
......@@ -211,7 +209,6 @@ void StabilityMetricsHelper::BrowserChildProcessCrashed() {
}
void StabilityMetricsHelper::LogLoadStarted() {
base::RecordAction(base::UserMetricsAction("PageLoad"));
IncrementPrefValue(prefs::kStabilityPageLoadCount);
IncrementLongPrefsValue(prefs::kUninstallMetricsPageLoadCount);
RecordStabilityEvent(StabilityEventType::kPageLoad);
......
......@@ -55,7 +55,7 @@ TEST_F(ApplicationBreadcrumbsLoggerTest, UserAction) {
// Tests that not_user_triggered User Action does not show up in breadcrumbs.
TEST_F(ApplicationBreadcrumbsLoggerTest, LogNotUserTriggeredAction) {
base::RecordAction(base::UserMetricsAction("PageLoad"));
base::RecordAction(base::UserMetricsAction("ActiveTabChanged"));
EXPECT_EQ(0U, breadcrumb_manager_.GetEvents(0).size());
}
......
......@@ -18077,6 +18077,12 @@ should be able to be added at any place in this file.
</action>
<action name="PageLoad" not_user_triggered="true">
<obsolete>
Deprecated. Not a good representation of user behaviour for page loads.
Check the count of PageLoad.ParseTiming.NavigationToParseStart or
IOS.PageLoadCount.Counts (on that platform) for a better approximation of
page loads.
</obsolete>
<owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<description>
When a page load starts. This can include page loads that are not completed,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment