[page_load_metrics] Add a NavigationThrottle for richer abort metrics

components/page_load_metrics/browser/metrics_web_contents_observer.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
 
 #include <algorithm>
 #include <ostream>
 #include <string>
 #include <utility>
@@ skipping 243 matching lines @@
     PageLoadMetricsEmbedderInterface* embedder_interface,
     const GURL& currently_committed_url,
     content::NavigationHandle* navigation_handle,
     int aborted_chain_size,
     int aborted_chain_size_same_url)
     : renderer_tracked_(false),
       navigation_start_(navigation_handle->NavigationStart()),
       url_(navigation_handle->GetURL()),
       abort_type_(ABORT_NONE),
       started_in_foreground_(in_foreground),
+      page_transition_(ui::PAGE_TRANSITION_LAST_CORE),
       aborted_chain_size_(aborted_chain_size),
       aborted_chain_size_same_url_(aborted_chain_size_same_url),
       embedder_interface_(embedder_interface) {
   DCHECK(!navigation_handle->HasCommitted());
   embedder_interface_->RegisterObservers(this);
   for (const auto& observer : observers_) {
     observer->OnStart(navigation_handle, currently_committed_url,
                       started_in_foreground_);
   }
 }
 
 PageLoadTracker::~PageLoadTracker() {
   const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
 
   if (info.time_to_commit && renderer_tracked() && timing_.IsEmpty()) {
     RecordInternalError(ERR_NO_IPCS_RECEIVED);
   }
-  // Recall that trackers that are given ABORT_UNKNOWN_NAVIGATION have their
-  // chain length added to the next navigation. Take care not to double count
-  // them. Also do not double count committed loads, which call this already.
-  if (commit_time_.is_null() && abort_type_ != ABORT_UNKNOWN_NAVIGATION)
+
+  // Don't include any aborts that resulted in a new navigation, as the chain
+  // length will be included in the aborter PageLoadTracker.
+  if (commit_time_.is_null() && abort_type_ != ABORT_UNKNOWN_NAVIGATION &&
+      abort_type_ != ABORT_RELOAD && abort_type_ != ABORT_FORWARD_BACK &&
+      abort_type_ != ABORT_NEW_NAVIGATION) {
     LogAbortChainHistograms(nullptr);
+  }
 
   for (const auto& observer : observers_) {
     observer->OnComplete(timing_, info);
   }
 }
 
 void PageLoadTracker::LogAbortChainHistograms(
     content::NavigationHandle* final_navigation) {
   if (aborted_chain_size_ == 0)
     return;
@@ skipping 80 matching lines @@
   LogAbortChainHistograms(navigation_handle);
 }
 
 void PageLoadTracker::FailedProvisionalLoad(
     content::NavigationHandle* navigation_handle) {
   for (const auto& observer : observers_) {
     observer->OnFailedProvisionalLoad(navigation_handle);
   }
 }
 
+// TODO(csharrison): We can get an approximation of whether the abort is user

[Bryan McQuade, 2016/07/12 16:45:29]

is the intent to record whether it's user initiated once we've updated browser
navigations to report true for user initiated? or has that happened already?

[Charlie Harrison, 2016/07/12 19:20:22]

It isn't necessary to do the browser navigation change (though it will be
convenient). I was just delaying this change to a follow up CL.

+// initiated or not from the NavigationHandle at this point.
+void PageLoadTracker::WillStartNavigationRequest(
+    content::NavigationHandle* navigation_handle) {
+  page_transition_ = navigation_handle->GetPageTransition();
+}
+
 void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
   for (const auto& observer : observers_) {
     observer->OnRedirect(navigation_handle);
   }
 }
 
 void PageLoadTracker::OnInputEvent(const blink::WebInputEvent& event) {
   for (const auto& observer : observers_) {
     observer->OnUserInput(event);
   }
@@ skipping 128 matching lines @@
     return;
 
   UpdateAbortInternal(abort_type, timestamp, is_certainly_browser_timestamp);
 }
 
 void PageLoadTracker::UpdateAbort(UserAbortType abort_type,
                                   base::TimeTicks timestamp,
                                   bool is_certainly_browser_timestamp) {
   DCHECK_NE(abort_type, ABORT_NONE);
   DCHECK_NE(abort_type, ABORT_OTHER);
-  DCHECK_EQ(abort_type_, ABORT_OTHER);
+  DCHECK(abort_type_ == ABORT_OTHER || abort_type_ == ABORT_UNKNOWN_NAVIGATION)
+      << "UpdateAbort called with abort_type_ = " << abort_type_;
 
   // For some aborts (e.g. navigations), the initiated timestamp can be earlier
   // than the timestamp that aborted the load. Taking the minimum gives the
   // closest user initiated time known.
   UpdateAbortInternal(abort_type, std::min(abort_time_, timestamp),
                       is_certainly_browser_timestamp);
 }
 
