Commit f84332a8 authored by Nicolás Peña Moreno's avatar Nicolás Peña Moreno Committed by Chromium LUCI CQ

Add UKM PageEndReason3

This CL slightly updates PageEndReason3 (new version from 2):
* HIDDEN is removed as a page end reason due to the fact that we no
longer flush the metric when the page is hidden.
* APP_ENTER_BACKGROUND is added as a reason as the metric can be
reported when this happens, and it is not a page closure per se.

UKM collection review:
https://docs.google.com/document/d/1nGbQlG5bdrKn_vFzZxIIiIzAGbwooC1n2S4DHYmVivI/

Bug: 1049260
Change-Id: I3ff183d73f36287f3a99590abd552ea15d37e252
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630746Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844317}
parent 87d4efed
...@@ -273,7 +273,8 @@ UkmPageLoadMetricsObserver::FlushMetricsOnAppEnterBackground( ...@@ -273,7 +273,8 @@ UkmPageLoadMetricsObserver::FlushMetricsOnAppEnterBackground(
RecordSmoothnessMetrics(); RecordSmoothnessMetrics();
// Assume that page ends on this method, as the app could be evicted right // Assume that page ends on this method, as the app could be evicted right
// after. // after.
RecordPageEndMetrics(&timing, current_time); RecordPageEndMetrics(&timing, current_time,
/* app_entered_background */ true);
return STOP_OBSERVING; return STOP_OBSERVING;
} }
...@@ -312,7 +313,8 @@ void UkmPageLoadMetricsObserver::OnFailedProvisionalLoad( ...@@ -312,7 +313,8 @@ void UkmPageLoadMetricsObserver::OnFailedProvisionalLoad(
if (is_portal_) if (is_portal_)
return; return;
RecordPageEndMetrics(nullptr, base::TimeTicks()); RecordPageEndMetrics(nullptr, base::TimeTicks(),
/* app_entered_background */ false);
if (was_hidden_) if (was_hidden_)
return; return;
...@@ -350,7 +352,8 @@ void UkmPageLoadMetricsObserver::OnComplete( ...@@ -350,7 +352,8 @@ void UkmPageLoadMetricsObserver::OnComplete(
ReportLayoutStability(); ReportLayoutStability();
RecordSmoothnessMetrics(); RecordSmoothnessMetrics();
ReportPerfectHeuristicsMetrics(); ReportPerfectHeuristicsMetrics();
RecordPageEndMetrics(&timing, current_time); RecordPageEndMetrics(&timing, current_time,
/* app_entered_background */ false);
RecordMobileFriendlinessMetrics(); RecordMobileFriendlinessMetrics();
} }
...@@ -1088,7 +1091,8 @@ void UkmPageLoadMetricsObserver::RecordMobileFriendlinessMetrics() { ...@@ -1088,7 +1091,8 @@ void UkmPageLoadMetricsObserver::RecordMobileFriendlinessMetrics() {
void UkmPageLoadMetricsObserver::RecordPageEndMetrics( void UkmPageLoadMetricsObserver::RecordPageEndMetrics(
const page_load_metrics::mojom::PageLoadTiming* timing, const page_load_metrics::mojom::PageLoadTiming* timing,
base::TimeTicks page_end_time) { base::TimeTicks page_end_time,
bool app_entered_background) {
ukm::builders::PageLoad builder(GetDelegate().GetPageUkmSourceId()); ukm::builders::PageLoad builder(GetDelegate().GetPageUkmSourceId());
// page_transition_ fits in a uint32_t, so we can safely cast to int64_t. // page_transition_ fits in a uint32_t, so we can safely cast to int64_t.
builder.SetNavigation_PageTransition(static_cast<int64_t>(page_transition_)); builder.SetNavigation_PageTransition(static_cast<int64_t>(page_transition_));
...@@ -1097,10 +1101,11 @@ void UkmPageLoadMetricsObserver::RecordPageEndMetrics( ...@@ -1097,10 +1101,11 @@ void UkmPageLoadMetricsObserver::RecordPageEndMetrics(
// to int64_t. // to int64_t.
int64_t page_end_reason = GetDelegate().GetPageEndReason(); int64_t page_end_reason = GetDelegate().GetPageEndReason();
if (page_end_reason == page_load_metrics::PageEndReason::END_NONE && if (page_end_reason == page_load_metrics::PageEndReason::END_NONE &&
was_hidden_) { app_entered_background) {
page_end_reason = page_load_metrics::PageEndReason::END_HIDDEN; page_end_reason =
page_load_metrics::PageEndReason::END_APP_ENTER_BACKGROUND;
} }
builder.SetNavigation_PageEndReason2(page_end_reason); builder.SetNavigation_PageEndReason3(page_end_reason);
bool is_user_initiated_navigation = bool is_user_initiated_navigation =
// All browser initiated page loads are user-initiated. // All browser initiated page loads are user-initiated.
GetDelegate().GetUserInitiatedInfo().browser_initiated || GetDelegate().GetUserInitiatedInfo().browser_initiated ||
......
...@@ -181,7 +181,8 @@ class UkmPageLoadMetricsObserver ...@@ -181,7 +181,8 @@ class UkmPageLoadMetricsObserver
// loads. // loads.
void RecordPageEndMetrics( void RecordPageEndMetrics(
const page_load_metrics::mojom::PageLoadTiming* timing, const page_load_metrics::mojom::PageLoadTiming* timing,
base::TimeTicks page_end_time); base::TimeTicks page_end_time,
bool app_entered_background);
// Records a score from the SiteEngagementService. Called when the page // Records a score from the SiteEngagementService. Called when the page
// becomes hidden, or at the end of the session if the page is never hidden. // becomes hidden, or at the end of the session if the page is never hidden.
......
...@@ -269,7 +269,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, Basic) { ...@@ -269,7 +269,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, Basic) {
kv.second.get(), PageLoad::kNavigation_PageTransitionName, kv.second.get(), PageLoad::kNavigation_PageTransitionName,
ui::PAGE_TRANSITION_LINK); ui::PAGE_TRANSITION_LINK);
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
kv.second.get(), PageLoad::kNavigation_PageEndReason2Name, kv.second.get(), PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
kv.second.get(), PageLoad::kParseTiming_NavigationToParseStartName, kv.second.get(), PageLoad::kParseTiming_NavigationToParseStartName,
...@@ -328,7 +328,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, FailedProvisionalLoad) { ...@@ -328,7 +328,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, FailedProvisionalLoad) {
kv.second.get(), PageLoad::kNavigation_PageTransitionName, kv.second.get(), PageLoad::kNavigation_PageTransitionName,
ui::PAGE_TRANSITION_LINK); ui::PAGE_TRANSITION_LINK);
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
kv.second.get(), PageLoad::kNavigation_PageEndReason2Name, kv.second.get(), PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_PROVISIONAL_LOAD_FAILED); page_load_metrics::END_PROVISIONAL_LOAD_FAILED);
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
kv.second.get(), kv.second.get(),
...@@ -477,7 +477,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, ...@@ -477,7 +477,7 @@ TEST_F(UkmPageLoadMetricsObserverTest,
EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric( EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name)); entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name));
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry, PageLoad::kNavigation_PageEndReason2Name, entry, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> internal_merged_entries = std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> internal_merged_entries =
...@@ -510,7 +510,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, AbortNeverForegrounded) { ...@@ -510,7 +510,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, AbortNeverForegrounded) {
EXPECT_EQ(1ul, merged_entries.size()); EXPECT_EQ(1ul, merged_entries.size());
const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get(); const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get();
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry, PageLoad::kNavigation_PageEndReason2Name, entry, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry, PageLoad::kExperimental_PageLoadTypeName, entry, PageLoad::kExperimental_PageLoadTypeName,
...@@ -552,7 +552,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, FCPPlusPlus_DiscardBackgroundResult) { ...@@ -552,7 +552,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, FCPPlusPlus_DiscardBackgroundResult) {
EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric( EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name)); entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name));
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry, PageLoad::kNavigation_PageEndReason2Name, entry, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> internal_merged_entries = std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> internal_merged_entries =
...@@ -1140,7 +1140,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, MultiplePageLoads) { ...@@ -1140,7 +1140,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, MultiplePageLoads) {
tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(entry1, tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(entry1,
GURL(kTestUrl1)); GURL(kTestUrl1));
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry1, PageLoad::kNavigation_PageEndReason2Name, entry1, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_NEW_NAVIGATION); page_load_metrics::END_NEW_NAVIGATION);
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry1, PageLoad::kPaintTiming_NavigationToFirstContentfulPaintName, 200); entry1, PageLoad::kPaintTiming_NavigationToFirstContentfulPaintName, 200);
...@@ -1154,7 +1154,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, MultiplePageLoads) { ...@@ -1154,7 +1154,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, MultiplePageLoads) {
tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(entry2, tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(entry2,
GURL(kTestUrl2)); GURL(kTestUrl2));
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
entry2, PageLoad::kNavigation_PageEndReason2Name, entry2, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric( EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
entry2, PageLoad::kParseTiming_NavigationToParseStartName)); entry2, PageLoad::kParseTiming_NavigationToParseStartName));
...@@ -1484,7 +1484,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, LayoutInstability) { ...@@ -1484,7 +1484,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, LayoutInstability) {
kLayoutInstability_CumulativeShiftScore_MainFrame_BeforeInputOrScrollName, kLayoutInstability_CumulativeShiftScore_MainFrame_BeforeInputOrScrollName,
100); 100);
ukm_recorder.ExpectEntryMetric(kv.second.get(), ukm_recorder.ExpectEntryMetric(kv.second.get(),
PageLoad::kNavigation_PageEndReason2Name, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
} }
...@@ -1867,6 +1867,23 @@ TEST_F(UkmPageLoadMetricsObserverTest, LCPHiddenWhileFlushing) { ...@@ -1867,6 +1867,23 @@ TEST_F(UkmPageLoadMetricsObserverTest, LCPHiddenWhileFlushing) {
true /* test_main_frame */); true /* test_main_frame */);
} }
TEST_F(UkmPageLoadMetricsObserverTest, AppEnterBackground) {
NavigateAndCommit(GURL(kTestUrl1));
page_load_metrics::mojom::PageLoadTiming timing;
page_load_metrics::InitPageLoadTimingForTest(&timing);
tester()->SimulateAppEnterBackground();
const auto& ukm_recorder = tester()->test_ukm_recorder();
std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> merged_entries =
ukm_recorder.GetMergedEntriesByName(PageLoad::kEntryName);
EXPECT_EQ(1ul, merged_entries.size());
const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get();
tester()->test_ukm_recorder().ExpectEntryMetric(
entry, PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_APP_ENTER_BACKGROUND);
}
class TestOfflinePreviewsUkmPageLoadMetricsObserver class TestOfflinePreviewsUkmPageLoadMetricsObserver
: public UkmPageLoadMetricsObserver { : public UkmPageLoadMetricsObserver {
public: public:
...@@ -1912,7 +1929,7 @@ TEST_F(OfflinePreviewsUKMPageLoadMetricsObserverTest, OfflinePreviewReported) { ...@@ -1912,7 +1929,7 @@ TEST_F(OfflinePreviewsUKMPageLoadMetricsObserverTest, OfflinePreviewReported) {
tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(kv.second.get(), tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(kv.second.get(),
GURL(kTestUrl1)); GURL(kTestUrl1));
tester()->test_ukm_recorder().ExpectEntryMetric( tester()->test_ukm_recorder().ExpectEntryMetric(
kv.second.get(), PageLoad::kNavigation_PageEndReason2Name, kv.second.get(), PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE); page_load_metrics::END_CLOSE);
} }
} }
......
...@@ -17,7 +17,7 @@ namespace page_load_metrics { ...@@ -17,7 +17,7 @@ namespace page_load_metrics {
enum PageEndReason { enum PageEndReason {
// Page lifetime has not yet ended (page is still active). Pages that // Page lifetime has not yet ended (page is still active). Pages that
// become hidden are logged in END_HIDDEN, so we expect low numbers in this // become hidden are logged in END_HIDDEN, so we expect low numbers in this
// bucket from the end of May 2020, i.e. in Navigation.PageEndReason2. // bucket from the end of May 2020.
END_NONE = 0, END_NONE = 0,
// The page was reloaded, possibly by the user. // The page was reloaded, possibly by the user.
...@@ -53,9 +53,14 @@ enum PageEndReason { ...@@ -53,9 +53,14 @@ enum PageEndReason {
END_OTHER = 9, END_OTHER = 9,
// The page became hidden but is still active. Added end of May 2020 for // The page became hidden but is still active. Added end of May 2020 for
// Navigation.PageEndReason2. // Navigation.PageEndReason2. Deprecated Jan 2021 with
// Navigation.PageEndReason3.
END_HIDDEN = 10, END_HIDDEN = 10,
// The page load has not yet ended but we need to flush the metrics because
// the app has entered the background.
END_APP_ENTER_BACKGROUND = 11,
PAGE_END_REASON_COUNT PAGE_END_REASON_COUNT
}; };
......
...@@ -56624,6 +56624,9 @@ Called by update_net_trust_anchors.py.--> ...@@ -56624,6 +56624,9 @@ Called by update_net_trust_anchors.py.-->
<int value="10" label="END_HIDDEN"> <int value="10" label="END_HIDDEN">
Page became hidden, but is still active. Page became hidden, but is still active.
</int> </int>
<int value="11" label="END_APP_BACKGROUND">
The metrics were flushed because the app entered the background.
</int>
</enum> </enum>
<enum name="PageLifecycleStateTransition"> <enum name="PageLifecycleStateTransition">
...@@ -10152,6 +10152,9 @@ be describing additional metrics about the same event. ...@@ -10152,6 +10152,9 @@ be describing additional metrics about the same event.
</summary> </summary>
</metric> </metric>
<metric name="Navigation.PageEndReason2" enum="PageEndReason"> <metric name="Navigation.PageEndReason2" enum="PageEndReason">
<obsolete>
Deprecated Jan 2021 in favor of Navigation.PageEndReason3.
</obsolete>
<summary> <summary>
The |page_load_metrics::PageEndReason| for the main frame navigation of The |page_load_metrics::PageEndReason| for the main frame navigation of
this page load. Replaced Navigation.PageEndReason at the end of May 2020 this page load. Replaced Navigation.PageEndReason at the end of May 2020
...@@ -10160,6 +10163,14 @@ be describing additional metrics about the same event. ...@@ -10160,6 +10163,14 @@ be describing additional metrics about the same event.
metric is recorded after the page became hidden. metric is recorded after the page became hidden.
</summary> </summary>
</metric> </metric>
<metric name="Navigation.PageEndReason3" enum="PageEndReason">
<summary>
The |page_load_metrics::PageEndReason| for the main frame navigation of
this page load. Replaced Navigation.PageEndReason2 on January 2021 to get
rid of END_HIDDEN and use END_APP_BACKGROUND on cases where the metrics
are flushed and the app enters the background.
</summary>
</metric>
<metric name="Navigation.PageTransition"> <metric name="Navigation.PageTransition">
<summary> <summary>
The |ui::PageTransition| for the main frame navigation of this page load. The |ui::PageTransition| for the main frame navigation of this page load.
......
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