PageLoadMetricsObservers observe individual page loads

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 8136caa588685ef6c1bcfdad062e635b4571f3c5..b640a9fbd2f778cc504734c301a3b9df1db26fa6 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -109,19 +109,26 @@ uint64_t RapporHistogramBucketIndex(const base::TimeDelta& time) {
 PageLoadTracker::PageLoadTracker(
     bool in_foreground,
     PageLoadMetricsEmbedderInterface* embedder_interface,
-    content::NavigationHandle* navigation_handle,
-    base::ObserverList<PageLoadMetricsObserver, true>* observers)
+    content::NavigationHandle* navigation_handle)
     : has_commit_(false),
       navigation_start_(navigation_handle->NavigationStart()),
       started_in_foreground_(in_foreground),
-      embedder_interface_(embedder_interface),
-      observers_(observers) {}
+      embedder_interface_(embedder_interface) {
+  embedder_interface_->RegisterObservers(this);
+  FOR_EACH_OBSERVER(PageLoadMetricsObserver, observers_,
+                    OnStart(navigation_handle));
+}
 
 PageLoadTracker::~PageLoadTracker() {
   if (has_commit_) {
     RecordTimingHistograms();
     RecordRappor();
   }
+  PageLoadExtraInfo info = GetPageLoadMetricsInfo();
+  FOR_EACH_OBSERVER(PageLoadMetricsObserver, observers_,
+                    OnComplete(timing_, info));
+  FOR_EACH_OBSERVER(PageLoadMetricsObserver, observers_,
+                    OnPageLoadMetricsGoingAway());
 }
 
 void PageLoadTracker::WebContentsHidden() {
@@ -143,14 +150,15 @@ void PageLoadTracker::Commit(content::NavigationHandle* navigation_handle) {
   // We log the event that this load started. Because we don't know if a load is
   // relevant or if it will commit before now, we have to log this event at
   // commit time.
-  RecordCommittedEvent(COMMITTED_LOAD_STARTED, !started_in_foreground_);
+  if (RendererTracked())
+    RecordCommittedEvent(RELEVANT_LOAD_STARTED, !started_in_foreground_);
 
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
+  FOR_EACH_OBSERVER(PageLoadMetricsObserver, observers_,
                     OnCommit(navigation_handle));
 }
 
 void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
+  FOR_EACH_OBSERVER(PageLoadMetricsObserver, observers_,
                     OnRedirect(navigation_handle));
 }
 
@@ -173,6 +181,18 @@ bool PageLoadTracker::HasBackgrounded() {
   return !started_in_foreground_ || !background_time_.is_null();
 }
 