-bool PageLoadTracker::IsLikelyProvisionalAbort(
-    base::TimeTicks abort_cause_time) {
+bool PageLoadTracker::WasAbortedRecently(base::TimeTicks abort_cause_time) {
   // Note that |abort_cause_time - abort_time| can be negative.
-  return abort_type_ == ABORT_OTHER &&
-         (abort_cause_time - abort_time_).InMilliseconds() < 100;
+  return (abort_cause_time - abort_time_).InMilliseconds() < 100;
 }
 
 bool PageLoadTracker::MatchesOriginalNavigation(
     content::NavigationHandle* navigation_handle) {
   // Neither navigation should have committed.
   DCHECK(!navigation_handle->HasCommitted());
   DCHECK(commit_time_.is_null());
   return navigation_handle->GetURL() == url_;
 }
 
@@ skipping 77 matching lines @@
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(MetricsWebContentsObserver, message,
                                    render_frame_host)
     IPC_MESSAGE_HANDLER(PageLoadMetricsMsg_TimingUpdated, OnTimingUpdated)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
+// TODO(csharrison): The only reason DidStartNavigation is needed is for unit
+// tests. Otherwise all this logic can go in WillStartNavigationRequest.
 void MetricsWebContentsObserver::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame())
     return;
   if (embedder_interface_->IsPrerendering(web_contents()))
     return;
   if (embedder_interface_->IsNewTabPageUrl(navigation_handle->GetURL()))
     return;
   if (navigation_handle->GetURL().spec().compare(url::kAboutBlankURL) == 0)
     return;
 
-  std::unique_ptr<PageLoadTracker> last_aborted =
-      NotifyAbortedProvisionalLoadsNewNavigation(navigation_handle);
-
   int chain_size_same_url = 0;
   int chain_size = 0;
