Generalize abort tracking to page end state tracking

chrome/browser/page_load_metrics/page_load_tracker.cc

 // Copyright 2016 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/page_load_tracker.h"
 
 #include <algorithm>
 #include <ostream>
 #include <string>
 #include <utility>
@@ skipping 46 matching lines @@
     "PageLoad.Internal.PageLoadCompleted.AfterAppBackground";
 
 }  // namespace internal
 
 void RecordInternalError(InternalErrorLoadEvent event) {
   UMA_HISTOGRAM_ENUMERATION(internal::kErrorEvents, event, ERR_LAST_ENTRY);
 }
 
 // TODO(csharrison): Add a case for client side redirects, which is what JS
 // initiated window.location / window.history navigations get set to.
-UserAbortType AbortTypeForPageTransition(ui::PageTransition transition) {
+PageEndReason EndReasonForPageTransition(ui::PageTransition transition) {
   if (transition & ui::PAGE_TRANSITION_CLIENT_REDIRECT) {
-    return ABORT_CLIENT_REDIRECT;
+    return END_CLIENT_REDIRECT;
   }
   if (ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD))
-    return ABORT_RELOAD;
+    return END_RELOAD;
   if (transition & ui::PAGE_TRANSITION_FORWARD_BACK)
-    return ABORT_FORWARD_BACK;
+    return END_FORWARD_BACK;
   if (ui::PageTransitionIsNewNavigation(transition))
-    return ABORT_NEW_NAVIGATION;
+    return END_NEW_NAVIGATION;
   NOTREACHED()
-      << "AbortTypeForPageTransition received unexpected ui::PageTransition: "
+      << "EndReasonForPageTransition received unexpected ui::PageTransition: "
       << transition;
-  return ABORT_OTHER;
+  return END_OTHER;
 }
 
 void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url) {
   if (aborted_chain_size_same_url > 0) {
     UMA_HISTOGRAM_COUNTS(internal::kAbortChainSizeSameURL,
                          aborted_chain_size_same_url);
   }
 }
 
 bool IsNavigationUserInitiated(content::NavigationHandle* handle) {
@@ skipping 199 matching lines @@
     content::NavigationHandle* navigation_handle,
     UserInitiatedInfo user_initiated_info,
     int aborted_chain_size,
     int aborted_chain_size_same_url)
     : did_stop_tracking_(false),
       app_entered_background_(false),
       navigation_start_(navigation_handle->NavigationStart()),
       url_(navigation_handle->GetURL()),
       start_url_(navigation_handle->GetURL()),
       did_commit_(false),
