Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Unified Diff: chrome/browser/page_load_metrics/page_load_tracker.cc

Issue 2481013007: Improve tracking of user initiated page loads. (Closed)
Patch Set: fix comment Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/page_load_metrics/page_load_tracker.cc
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.cc b/chrome/browser/page_load_metrics/page_load_tracker.cc
index 4e672640c8ea38e1e8694db2ad75263a566890b9..c8555732666d06354acbd6c00a7991def0d6f85e 100644
--- a/chrome/browser/page_load_metrics/page_load_tracker.cc
+++ b/chrome/browser/page_load_metrics/page_load_tracker.cc
@@ -87,10 +87,10 @@ void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url) {
}
// TODO(crbug.com/617904): Browser initiated navigations should have
-// HasUserGesture() set to true. Update this once we get enough data from just
-// renderer initiated aborts.
+// HasUserGesture() set to true. In the meantime, we consider all
+// browser-initiated navigations to be user initiated.
bool IsNavigationUserInitiated(content::NavigationHandle* handle) {
- return handle->HasUserGesture();
+ return handle->HasUserGesture() || !handle->IsRendererInitiated();
Charlie Harrison 2016/11/10 22:26:05 Can you reference the crbug that shows this proper
Bryan McQuade 2016/11/10 22:52:50 Done
}
namespace {
@@ -294,7 +294,7 @@ PageLoadTracker::PageLoadTracker(
page_transition_(navigation_handle->GetPageTransition()),
num_cache_requests_(0),
num_network_requests_(0),
- user_gesture_(IsNavigationUserInitiated(navigation_handle)),
+ user_initiated_(IsNavigationUserInitiated(navigation_handle)),
aborted_chain_size_(aborted_chain_size),
aborted_chain_size_same_url_(aborted_chain_size_same_url),
embedder_interface_(embedder_interface) {
@@ -435,7 +435,7 @@ void PageLoadTracker::Commit(content::NavigationHandle* navigation_handle) {
committed_url_ = navigation_handle->GetURL();
// Some transitions (like CLIENT_REDIRECT) are only known at commit time.
page_transition_ = navigation_handle->GetPageTransition();
- user_gesture_ = navigation_handle->HasUserGesture();
+ user_initiated_ = IsNavigationUserInitiated(navigation_handle);
INVOKE_AND_PRUNE_OBSERVERS(observers_, OnCommit, navigation_handle);
LogAbortChainHistograms(navigation_handle);
@@ -594,7 +594,7 @@ PageLoadExtraInfo PageLoadTracker::ComputePageLoadExtraInfo() {
DCHECK(abort_type_ != ABORT_NONE || !abort_user_initiated_);
return PageLoadExtraInfo(
first_background_time, first_foreground_time, started_in_foreground_,
- user_gesture_, committed_url_, start_url_, abort_type_,
+ user_initiated_, committed_url_, start_url_, abort_type_,
abort_user_initiated_, time_to_abort, num_cache_requests_,
num_network_requests_, metadata_);
}
@@ -664,6 +664,14 @@ void PageLoadTracker::UpdateAbortInternal(UserAbortType abort_type,
}
abort_type_ = abort_type;
abort_time_ = timestamp;
+ // A client redirect can never be user initiated. Due to the way Blink
+ // implements user gesture tracking, where all events that occur within 1
+ // second after a user interaction are considered to be triggered by user
+ // activation (based on HTML spec:
+ // https://html.spec.whatwg.org/multipage/interaction.html#triggered-by-user-activation),
+ // these navs may sometimes be reported as user initiated by Blink. Thus, we
+ // explicitly filter these types of aborts out when deciding if the abort was
+ // user initiated.
abort_user_initiated_ = user_initiated && abort_type != ABORT_CLIENT_REDIRECT;
if (is_certainly_browser_timestamp) {

Powered by Google App Engine
This is Rietveld 408576698