-  if (last_aborted) {
+  if (!aborted_provisional_loads_.empty()) {
+    PageLoadTracker* last_aborted = aborted_provisional_loads_.back().get();
     if (last_aborted->MatchesOriginalNavigation(navigation_handle)) {
       chain_size_same_url = last_aborted->aborted_chain_size_same_url() + 1;
     } else if (last_aborted->aborted_chain_size_same_url() > 0) {
       LogAbortChainSameURLHistogram(
           last_aborted->aborted_chain_size_same_url());
     }
     chain_size = last_aborted->aborted_chain_size() + 1;
   }
 
   // Pass in the last committed url to the PageLoadTracker. If the MWCO has
@@ skipping 11 matching lines @@
   has_navigated_ = true;
 
   // We can have two provisional loads in some cases. E.g. a same-site
   // navigation can have a concurrent cross-process navigation started
   // from the omnibox.
   DCHECK_GT(2ul, provisional_loads_.size());
   // Passing raw pointers to observers_ and embedder_interface_ is safe because
   // the MetricsWebContentsObserver owns them both list and they are torn down
   // after the PageLoadTracker. The PageLoadTracker does not hold on to
   // committed_load_ or navigation_handle beyond the scope of the constructor.
-  provisional_loads_.insert(std::make_pair(
+  auto it = provisional_loads_.insert(std::make_pair(
       navigation_handle,
       base::WrapUnique(new PageLoadTracker(
           in_foreground_, embedder_interface_.get(), currently_committed_url,
           navigation_handle, chain_size, chain_size_same_url))));
+  // Don't clear the aborted provisional loads here because
+  // WillStartNavigationRequest will soon be called, providing a better value
+  // for the page transition and user gesture.
+  NotifyAbortedProvisionalLoadsNewNavigation(it.first->second.get());
+}
+
+void MetricsWebContentsObserver::WillStartNavigationRequest(
+    content::NavigationHandle* navigation_handle) {
+  auto it = provisional_loads_.find(navigation_handle);
+  if (it == provisional_loads_.end())
+    return;
+  it->second->WillStartNavigationRequest(navigation_handle);
+  NotifyAbortedProvisionalLoadsNewNavigation(it->second.get());
+  aborted_provisional_loads_.clear();
 }
 
 const PageLoadExtraInfo
 MetricsWebContentsObserver::GetPageLoadExtraInfoForCommittedLoad() {
   DCHECK(committed_load_);
   return committed_load_->ComputePageLoadExtraInfo();
 }
 
 void MetricsWebContentsObserver::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
@@ skipping 140 matching lines @@
 }
 
 void MetricsWebContentsObserver::NotifyAbortAllLoads(UserAbortType abort_type) {
   NotifyAbortAllLoadsWithTimestamp(abort_type, base::TimeTicks::Now(), true);
 }
 
 void MetricsWebContentsObserver::NotifyAbortAllLoadsWithTimestamp(
     UserAbortType abort_type,
     base::TimeTicks timestamp,
     bool is_certainly_browser_timestamp) {
-  if (committed_load_)
+  if (committed_load_) {
     committed_load_->NotifyAbort(abort_type, timestamp,
+
                                  is_certainly_browser_timestamp);
+  }
   for (const auto& kv : provisional_loads_) {
     kv.second->NotifyAbort(abort_type, timestamp,
                            is_certainly_browser_timestamp);
   }
   for (const auto& tracker : aborted_provisional_loads_) {
-    if (tracker->IsLikelyProvisionalAbort(timestamp))
+    // Only update an aborted provisional load if it has an OTHER type. Do not
+    // include UNKNOWN_NAVIGATION here because those are handled in
+    // NotifyAbortedProvisionalLoadsNewNavigation.
+    // TODO(csharrison): This is an implementation flaw due to how the unit
+    // testing framework avoids calling WillStartNavigationRequest.
+    if (tracker->WasAbortedRecently(timestamp) &&
+        tracker->abort_type() == ABORT_OTHER) {
       tracker->UpdateAbort(abort_type, timestamp,
                            is_certainly_browser_timestamp);
+    }
   }
   aborted_provisional_loads_.clear();
 }
 
-std::unique_ptr<PageLoadTracker>
-MetricsWebContentsObserver::NotifyAbortedProvisionalLoadsNewNavigation(
-    content::NavigationHandle* new_navigation) {
-  // If there are multiple aborted loads that can be attributed to this one,
-  // just count the latest one for simplicity. Other loads will fall into the
-  // OTHER bucket, though there shouldn't be very many.
+void MetricsWebContentsObserver::NotifyAbortedProvisionalLoadsNewNavigation(
+    PageLoadTracker* new_navigation) {
   if (aborted_provisional_loads_.size() == 0)
-    return nullptr;
+    return;
   if (aborted_provisional_loads_.size() > 1)
     RecordInternalError(ERR_NAVIGATION_SIGNALS_MULIPLE_ABORTED_LOADS);
 
-  std::unique_ptr<PageLoadTracker> last_aborted_load =
-      std::move(aborted_provisional_loads_.back());
-  aborted_provisional_loads_.pop_back();
-
-  base::TimeTicks timestamp = new_navigation->NavigationStart();
-  if (last_aborted_load->IsLikelyProvisionalAbort(timestamp))
-    last_aborted_load->UpdateAbort(ABORT_UNKNOWN_NAVIGATION, timestamp, false);
-
-  aborted_provisional_loads_.clear();
-  return last_aborted_load;
+  for (const auto& it : aborted_provisional_loads_) {
+    base::TimeTicks timestamp = new_navigation->navigation_start();
+    if (it->WasAbortedRecently(timestamp) &&
+        (it->abort_type() == ABORT_OTHER ||
+         it->abort_type() == ABORT_UNKNOWN_NAVIGATION)) {
+      ui::PageTransition transition = new_navigation->page_transition();
+      UserAbortType abort_type = transition == ui::PAGE_TRANSITION_LAST_CORE
+                                     ? ABORT_UNKNOWN_NAVIGATION
+                                     : AbortTypeForPageTransition(transition);
+      it->UpdateAbort(abort_type, timestamp, false);
+    }
+  }
 }
 
 void MetricsWebContentsObserver::OnTimingUpdated(
     content::RenderFrameHost* render_frame_host,
     const PageLoadTiming& timing,
     const PageLoadMetadata& metadata) {
   bool error = false;
   if (!committed_load_ || !committed_load_->renderer_tracked()) {
     RecordInternalError(ERR_IPC_WITH_NO_RELEVANT_LOAD);
     error = true;
@@ skipping 19 matching lines @@
 
   if (!committed_load_->UpdateTiming(timing, metadata)) {
     // If the page load tracker cannot update its timing, something is wrong
     // with the IPC (it's from another load, or it's invalid in some other way).
     // We expect this to be a rare occurrence.
     RecordInternalError(ERR_BAD_TIMING_IPC);
   }
 }
 
 }  // namespace page_load_metrics