-      abort_type_(ABORT_NONE),
-      abort_user_initiated_info_(UserInitiatedInfo::NotUserInitiated()),
+      page_end_reason_(END_NONE),
+      page_end_user_initiated_info_(UserInitiatedInfo::NotUserInitiated()),
       started_in_foreground_(in_foreground),
       page_transition_(navigation_handle->GetPageTransition()),
       user_initiated_info_(user_initiated_info),
       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);
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnStart, navigation_handle,
                              currently_committed_url, started_in_foreground_);
 }
 
 PageLoadTracker::~PageLoadTracker() {
   if (app_entered_background_) {
     RecordAppBackgroundPageLoadCompleted(true);
   }
 
   if (did_stop_tracking_)
     return;
 
+  if (started_in_foreground_) {

[Charlie Harrison, 2017/02/17 22:36:49]

I kinda think we should just set page_end_time_ = Now() here.

[Bryan McQuade, 2017/02/18 00:30:15]

Agree, done. I kept the error counter since I do want to know how often this
happens in the field (should be never).

+    DCHECK(!page_end_time_.is_null());
+    if (page_end_time_.is_null())
+      RecordInternalError(ERR_NO_PAGE_LOAD_END_TIME);
+  }
+
   if (!did_commit_) {
     if (!failed_provisional_load_info_)
       RecordInternalError(ERR_NO_COMMIT_OR_FAILED_PROVISIONAL_LOAD);
 
     // Don't include any aborts that resulted in a new navigation, as the chain
     // length will be included in the aborter PageLoadTracker.
-    if (abort_type_ != ABORT_RELOAD && abort_type_ != ABORT_FORWARD_BACK &&
-        abort_type_ != ABORT_NEW_NAVIGATION) {
+    if (page_end_reason_ != END_RELOAD &&
+        page_end_reason_ != END_FORWARD_BACK &&
+        page_end_reason_ != END_NEW_NAVIGATION) {
       LogAbortChainHistograms(nullptr);
     }
   } else if (timing_.IsEmpty()) {
     RecordInternalError(ERR_NO_IPCS_RECEIVED);
   }
 
   const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
   for (const auto& observer : observers_) {
     if (failed_provisional_load_info_) {
       observer->OnFailedProvisionalLoad(*failed_provisional_load_info_, info);
@@ skipping 20 matching lines @@
 
   // The following is only executed for committing trackers.
   DCHECK(did_commit_);
 
   // Note that histograms could be separated out by this commit's transition
   // type, but for simplicity they will all be bucketed together.
   LogAbortChainSameURLHistogram(aborted_chain_size_same_url_);
 
   ui::PageTransition committed_transition =
       final_navigation->GetPageTransition();
-  switch (AbortTypeForPageTransition(committed_transition)) {
-    case ABORT_RELOAD:
+  switch (EndReasonForPageTransition(committed_transition)) {
+    case END_RELOAD:
       UMA_HISTOGRAM_COUNTS(internal::kAbortChainSizeReload,
                            aborted_chain_size_);
       return;
-    case ABORT_FORWARD_BACK:
+    case END_FORWARD_BACK:
       UMA_HISTOGRAM_COUNTS(internal::kAbortChainSizeForwardBack,
                            aborted_chain_size_);
       return;
     // TODO(csharrison): Refactor this code so it is based on the WillStart*
     // code path instead of the committed load code path. Then, for every abort
     // chain, log a histogram of the counts of each of these metrics. For now,
     // merge client redirects with new navigations, which was (basically) the
     // previous behavior.
-    case ABORT_CLIENT_REDIRECT:
-    case ABORT_NEW_NAVIGATION:
+    case END_CLIENT_REDIRECT:
+    case END_NEW_NAVIGATION:
       UMA_HISTOGRAM_COUNTS(internal::kAbortChainSizeNewNavigation,
                            aborted_chain_size_);
       return;
     default:
       NOTREACHED()
           << "LogAbortChainHistograms received unexpected ui::PageTransition: "
           << committed_transition;
       return;
   }
 }
 
 void PageLoadTracker::WebContentsHidden() {
   // Only log the first time we background in a given page load.
   if (background_time_.is_null()) {
     // Make sure we either started in the foreground and haven't been
     // foregrounded yet, or started in the background and have already been
     // foregrounded.
     DCHECK_EQ(started_in_foreground_, foreground_time_.is_null());
     background_time_ = base::TimeTicks::Now();
     ClampBrowserTimestampIfInterProcessTimeTickSkew(&background_time_);
-    // Though most cases where a tab is backgrounded are user initiated, we
-    // can't be certain that we were backgrounded due to a user action. For
-    // example, on Android, the screen times out after a period of inactivity,
-    // resulting in a non-user-initiated backgrounding.
-    NotifyAbort(ABORT_BACKGROUND, UserInitiatedInfo::NotUserInitiated(),
-                background_time_, true);
   }
   const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnHidden, timing_, info);
 }
 
 void PageLoadTracker::WebContentsShown() {
   // Only log the first time we foreground in a given page load.
   if (foreground_time_.is_null()) {
     // Make sure we either started in the background and haven't been
     // backgrounded yet, or started in the foreground and have already been
@@ skipping 25 matching lines @@
   url_ = navigation_handle->GetURL();
   // Some transitions (like CLIENT_REDIRECT) are only known at commit time.
   page_transition_ = navigation_handle->GetPageTransition();
   user_initiated_info_.user_gesture = navigation_handle->HasUserGesture();
 
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnCommit, navigation_handle);
   LogAbortChainHistograms(navigation_handle);
 }
 
 void PageLoadTracker::FailedProvisionalLoad(
-    content::NavigationHandle* navigation_handle) {
+    content::NavigationHandle* navigation_handle,
+    base::TimeTicks failed_load_time) {
   DCHECK(!failed_provisional_load_info_);
   failed_provisional_load_info_.reset(new FailedProvisionalLoadInfo(
-      base::TimeTicks::Now() - navigation_handle->NavigationStart(),
+      failed_load_time - navigation_handle->NavigationStart(),
       navigation_handle->GetNetErrorCode()));
 }
 
 void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
   url_ = navigation_handle->GetURL();
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnRedirect, navigation_handle);
 }
 
 void PageLoadTracker::OnInputEvent(const blink::WebInputEvent& event) {
   input_tracker_.OnInputEvent(event);
@@ skipping 106 matching lines @@
 
   if (!event_time->is_null() && *event_time < navigation_start_) {
     RecordInternalError(ERR_INTER_PROCESS_TIME_TICK_SKEW);
     *event_time = navigation_start_;
   }
 }
 
 PageLoadExtraInfo PageLoadTracker::ComputePageLoadExtraInfo() {
   base::Optional<base::TimeDelta> first_background_time;
   base::Optional<base::TimeDelta> first_foreground_time;
-  base::Optional<base::TimeDelta> time_to_abort;
+  base::Optional<base::TimeDelta> page_end_time;
 
   if (!background_time_.is_null()) {
     DCHECK_GE(background_time_, navigation_start_);
     first_background_time = background_time_ - navigation_start_;
   }
 
   if (!foreground_time_.is_null()) {
     DCHECK_GE(foreground_time_, navigation_start_);
     first_foreground_time = foreground_time_ - navigation_start_;
   }
 
-  if (abort_type_ != ABORT_NONE) {
-    DCHECK_GE(abort_time_, navigation_start_);
-    time_to_abort = abort_time_ - navigation_start_;
+  if (page_end_reason_ != END_NONE) {
+    DCHECK_GE(page_end_time_, navigation_start_);
+    page_end_time = page_end_time_ - navigation_start_;
   } else {
-    DCHECK(abort_time_.is_null());
+    DCHECK(page_end_time_.is_null());
   }
 
-  // abort_type_ == ABORT_NONE implies abort_user_initiated_info_ is not user
-  // initiated.
-  DCHECK(abort_type_ != ABORT_NONE ||
-         (!abort_user_initiated_info_.browser_initiated &&
-          !abort_user_initiated_info_.user_gesture &&
-          !abort_user_initiated_info_.user_input_event));
+  // page_end_reason_ == END_NONE implies page_end_user_initiated_info_ is not
+  // user initiated.
+  DCHECK(page_end_reason_ != END_NONE ||
+         (!page_end_user_initiated_info_.browser_initiated &&
+          !page_end_user_initiated_info_.user_gesture &&
+          !page_end_user_initiated_info_.user_input_event));
   return PageLoadExtraInfo(
       first_background_time, first_foreground_time, started_in_foreground_,
-      user_initiated_info_, url(), start_url_, did_commit_, abort_type_,
-      abort_user_initiated_info_, time_to_abort, metadata_);
+      user_initiated_info_, url(), start_url_, did_commit_, page_end_reason_,
+      page_end_user_initiated_info_, page_end_time, metadata_);
 }
 
 bool PageLoadTracker::HasMatchingNavigationRequestID(
     const content::GlobalRequestID& request_id) const {
   DCHECK(request_id != content::GlobalRequestID());
   return navigation_request_id_.has_value() &&
          navigation_request_id_.value() == request_id;
 }
 
-void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
-                                  UserInitiatedInfo user_initiated_info,
-                                  base::TimeTicks timestamp,
-                                  bool is_certainly_browser_timestamp) {
-  DCHECK_NE(abort_type, ABORT_NONE);
-  // Use UpdateAbort to update an already notified PageLoadTracker.
-  if (abort_type_ != ABORT_NONE)
+void PageLoadTracker::NotifyPageEnd(PageEndReason page_end_reason,
+                                    UserInitiatedInfo user_initiated_info,
+                                    base::TimeTicks timestamp,
+                                    bool is_certainly_browser_timestamp) {
+  DCHECK_NE(page_end_reason, END_NONE);
+  // Use UpdatePageEnd to update an already notified PageLoadTracker.
+  if (page_end_reason_ != END_NONE)
     return;
 
-  UpdateAbortInternal(abort_type, user_initiated_info, timestamp,
-                      is_certainly_browser_timestamp);
+  UpdatePageEndInternal(page_end_reason, user_initiated_info, timestamp,
+                        is_certainly_browser_timestamp);
 }
 
-void PageLoadTracker::UpdateAbort(UserAbortType abort_type,
-                                  UserInitiatedInfo user_initiated_info,
-                                  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);
+void PageLoadTracker::UpdatePageEnd(PageEndReason page_end_reason,
+                                    UserInitiatedInfo user_initiated_info,
+                                    base::TimeTicks timestamp,
+                                    bool is_certainly_browser_timestamp) {
+  DCHECK_NE(page_end_reason, END_NONE);
+  DCHECK_NE(page_end_reason, END_OTHER);
+  DCHECK_EQ(page_end_reason_, END_OTHER);
+  DCHECK(!page_end_time_.is_null());
+  if (page_end_time_.is_null() || page_end_reason_ != END_OTHER)
+    return;
 
   // 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, user_initiated_info,
-                      std::min(abort_time_, timestamp),
-                      is_certainly_browser_timestamp);
+  UpdatePageEndInternal(page_end_reason, user_initiated_info,
+                        std::min(page_end_time_, timestamp),
+                        is_certainly_browser_timestamp);
 }
 
 bool PageLoadTracker::IsLikelyProvisionalAbort(
     base::TimeTicks abort_cause_time) const {
-  // Note that |abort_cause_time - abort_time| can be negative.
-  return abort_type_ == ABORT_OTHER &&
-         (abort_cause_time - abort_time_).InMilliseconds() < 100;
+  // Note that |abort_cause_time - page_end_time_| can be negative.
+  return page_end_reason_ == END_OTHER &&
+         (abort_cause_time - page_end_time_).InMilliseconds() < 100;
 }
 
 bool PageLoadTracker::MatchesOriginalNavigation(
     content::NavigationHandle* navigation_handle) {
   // Neither navigation should have committed.
   DCHECK(!navigation_handle->HasCommitted());
   DCHECK(!did_commit_);
   return navigation_handle->GetURL() == start_url_;
 }
 