+void PageLoadTracker::SetRendererTracked(bool renderer_tracked) {
+  renderer_tracked_ = renderer_tracked;
+}
+
+void PageLoadTracker::AddObserver(PageLoadMetricsObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void PageLoadTracker::RemoveObserver(PageLoadMetricsObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
   base::TimeDelta first_background_time;
   base::TimeDelta first_foreground_time;
@@ -181,7 +201,7 @@ PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
   if (!foreground_time_.is_null() && !started_in_foreground_)
     first_foreground_time = foreground_time_ - navigation_start_;
   return PageLoadExtraInfo(first_background_time, first_foreground_time,
-                           started_in_foreground_);
+                           started_in_foreground_, has_commit_);
 }
 
 const GURL& PageLoadTracker::GetCommittedURL() {
@@ -211,14 +231,13 @@ base::TimeDelta PageLoadTracker::GetBackgroundDelta() {
 
 void PageLoadTracker::RecordTimingHistograms() {
   DCHECK(has_commit_);
-  if (timing_.IsEmpty()) {
+  if (timing_.IsEmpty() && RendererTracked()) {
     RecordInternalError(ERR_NO_IPCS_RECEIVED);
     return;
   }
 
-  PageLoadExtraInfo info = GetPageLoadMetricsInfo();
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
-                    OnComplete(timing_, info));
+  if (!RendererTracked())
+    return;
 
   base::TimeDelta background_delta = GetBackgroundDelta();
 
@@ -239,16 +258,16 @@ void PageLoadTracker::RecordTimingHistograms() {
     }
   }
   if (timing_.first_layout.is_zero()) {
-    RecordCommittedEvent(COMMITTED_LOAD_FAILED_BEFORE_FIRST_LAYOUT,
+    RecordCommittedEvent(RELEVANT_LOAD_FAILED_BEFORE_FIRST_LAYOUT,
                          HasBackgrounded());
   } else {
     if (timing_.first_layout < background_delta) {
       PAGE_LOAD_HISTOGRAM(kHistogramFirstLayout, timing_.first_layout);
-      RecordCommittedEvent(COMMITTED_LOAD_SUCCESSFUL_FIRST_LAYOUT, false);
+      RecordCommittedEvent(RELEVANT_LOAD_SUCCESSFUL_FIRST_LAYOUT, false);
     } else {
       PAGE_LOAD_HISTOGRAM(kBackgroundHistogramFirstLayout,
                           timing_.first_layout);
-      RecordCommittedEvent(COMMITTED_LOAD_SUCCESSFUL_FIRST_LAYOUT, true);
+      RecordCommittedEvent(RELEVANT_LOAD_SUCCESSFUL_FIRST_LAYOUT, true);
     }
   }
   if (!timing_.first_paint.is_zero()) {
@@ -317,14 +336,14 @@ void PageLoadTracker::RecordProvisionalEvent(ProvisionalLoadEvent event) {
 // RecordCommittedEvent needs a backgrounded input because we need to special
 // case a few events that need either precise timing measurements, or different
 // logic than simply "Did I background before logging this event?"
-void PageLoadTracker::RecordCommittedEvent(CommittedLoadEvent event,
+void PageLoadTracker::RecordCommittedEvent(CommittedRelevantLoadEvent event,
                                            bool backgrounded) {
   if (backgrounded) {
     UMA_HISTOGRAM_ENUMERATION(kBackgroundCommittedEvents, event,
-                              COMMITTED_LOAD_LAST_ENTRY);
+                              RELEVANT_LOAD_LAST_ENTRY);
   } else {
     UMA_HISTOGRAM_ENUMERATION(kCommittedEvents, event,
-                              COMMITTED_LOAD_LAST_ENTRY);
+                              RELEVANT_LOAD_LAST_ENTRY);
   }
 }
 
@@ -379,18 +398,6 @@ MetricsWebContentsObserver::~MetricsWebContentsObserver() {
   // Reset PageLoadTrackers so observers get final notifications.
   committed_load_.reset();
   provisional_loads_.clear();
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, observers_,
-                    OnPageLoadMetricsGoingAway());
-}
-
-void MetricsWebContentsObserver::AddObserver(
-    PageLoadMetricsObserver* observer) {
-  observers_.AddObserver(observer);
-}
-
-void MetricsWebContentsObserver::RemoveObserver(
-    PageLoadMetricsObserver* observer) {
-  observers_.RemoveObserver(observer);
 }
 
 bool MetricsWebContentsObserver::OnMessageReceived(
@@ -420,9 +427,9 @@ void MetricsWebContentsObserver::DidStartNavigation(
   // the MetricsWebContentsObserver owns them both list and they are torn down
   // after the PageLoadTracker.
   provisional_loads_.insert(std::make_pair(
-      navigation_handle, make_scoped_ptr(new PageLoadTracker(
-                             in_foreground_, embedder_interface_.get(),
-                             navigation_handle, &observers_))));
+      navigation_handle,
+      make_scoped_ptr(new PageLoadTracker(
+          in_foreground_, embedder_interface_.get(), navigation_handle))));
 }
 
 void MetricsWebContentsObserver::DidFinishNavigation(
@@ -456,18 +463,15 @@ void MetricsWebContentsObserver::DidFinishNavigation(
   if (navigation_handle->IsSamePage())
     return;
 
-  // Eagerly log the previous UMA even if we don't care about the current
-  // navigation.
-  committed_load_.reset();
+  committed_load_ = finished_nav.Pass();
 
   const GURL& browser_url = web_contents()->GetLastCommittedURL();
   const std::string& mime_type = web_contents()->GetContentsMimeType();
   DCHECK(!browser_url.is_empty());
   DCHECK(!mime_type.empty());
-  if (!IsRelevantNavigation(navigation_handle, browser_url, mime_type))
-    return;
+  committed_load_->SetRendererTracked(
+      IsRelevantNavigation(navigation_handle, browser_url, mime_type));
 
-  committed_load_ = finished_nav.Pass();
   committed_load_->Commit(navigation_handle);
 }
 
@@ -511,8 +515,8 @@ void MetricsWebContentsObserver::OnTimingUpdated(
     content::RenderFrameHost* render_frame_host,
     const PageLoadTiming& timing) {
   bool error = false;
-  if (!committed_load_) {
-    RecordInternalError(ERR_IPC_WITH_NO_COMMITTED_LOAD);
+  if (!committed_load_ || !committed_load_->RendererTracked()) {
+    RecordInternalError(ERR_IPC_WITH_NO_RELEVANT_LOAD);
     error = true;
   }