Refactor PageLoadMetricsObserver completion callback policy

chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_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 "chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h"
 #include <string>
 
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
@@ skipping 374 matching lines @@
 
 void FromGWSPageLoadMetricsLogger::SetPreviouslyCommittedUrl(const GURL& url) {
   previously_committed_url_is_search_results_ = IsGoogleSearchResultUrl(url);
   previously_committed_url_is_search_redirector_ =
       IsGoogleSearchRedirectorUrl(url);
 }
 
 void FromGWSPageLoadMetricsLogger::SetProvisionalUrl(const GURL& url) {
   provisional_url_has_search_hostname_ =
       IsGoogleSearchHostname(url.host_piece());
-  provisional_url_is_non_http_or_https_ = !url.SchemeIsHTTPOrHTTPS();
 }
 
 FromGWSPageLoadMetricsObserver::FromGWSPageLoadMetricsObserver() {}
 
 void FromGWSPageLoadMetricsObserver::OnStart(
     content::NavigationHandle* navigation_handle,
     const GURL& currently_committed_url,
     bool started_in_foreground) {
   logger_.SetPreviouslyCommittedUrl(currently_committed_url);
   logger_.SetProvisionalUrl(navigation_handle->GetURL());
@@ skipping 63 matching lines @@
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
   logger_.OnParseStop(timing, extra_info);
 }
 
 void FromGWSPageLoadMetricsObserver::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
   logger_.OnComplete(timing, extra_info);
 }
 
+void FromGWSPageLoadMetricsObserver::OnFailedProvisionalLoad(
+    const page_load_metrics::FailedProvisionalLoadInfo& failed_load_info,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  logger_.OnFailedProvisionalLoad(failed_load_info, extra_info);
+}
+
 void FromGWSPageLoadMetricsObserver::OnUserInput(
     const blink::WebInputEvent& event) {
   logger_.OnUserInput(event);
 }
 
 void FromGWSPageLoadMetricsLogger::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
-  if (!ShouldLogMetrics(extra_info.committed_url))
+  if (!ShouldLogPostCommitMetrics(extra_info.committed_url))
     return;
 
-  // If we have a committed load but |timing.IsEmpty()|, then this load was not
-  // tracked by the renderer. In this case, it is not possible to know whether
-  // the abort signals came before the page painted. Additionally, for
-  // consistency with core PageLoad metrics, we ignore non-render-tracked
-  // loads when tracking aborts after commit.
   UserAbortType abort_type = extra_info.abort_type;
   if (!WasAbortedInForeground(abort_type, extra_info.time_to_abort, extra_info))
     return;
 
-  base::TimeDelta time_to_abort = extra_info.time_to_abort.value();
-  if (extra_info.committed_url.is_empty()) {
-    LogProvisionalAborts(abort_type, time_to_abort);
-    return;
-  }
-
-  // If we didn't receive any timing data but did commit, this is likely not a
-  // renderer-tracked navigation, so ignore it.
+  // If we did not receive any timing IPCs from the render process, we can't
+  // know for certain if the page was truly aborted before paint, or if the
+  // abort happened before we received the IPC from the render process. Thus, we
+  // do not log aborts for these page loads. Tracked page loads that receive no
+  // timing IPCs are tracked via the ERR_NO_IPCS_RECEIVED error code in the
+  // PageLoad.Events.InternalError histogram, so we can keep track of how often
+  // this happens.
   if (timing.IsEmpty())
     return;
 
+  base::TimeDelta time_to_abort = extra_info.time_to_abort.value();
   if (!timing.first_paint || timing.first_paint >= time_to_abort) {
     LogCommittedAbortsBeforePaint(abort_type, time_to_abort);
   } else if (WasAbortedBeforeInteraction(abort_type,
                                          first_user_interaction_after_paint_,
                                          extra_info.time_to_abort)) {
     LogAbortsAfterPaintBeforeInteraction(abort_type, time_to_abort);
   }
 }
 
