 Chromium Code Reviews
 Chromium Code Reviews Issue 2481013007:
  Improve tracking of user initiated page loads.  (Closed)
    
  
    Issue 2481013007:
  Improve tracking of user initiated page loads.  (Closed) 
  | 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) { |