Remove PageLoad.Timing2.NavigationToCommit histograms.

chrome/browser/page_load_metrics/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 "chrome/browser/page_load_metrics/metrics_web_contents_observer.h"
 
 #include <algorithm>
 #include <ostream>
 #include <string>
 #include <utility>
@@ skipping 47 matching lines @@
 const char kAbortChainSizeNewNavigation[] =
     "PageLoad.Internal.ProvisionalAbortChainSize.NewNavigation";
 const char kAbortChainSizeSameURL[] =
     "PageLoad.Internal.ProvisionalAbortChainSize.SameURL";
 const char kAbortChainSizeNoCommit[] =
     "PageLoad.Internal.ProvisionalAbortChainSize.NoCommit";
 const char kClientRedirectFirstPaintToNavigation[] =
     "PageLoad.Internal.ClientRedirect.FirstPaintToNavigation";
 const char kClientRedirectWithoutPaint[] =
     "PageLoad.Internal.ClientRedirect.NavigationWithoutPaint";
-const char kCommitToCompleteNoTimingIPCs[] =
-    "PageLoad.Internal.CommitToComplete.NoTimingIPCs";
 const char kPageLoadCompletedAfterAppBackground[] =
     "PageLoad.Internal.PageLoadCompleted.AfterAppBackground";
 
 }  // namespace internal
 
 namespace {
 
 // Helper to allow use of Optional<> values in LOG() messages.
 std::ostream& operator<<(std::ostream& os,
                          const base::Optional<base::TimeDelta>& opt) {
@@ skipping 215 matching lines @@
 PageLoadTracker::PageLoadTracker(
     bool in_foreground,
     PageLoadMetricsEmbedderInterface* embedder_interface,
     const GURL& currently_committed_url,
     content::NavigationHandle* navigation_handle,
     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_(url_),
+      start_url_(navigation_handle->GetURL()),
       abort_type_(ABORT_NONE),
       abort_user_initiated_(false),
       started_in_foreground_(in_foreground),
       page_transition_(navigation_handle->GetPageTransition()),
       num_cache_requests_(0),
       num_network_requests_(0),
       user_gesture_(IsNavigationUserInitiated(navigation_handle)),
       aborted_chain_size_(aborted_chain_size),
       aborted_chain_size_same_url_(aborted_chain_size_same_url),
       embedder_interface_(embedder_interface) {
@@ skipping 13 matching lines @@
 }
 
 PageLoadTracker::~PageLoadTracker() {
   if (app_entered_background_) {
     RecordAppBackgroundPageLoadCompleted(true);
   }
 
   if (did_stop_tracking_)
     return;
 
-  if (commit_time_.is_null()) {
+  if (committed_url_.is_empty()) {
     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) {
       LogAbortChainHistograms(nullptr);
     }
   } else if (timing_.IsEmpty()) {
     RecordInternalError(ERR_NO_IPCS_RECEIVED);
-    PAGE_LOAD_HISTOGRAM(internal::kCommitToCompleteNoTimingIPCs,
-                        base::TimeTicks::Now() - commit_time_);
   }
 
   const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
   for (const auto& observer : observers_) {
     if (failed_provisional_load_info_) {
       observer->OnFailedProvisionalLoad(*failed_provisional_load_info_, info);
-    } else if (info.time_to_commit) {
+    } else if (!info.committed_url.is_empty()) {
       observer->OnComplete(timing_, info);
     }
   }
 }
 
 void PageLoadTracker::LogAbortChainHistograms(
     content::NavigationHandle* final_navigation) {
   if (aborted_chain_size_ == 0)
     return;
   // Note that this could be broken out by this navigation's abort type, if more
   // granularity is needed. Add one to the chain size to count the current
   // navigation. In the other cases, the current navigation is the final
   // navigation (which commits).
   if (!final_navigation) {
     UMA_HISTOGRAM_COUNTS(internal::kAbortChainSizeNoCommit,
                          aborted_chain_size_ + 1);
     LogAbortChainSameURLHistogram(aborted_chain_size_same_url_ + 1);
     return;
   }
 
   // The following is only executed for committing trackers.
-  DCHECK(!commit_time_.is_null());
+  DCHECK(!committed_url_.is_empty());
 
   // 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:
       UMA_HISTOGRAM_COUNTS(internal::kAbortChainSizeReload,
@@ skipping 52 matching lines @@
     DCHECK_NE(started_in_foreground_, background_time_.is_null());
     foreground_time_ = base::TimeTicks::Now();
     ClampBrowserTimestampIfInterProcessTimeTickSkew(&foreground_time_);
   }
 
   for (const auto& observer : observers_)
     observer->OnShown();
 }
 
 void PageLoadTracker::Commit(content::NavigationHandle* navigation_handle) {
-  // TODO(bmcquade): To improve accuracy, consider adding commit time to
-  // NavigationHandle. Taking a timestamp here should be close enough for now.
-  commit_time_ = base::TimeTicks::Now();
-  ClampBrowserTimestampIfInterProcessTimeTickSkew(&commit_time_);
-  url_ = navigation_handle->GetURL();
+  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();
 
   INVOKE_AND_PRUNE_OBSERVERS(observers_, OnCommit, navigation_handle);
   LogAbortChainHistograms(navigation_handle);
 }
 
 void PageLoadTracker::FailedProvisionalLoad(
     content::NavigationHandle* navigation_handle) {
@@ skipping 50 matching lines @@
   // different start time from an earlier struct is considered invalid.
   bool valid_timing_descendent =
       timing_.navigation_start.is_null() ||
       timing_.navigation_start == new_timing.navigation_start;
   // Ensure flags sent previously are still present in the new metadata fields.
   bool valid_behavior_descendent =
       (metadata_.behavior_flags & new_metadata.behavior_flags) ==
       metadata_.behavior_flags;
   if (IsValidPageLoadTiming(new_timing) && valid_timing_descendent &&
       valid_behavior_descendent) {
-    DCHECK(!commit_time_.is_null());  // OnCommit() must be called first.
+    DCHECK(!committed_url_.is_empty());  // OnCommit() must be called first.
     // There are some subtle ordering constraints here. GetPageLoadMetricsInfo()
     // must be called before DispatchObserverTimingCallbacks, but its
     // implementation depends on the state of metadata_, so we need to update
     // metadata_ before calling GetPageLoadMetricsInfo. Thus, we make a copy of
     // timing here, update timing_ and metadata_, and then proceed to dispatch
     // the observer timing callbacks.
     const PageLoadTiming last_timing = timing_;
     timing_ = new_timing;
 
     const PageLoadMetadata last_metadata = metadata_;
@@ skipping 50 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> time_to_commit;
 
   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_;
   } else {
     DCHECK(abort_time_.is_null());
   }
 
-  if (!commit_time_.is_null()) {
-    DCHECK_GE(commit_time_, navigation_start_);
-    time_to_commit = commit_time_ - navigation_start_;
-  }
-
   // abort_type_ == ABORT_NONE implies !abort_user_initiated_.
   DCHECK(abort_type_ != ABORT_NONE || !abort_user_initiated_);
   return PageLoadExtraInfo(
       first_background_time, first_foreground_time, started_in_foreground_,
-      user_gesture_, commit_time_.is_null() ? GURL() : url_, start_url_,
-      time_to_commit, abort_type_, abort_user_initiated_, time_to_abort,
-      num_cache_requests_, num_network_requests_, metadata_);
+      user_gesture_, committed_url_, start_url_, abort_type_,
+      abort_user_initiated_, time_to_abort, num_cache_requests_,
+      num_network_requests_, metadata_);
 }
 
 void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
                                   bool user_initiated,
                                   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)
     return;
@@ skipping 22 matching lines @@
     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;
 }
 
 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_;
+  DCHECK(committed_url_.is_empty());
+  return navigation_handle->GetURL() == start_url_;
 }
 
 void PageLoadTracker::UpdateAbortInternal(UserAbortType abort_type,
                                           bool user_initiated,
                                           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
@@ skipping 419 matching lines @@
     content::NavigationHandle* navigation_handle) const {
   DCHECK(navigation_handle->IsInMainFrame());
   DCHECK(!navigation_handle->HasCommitted() ||
          !navigation_handle->IsSamePage());
 
   return BrowserPageTrackDecider(embedder_interface_.get(), web_contents(),
                                  navigation_handle).ShouldTrack();
 }
 
 }  // namespace page_load_metrics