Skip to content

Commit

Permalink
Add a user initiated navigation metric to UserPerceivedPageVisit event.
Browse files Browse the repository at this point in the history
UKM privacy doc: https://docs.google.com/document/d/1ViOw8xg1xlVdFNr81A4y2WC3Eu3YFGhzML4MAEZ2aag/edit#

Bug: 1191031
Change-Id: I43c4a1152d8a5028c41eb5d0144bfaf36ac070ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3292475
Reviewed-by: Nicolás Peña <npm@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Darren Willis <djw@chromium.org>
Cr-Commit-Position: refs/heads/main@{#951826}
  • Loading branch information
Darren W authored and Chromium LUCI CQ committed Dec 15, 2021
1 parent 7098eb8 commit 413b776
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,11 @@ void UkmPageLoadMetricsObserver::RecordPageEndMetrics(
RecordMemoriesMetrics(builder, page_end_reason);

builder.Record(ukm::UkmRecorder::Get());

// Also log UserInitiated in UserPerceivedPageVisit.
ukm::builders::UserPerceivedPageVisit(GetDelegate().GetPageUkmSourceId())
.SetUserInitiated(is_user_initiated_navigation)
.Record(ukm::UkmRecorder::Get());
}

absl::optional<int64_t>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,15 @@ TEST_F(UkmPageLoadMetricsObserverTest, Basic) {
EXPECT_TRUE(tester()->test_ukm_recorder().EntryHasMetric(
kv.second.get(), PageLoad::kPageTiming_ForegroundDurationName));
}
std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> pagevisit_entries =
tester()->test_ukm_recorder().GetMergedEntriesByName(
ukm::builders::UserPerceivedPageVisit::kEntryName);
EXPECT_EQ(1ul, pagevisit_entries.size());
for (const auto& kv : pagevisit_entries) {
tester()->test_ukm_recorder().ExpectEntryMetric(
kv.second.get(),
ukm::builders::UserPerceivedPageVisit::kUserInitiatedName, true);
}
}

TEST_F(UkmPageLoadMetricsObserverTest, FailedProvisionalLoad) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,16 @@ void BackForwardCachePageLoadMetricsObserver::RecordMetricsOnPageVisitEnd(
if (has_ever_entered_back_forward_cache_) {
page_load_metrics::RecordPageVisitFinalStatusForTiming(
timing, GetDelegate(), GetLastUkmSourceIdForBackForwardCacheRestore());
bool is_user_initiated_navigation =
// All browser initiated page loads are user-initiated.
GetDelegate().GetUserInitiatedInfo().browser_initiated ||
// Renderer-initiated navigations are user-initiated if there is an
// associated input event.
GetDelegate().GetUserInitiatedInfo().user_input_event;
ukm::builders::UserPerceivedPageVisit(
GetLastUkmSourceIdForBackForwardCacheRestore())
.SetUserInitiated(is_user_initiated_navigation)
.Record(ukm::UkmRecorder::Get());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/test/simple_test_tick_clock.h"
#include "components/page_load_metrics/browser/fake_page_load_metrics_observer_delegate.h"
#include "components/page_load_metrics/browser/observers/page_load_metrics_observer_content_test_harness.h"
#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
#include "components/page_load_metrics/browser/page_load_tracker.h"
#include "content/public/test/mock_navigation_handle.h"
#include "services/metrics/public/cpp/metrics_utils.h"
Expand Down Expand Up @@ -534,3 +535,62 @@ TEST_F(BackForwardCachePageLoadMetricsObserverTest, TestLogsNonCWVPageVisit) {
EXPECT_FALSE(result_metrics[1].begin()->second);
}
}

TEST_F(BackForwardCachePageLoadMetricsObserverTest, TestLogsUserInitiated) {
auto& test_ukm_recorder = tester()->test_ukm_recorder();
auto fake_bfcache_restore =
PageLoadMetricsObserverDelegate::BackForwardCacheRestore(
/*was_in_foreground=*/true, base::TimeTicks());
fake_delegate_->AddBackForwardCacheRestore(fake_bfcache_restore);

fake_delegate_->user_initiated_info_ =
page_load_metrics::UserInitiatedInfo::NotUserInitiated();
observer_with_fake_delegate_->OnComplete(timing_);

auto result_metrics = test_ukm_recorder.FilteredHumanReadableMetricForEntry(
UserPerceivedPageVisit::kEntryName,
UserPerceivedPageVisit::kUserInitiatedName);
EXPECT_EQ(1U, result_metrics.size());
EXPECT_EQ(UserPerceivedPageVisit::kUserInitiatedName,
result_metrics[0].begin()->first);
EXPECT_FALSE(result_metrics[0].begin()->second);

// Browser initiated; this is always considered user initiated.
fake_delegate_->user_initiated_info_ =
page_load_metrics::UserInitiatedInfo::BrowserInitiated();
observer_with_fake_delegate_->OnComplete(timing_);

result_metrics = test_ukm_recorder.FilteredHumanReadableMetricForEntry(
UserPerceivedPageVisit::kEntryName,
UserPerceivedPageVisit::kUserInitiatedName);
EXPECT_EQ(2U, result_metrics.size());
EXPECT_EQ(UserPerceivedPageVisit::kUserInitiatedName,
result_metrics[1].begin()->first);
EXPECT_TRUE(result_metrics[1].begin()->second);

// Renderer initiated, with user input, considered user initiated.
fake_delegate_->user_initiated_info_ =
page_load_metrics::UserInitiatedInfo::RenderInitiated(true, true);
observer_with_fake_delegate_->OnComplete(timing_);

result_metrics = test_ukm_recorder.FilteredHumanReadableMetricForEntry(
UserPerceivedPageVisit::kEntryName,
UserPerceivedPageVisit::kUserInitiatedName);
EXPECT_EQ(3U, result_metrics.size());
EXPECT_EQ(UserPerceivedPageVisit::kUserInitiatedName,
result_metrics[2].begin()->first);
EXPECT_TRUE(result_metrics[2].begin()->second);

// Renderer initiated, without user input, not considered user initiated.
fake_delegate_->user_initiated_info_ =
page_load_metrics::UserInitiatedInfo::RenderInitiated(false, false);
observer_with_fake_delegate_->OnComplete(timing_);

result_metrics = test_ukm_recorder.FilteredHumanReadableMetricForEntry(
UserPerceivedPageVisit::kEntryName,
UserPerceivedPageVisit::kUserInitiatedName);
EXPECT_EQ(4U, result_metrics.size());
EXPECT_EQ(UserPerceivedPageVisit::kUserInitiatedName,
result_metrics[3].begin()->first);
EXPECT_FALSE(result_metrics[3].begin()->second);
}
7 changes: 7 additions & 0 deletions tools/metrics/ukm/ukm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18650,6 +18650,13 @@ be describing additional metrics about the same event.
happens.
</summary>
</metric>
<metric name="UserInitiated" enum="Boolean">
<summary>
True if this page visit was initiated by user input. For renderer
navigations, only true if the code can trace back to the original input
event - there are cases where this known not to be possible.
</summary>
</metric>
</event>

<event name="UserSettingsEvent">
Expand Down

0 comments on commit 413b776

Please sign in to comment.