Remove WebContentsObserver::DidCommitNavigation

components/page_load_metrics/browser/metrics_web_contents_observer.cc

Index: components/page_load_metrics/browser/metrics_web_contents_observer.cc
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.cc b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
index 080f00b076152f9052e85a3fe361e88888520c83..3a1fbd9302c38026492c1c708a2043fa0efe8827 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -71,8 +71,10 @@ bool MetricsWebContentsObserver::OnMessageReceived(
   return handled;
 }
 
-void MetricsWebContentsObserver::DidCommitNavigation(
+void MetricsWebContentsObserver::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->HasCommitted())

[Charlie Harrison, 2015/09/22 19:39:45]

Will this ever be called while the previous navigation is ongoing? If not, could
you remove this early return condition? We want to log the previous page load
timing even if the subsequent navigation results in an error.

[clamy, 2015/09/22 23:04:45]

Yes it can be called when a previous navigation is ongoing. If the navigation
results in an error that commits an error page HasCommitted will be true. If the
navigation results in an error that does not commit an error page (ie it gets
aborted/canceled) then HasCommitted will be false. The early return here is
equivalent to only implementing DidCommitNavigation before its removal: if
handle->HasCommitted is false, then DidCommitNavigation would not have been
called (you would only get DidFinishNavigation).

+    return;
   if (navigation_handle->IsInMainFrame() && !navigation_handle->IsSamePage())
     RecordTimingHistograms();
   if (IsRelevantNavigation(navigation_handle))
@@ -148,7 +150,7 @@ bool MetricsWebContentsObserver::IsRelevantNavigation(
   const GURL& browser_url = web_contents()->GetLastCommittedURL();
   return navigation_handle->IsInMainFrame() &&
          !navigation_handle->IsSamePage() &&
-         navigation_handle->HasCommittedDocument() &&
+         !navigation_handle->IsErrorPage() &&
          navigation_handle->GetURL().SchemeIsHTTPOrHTTPS() &&
          browser_url.SchemeIsHTTPOrHTTPS();
 }