Commit 2dcabaf2 authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[CPM Update] Split LoadingStarted event out of navigation histogram.

This way the events in navigation histogram are mutually exclusive.
Also updated histograms.xml and enum.xml which I forgot to update earlier.

Bug: 786551
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I19152cc808e41d6d7a019875145b895a0e48ba0e
Reviewed-on: https://chromium-review.googlesource.com/786670
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519815}
parent 2ae71b61
...@@ -22,8 +22,10 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider, ...@@ -22,8 +22,10 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider,
public web::GlobalWebStateObserver { public web::GlobalWebStateObserver {
public: public:
// Buckets for the histogram that counts events relevant for counting page // Buckets for the histogram that counts events relevant for counting page
// loads. The *_NAVIGATION events are mutually exclusive. // loads. These events are mutually exclusive.
enum class StabilityMetricEventType { // These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class PageLoadCountNavigationType {
// A chrome:// URL navigation. This is not counted for page load. // A chrome:// URL navigation. This is not counted for page load.
CHROME_URL_NAVIGATION = 0, CHROME_URL_NAVIGATION = 0,
// A same-document web (i.e. not chrome:// URL) navigation. This is not // A same-document web (i.e. not chrome:// URL) navigation. This is not
...@@ -32,10 +34,10 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider, ...@@ -32,10 +34,10 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider,
// A navigation that is not SAME_DOCUMENT_WEB or CHROME_URL. It is counted // A navigation that is not SAME_DOCUMENT_WEB or CHROME_URL. It is counted
// as a page load. // as a page load.
PAGE_LOAD_NAVIGATION = 2, PAGE_LOAD_NAVIGATION = 2,
// A loading start event. This is the legacy page load count.
// TODO(crbug.com/786547): Deprecate this counter after page load count cuts // OBSOLETE VALUES. DO NOT REUSE.
// over to be based on DidStartNavigation. OBSOLETE_LOADING_STARTED = 3,
LOADING_STARTED = 3,
COUNT COUNT
}; };
...@@ -59,7 +61,8 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider, ...@@ -59,7 +61,8 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider,
// Records a renderer process crash. // Records a renderer process crash.
void LogRendererCrash(); void LogRendererCrash();
static const char kPageLoadCountMigrationEventKey[]; static const char kPageLoadCountLoadingStartedMetric[];
static const char kPageLoadCountMetric[];
private: private:
metrics::StabilityMetricsHelper helper_; metrics::StabilityMetricsHelper helper_;
......
...@@ -13,9 +13,13 @@ ...@@ -13,9 +13,13 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
// Name of the UMA enum histogram that counts DidStartNavigation events by type.
const char IOSChromeStabilityMetricsProvider::kPageLoadCountMetric[] =
"IOS.PageLoadCount.Counts";
// Name of the UMA enum history that counts DidStartLoading events.
const char const char
IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey[] = IOSChromeStabilityMetricsProvider::kPageLoadCountLoadingStartedMetric[] =
"IOS.PageLoadCountMigration.Counts"; "IOS.PageLoadCount.LoadingStarted";
IOSChromeStabilityMetricsProvider::IOSChromeStabilityMetricsProvider( IOSChromeStabilityMetricsProvider::IOSChromeStabilityMetricsProvider(
PrefService* local_state) PrefService* local_state)
...@@ -58,9 +62,7 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartLoading( ...@@ -58,9 +62,7 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartLoading(
if (!recording_enabled_) if (!recording_enabled_)
return; return;
UMA_HISTOGRAM_ENUMERATION(kPageLoadCountMigrationEventKey, UMA_HISTOGRAM_BOOLEAN(kPageLoadCountLoadingStartedMetric, true);
StabilityMetricEventType::LOADING_STARTED,
StabilityMetricEventType::COUNT);
helper_.LogLoadStarted(); helper_.LogLoadStarted();
} }
...@@ -70,17 +72,17 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartNavigation( ...@@ -70,17 +72,17 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartNavigation(
if (!recording_enabled_) if (!recording_enabled_)
return; return;
StabilityMetricEventType type = PageLoadCountNavigationType type =
StabilityMetricEventType::PAGE_LOAD_NAVIGATION; PageLoadCountNavigationType::PAGE_LOAD_NAVIGATION;
if (navigation_context->GetUrl().SchemeIs(kChromeUIScheme)) { if (navigation_context->GetUrl().SchemeIs(kChromeUIScheme)) {
type = StabilityMetricEventType::CHROME_URL_NAVIGATION; type = PageLoadCountNavigationType::CHROME_URL_NAVIGATION;
} else if (navigation_context->IsSameDocument()) { } else if (navigation_context->IsSameDocument()) {
type = StabilityMetricEventType::SAME_DOCUMENT_WEB_NAVIGATION; type = PageLoadCountNavigationType::SAME_DOCUMENT_WEB_NAVIGATION;
} else { } else {
// TODO(crbug.com/786547): Move helper_.LogLoadStarted() here. // TODO(crbug.com/786547): Move helper_.LogLoadStarted() here.
} }
UMA_HISTOGRAM_ENUMERATION(kPageLoadCountMigrationEventKey, type, UMA_HISTOGRAM_ENUMERATION(kPageLoadCountMetric, type,
StabilityMetricEventType::COUNT); PageLoadCountNavigationType::COUNT);
} }
void IOSChromeStabilityMetricsProvider::RenderProcessGone( void IOSChromeStabilityMetricsProvider::RenderProcessGone(
......
...@@ -45,8 +45,8 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, ...@@ -45,8 +45,8 @@ TEST_F(IOSChromeStabilityMetricsProviderTest,
EXPECT_EQ(0, system_profile.stability().page_load_count()); EXPECT_EQ(0, system_profile.stability().page_load_count());
EXPECT_TRUE(histogram_tester_ EXPECT_TRUE(histogram_tester_
.GetTotalCountsForPrefix(IOSChromeStabilityMetricsProvider:: .GetTotalCountsForPrefix(
kPageLoadCountMigrationEventKey) IOSChromeStabilityMetricsProvider::kPageLoadCountMetric)
.empty()); .empty());
// A load should increment metrics if recording is enabled. // A load should increment metrics if recording is enabled.
...@@ -57,12 +57,8 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, ...@@ -57,12 +57,8 @@ TEST_F(IOSChromeStabilityMetricsProviderTest,
provider.ProvideStabilityMetrics(&system_profile); provider.ProvideStabilityMetrics(&system_profile);
EXPECT_EQ(1, system_profile.stability().page_load_count()); EXPECT_EQ(1, system_profile.stability().page_load_count());
histogram_tester_.ExpectUniqueSample( histogram_tester_.ExpectTotalCount(
IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey, IOSChromeStabilityMetricsProvider::kPageLoadCountLoadingStartedMetric, 1);
static_cast<base::HistogramBase::Sample>(
IOSChromeStabilityMetricsProvider::StabilityMetricEventType::
LOADING_STARTED),
1);
} }
TEST_F(IOSChromeStabilityMetricsProviderTest, TEST_F(IOSChromeStabilityMetricsProviderTest,
...@@ -75,9 +71,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, ...@@ -75,9 +71,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest,
provider.WebStateDidStartNavigation(kNullWebState, &context); provider.WebStateDidStartNavigation(kNullWebState, &context);
histogram_tester_.ExpectUniqueSample( histogram_tester_.ExpectUniqueSample(
IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey, IOSChromeStabilityMetricsProvider::kPageLoadCountMetric,
static_cast<base::HistogramBase::Sample>( static_cast<base::HistogramBase::Sample>(
IOSChromeStabilityMetricsProvider::StabilityMetricEventType:: IOSChromeStabilityMetricsProvider::PageLoadCountNavigationType::
SAME_DOCUMENT_WEB_NAVIGATION), SAME_DOCUMENT_WEB_NAVIGATION),
1); 1);
...@@ -97,9 +93,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, ...@@ -97,9 +93,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest,
provider.WebStateDidStartNavigation(kNullWebState, &context); provider.WebStateDidStartNavigation(kNullWebState, &context);
histogram_tester_.ExpectUniqueSample( histogram_tester_.ExpectUniqueSample(
IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey, IOSChromeStabilityMetricsProvider::kPageLoadCountMetric,
static_cast<base::HistogramBase::Sample>( static_cast<base::HistogramBase::Sample>(
IOSChromeStabilityMetricsProvider::StabilityMetricEventType:: IOSChromeStabilityMetricsProvider::PageLoadCountNavigationType::
CHROME_URL_NAVIGATION), CHROME_URL_NAVIGATION),
1); 1);
...@@ -119,9 +115,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, ...@@ -119,9 +115,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest,
provider.WebStateDidStartNavigation(kNullWebState, &context); provider.WebStateDidStartNavigation(kNullWebState, &context);
histogram_tester_.ExpectUniqueSample( histogram_tester_.ExpectUniqueSample(
IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey, IOSChromeStabilityMetricsProvider::kPageLoadCountMetric,
static_cast<base::HistogramBase::Sample>( static_cast<base::HistogramBase::Sample>(
IOSChromeStabilityMetricsProvider::StabilityMetricEventType:: IOSChromeStabilityMetricsProvider::PageLoadCountNavigationType::
CHROME_URL_NAVIGATION), CHROME_URL_NAVIGATION),
1); 1);
...@@ -137,9 +133,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, WebNavigationShouldLogPageLoad) { ...@@ -137,9 +133,9 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, WebNavigationShouldLogPageLoad) {
provider.WebStateDidStartNavigation(kNullWebState, &context); provider.WebStateDidStartNavigation(kNullWebState, &context);
histogram_tester_.ExpectUniqueSample( histogram_tester_.ExpectUniqueSample(
IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey, IOSChromeStabilityMetricsProvider::kPageLoadCountMetric,
static_cast<base::HistogramBase::Sample>( static_cast<base::HistogramBase::Sample>(
IOSChromeStabilityMetricsProvider::StabilityMetricEventType:: IOSChromeStabilityMetricsProvider::PageLoadCountNavigationType::
PAGE_LOAD_NAVIGATION), PAGE_LOAD_NAVIGATION),
1); 1);
......
...@@ -22489,6 +22489,13 @@ Called by update_gpu_driver_bug_workaround_entries.py.--> ...@@ -22489,6 +22489,13 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
<int value="1" label="Remote suggestions"/> <int value="1" label="Remote suggestions"/>
</enum> </enum>
<enum name="IOSPageLoadCountNavigationType">
<int value="0" label="Chrome URL Navigation"/>
<int value="1" label="Same Document Web Navigation"/>
<int value="2" label="Page Load Navigation"/>
<int value="3" label="Obsolete - Loading Started"/>
</enum>
<enum name="IOSRepeatedExternalAppPromptResponse"> <enum name="IOSRepeatedExternalAppPromptResponse">
<int value="0" label="Blocked"> <int value="0" label="Blocked">
User selected to block the application for the current session. User selected to block the application for the current session.
...@@ -30446,6 +30446,32 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. ...@@ -30446,6 +30446,32 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary> </summary>
</histogram> </histogram>
<histogram name="IOS.PageLoadCount.Counts"
enum="IOSPageLoadCountNavigationType">
<owner>danyao@chromium.org</owner>
<summary>The number of navigation started events by navigation type.</summary>
</histogram>
<histogram name="IOS.PageLoadCount.LoadingStarted">
<owner>danyao@chromium.org</owner>
<summary>
The &quot;true&quot; value of this boolean histogram counts the number of
page loading started events. The &quot;false&quot; value will never be seen.
</summary>
</histogram>
<histogram name="IOS.PageLoadCountMigration.Counts"
enum="IOSPageLoadCountNavigationType">
<obsolete>
Deprecated 2017-11.
</obsolete>
<owner>danyao@chromium.org</owner>
<summary>
Counts different types of navigation and loading events that are relevant to
counting page loads.
</summary>
</histogram>
<histogram name="IOS.PageLoadTiming.OmnibarToPageLoaded" units="ms"> <histogram name="IOS.PageLoadTiming.OmnibarToPageLoaded" units="ms">
<owner>danyao@chromium.org</owner> <owner>danyao@chromium.org</owner>
<summary> <summary>
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