-void PageLoadTracker::UpdateAbortInternal(UserAbortType abort_type,
-                                          UserInitiatedInfo user_initiated_info,
-                                          base::TimeTicks timestamp,
-                                          bool is_certainly_browser_timestamp) {
+void PageLoadTracker::UpdatePageEndInternal(
+    PageEndReason page_end_reason,
+    UserInitiatedInfo user_initiated_info,
+    base::TimeTicks timestamp,
+    bool is_certainly_browser_timestamp) {
   // When a provisional navigation commits, that navigation's start time is
   // interpreted as the abort time for other provisional loads in the tab.
   // However, this only makes sense if the committed load started after the
   // aborted provisional loads started. Thus we ignore cases where the committed
   // load started before the aborted provisional load, as this would result in
   // recording a negative time-to-abort. The real issue here is that we have to
   // infer the cause of aborts. It would be better if the navigation code could
   // instead report the actual cause of an aborted navigation. See crbug/571647
   // for details.
   if (timestamp < navigation_start_) {
-    RecordInternalError(ERR_ABORT_BEFORE_NAVIGATION_START);
-    abort_type_ = ABORT_NONE;
-    abort_time_ = base::TimeTicks();
+    RecordInternalError(ERR_END_BEFORE_NAVIGATION_START);
+    page_end_reason_ = END_NONE;
+    page_end_time_ = base::TimeTicks();
     return;
   }
-  abort_type_ = abort_type;
-  abort_time_ = timestamp;
+  page_end_reason_ = page_end_reason;
+  page_end_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.
-  if (abort_type != ABORT_CLIENT_REDIRECT)
-    abort_user_initiated_info_ = user_initiated_info;
+  if (page_end_reason != END_CLIENT_REDIRECT)
+    page_end_user_initiated_info_ = user_initiated_info;
 
   if (is_certainly_browser_timestamp) {
-    ClampBrowserTimestampIfInterProcessTimeTickSkew(&abort_time_);
+    ClampBrowserTimestampIfInterProcessTimeTickSkew(&page_end_time_);
   }
 }
 
 }  // namespace page_load_metrics