-bool FromGWSPageLoadMetricsLogger::ShouldLogMetrics(const GURL& committed_url) {
+void FromGWSPageLoadMetricsLogger::OnFailedProvisionalLoad(
+    const page_load_metrics::FailedProvisionalLoadInfo& failed_load_info,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  if (!ShouldLogFailedProvisionalLoadMetrics())
+    return;
+
+  UserAbortType abort_type = extra_info.abort_type;
+  if (!WasAbortedInForeground(abort_type, extra_info.time_to_abort, extra_info))
+    return;
+
+  LogProvisionalAborts(abort_type, extra_info.time_to_abort.value());
+}
+
+bool FromGWSPageLoadMetricsLogger::ShouldLogFailedProvisionalLoadMetrics() {
+  // See comment in ShouldLogPostCommitMetrics above the call to
+  // IsGoogleSearchHostname for more info on this if test.
+  if (provisional_url_has_search_hostname_)
+    return false;
+
+  return previously_committed_url_is_search_results_ ||
+         previously_committed_url_is_search_redirector_;
+}
+
+bool FromGWSPageLoadMetricsLogger::ShouldLogPostCommitMetrics(
+    const GURL& committed_url) {
+  DCHECK(!committed_url.is_empty());
+
   // If this page has a URL on a known google search hostname, then it may be a
   // page associated with search (either a search results page, or a search
   // redirector url), so we should not log stats. We could try to detect only
   // the specific known search URLs here, and log navigations to other pages on
   // the google search hostname (for example, a search for 'about google'
   // includes a result for https://www.google.com/about/), however, we assume
   // these cases are relatively uncommon, and we run the risk of logging metrics
   // for some search redirector URLs. Thus we choose the more conservative
-  // approach of ignoring all urls on known search hostnames. We use the
-  // provisional url if the navigation didn't commit. Also ignore navigations to
-  // other URL schemes, such as app navigations via intent://.
-  if (committed_url.is_empty()) {
-    if (provisional_url_has_search_hostname_ ||
-        provisional_url_is_non_http_or_https_)
-      return false;
-  } else {
-    if (IsGoogleSearchHostname(committed_url.host_piece()) ||
-        !committed_url.SchemeIsHTTPOrHTTPS())
-      return false;
-  }
+  // approach of ignoring all urls on known search hostnames.
+  if (IsGoogleSearchHostname(committed_url.host_piece()))
+    return false;
 
   // We're only interested in tracking navigations (e.g. clicks) initiated via
   // links. Note that the redirector will mask these, so don't enforce this if
   // the navigation came from a redirect url. TODO(csharrison): Use this signal
   // for provisional loads when the content APIs allow for it.
   if (previously_committed_url_is_search_results_ &&
-      (committed_url.is_empty() || navigation_initiated_via_link_)) {
+      navigation_initiated_via_link_) {
     return true;
   }
 
   // If the navigation was via the search redirector, then the information about
   // whether the navigation was from a link would have been associated with the
   // navigation to the redirector, and not included in the redirected
   // navigation. Therefore, do not require link navigation this case.
   return previously_committed_url_is_search_redirector_;
 }
 
 bool FromGWSPageLoadMetricsLogger::ShouldLogForegroundEventAfterCommit(
     const base::Optional<base::TimeDelta>& event,
     const page_load_metrics::PageLoadExtraInfo& info) {
   DCHECK(!info.committed_url.is_empty())
       << "ShouldLogForegroundEventAfterCommit called without committed URL.";
-  return ShouldLogMetrics(info.committed_url) &&
+  return ShouldLogPostCommitMetrics(info.committed_url) &&
          WasStartedInForegroundOptionalEventInForeground(event, info);
 }
 
 void FromGWSPageLoadMetricsLogger::OnDomContentLoadedEventStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
   if (ShouldLogForegroundEventAfterCommit(timing.dom_content_loaded_event_start,
                                           extra_info)) {
     PAGE_LOAD_HISTOGRAM(internal::kHistogramFromGWSDomContentLoaded,
                         timing.dom_content_loaded_event_start.value());
@@ skipping 77 matching lines @@
 }
 
 void FromGWSPageLoadMetricsLogger::OnUserInput(
     const blink::WebInputEvent& event) {
   if (first_paint_triggered_ && !first_user_interaction_after_paint_) {
     DCHECK(!navigation_start_.is_null());
     first_user_interaction_after_paint_ =
         base::TimeTicks::Now() - navigation_start_;
   }
 }