Instead of PageLoadMetricsObservers observing load events occurring in a tab, this change makes observers observe individual page loads.

This allows observers to keep a much simpler state of the world, because they only get notifications about a single load.

This change forces observers lifetime to be scoped to a single page load.
BUG=546116

Review URL: https://codereview.chromium.org/1473153002

Cr-Commit-Position: refs/heads/master@{#363480}

15 files changed, 195 insertions, 224 deletions

diff --git a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
index da8046a..bccbd59 100644
--- a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
@@ -57,9 +57,8 @@ bool ShouldLogEvent(const base::TimeDelta& event,
 
 }  // namespace
 
-FromGWSPageLoadMetricsObserver::FromGWSPageLoadMetricsObserver(
-    page_load_metrics::PageLoadMetricsObservable* metrics)
-    : navigation_from_gws_(false), metrics_(metrics) {}
+FromGWSPageLoadMetricsObserver::FromGWSPageLoadMetricsObserver()
+    : navigation_from_gws_(false) {}
 
 void FromGWSPageLoadMetricsObserver::OnCommit(
     content::NavigationHandle* navigation_handle) {
@@ -116,11 +115,6 @@ void FromGWSPageLoadMetricsObserver::OnComplete(
   }
 }
 
-void FromGWSPageLoadMetricsObserver::OnPageLoadMetricsGoingAway() {
-  metrics_->RemoveObserver(this);
-  delete this;
-}
-
 void FromGWSPageLoadMetricsObserver::SetCommittedURLAndReferrer(
     const GURL& url,
     const content::Referrer& referrer) {
diff --git a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h
index 58eebca..0465ede 100644
--- a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h
+++ b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h
@@ -11,14 +11,12 @@
 class FromGWSPageLoadMetricsObserver
     : public page_load_metrics::PageLoadMetricsObserver {
  public:
-  explicit FromGWSPageLoadMetricsObserver(
-      page_load_metrics::PageLoadMetricsObservable* metrics);
+  FromGWSPageLoadMetricsObserver();
   // page_load_metrics::PageLoadMetricsObserver implementation:
   void OnCommit(content::NavigationHandle* navigation_handle) override;
   void OnComplete(
       const page_load_metrics::PageLoadTiming& timing,
       const page_load_metrics::PageLoadExtraInfo& extra_info) override;
-  void OnPageLoadMetricsGoingAway() override;
 
  protected:
   // Called in tests.
@@ -27,7 +25,6 @@ class FromGWSPageLoadMetricsObserver
 
  private:
   bool navigation_from_gws_;
-  page_load_metrics::PageLoadMetricsObservable* const metrics_;
 
   DISALLOW_COPY_AND_ASSIGN(FromGWSPageLoadMetricsObserver);
 };
diff --git a/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc b/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc
index c961d9c..9d47fd04 100644
--- a/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc
@@ -41,29 +41,22 @@ bool IsGoogleCaptcha(const GURL& url) {
       && base::StartsWith(url.path(), "/sorry", base::CompareCase::SENSITIVE);
 }
 
-GoogleCaptchaObserver::GoogleCaptchaObserver(
-    page_load_metrics::PageLoadMetricsObservable* metrics)
-    : saw_solution_(false), metrics_(metrics) {}
+GoogleCaptchaObserver::GoogleCaptchaObserver() : saw_solution_(false) {}
 
 void GoogleCaptchaObserver::OnCommit(
     content::NavigationHandle* navigation_handle) {
-  if (IsGoogleCaptcha(navigation_handle->GetURL()))
+  if (!navigation_handle->IsSamePage()
+      && IsGoogleCaptcha(navigation_handle->GetURL())) {
     RecordGoogleCaptchaEvent(GOOGLE_CAPTCHA_SHOWN);
-  if (saw_solution_) {
-    RecordGoogleCaptchaEvent(GOOGLE_CAPTCHA_SOLVED);
-    saw_solution_ = false;
   }
 }
 
 void GoogleCaptchaObserver::OnRedirect(
     content::NavigationHandle* navigation_handle) {
-  if (IsGoogleCaptcha(navigation_handle->GetReferrer().url))
+  if (IsGoogleCaptcha(navigation_handle->GetReferrer().url) && !saw_solution_) {
+    RecordGoogleCaptchaEvent(GOOGLE_CAPTCHA_SOLVED);
     saw_solution_ = true;
-}
-
-void GoogleCaptchaObserver::OnPageLoadMetricsGoingAway() {
-  metrics_->RemoveObserver(this);
-  delete this;
+  }
 }
 
 }  // namespace google_captcha_observer
diff --git a/chrome/browser/page_load_metrics/observers/google_captcha_observer.h b/chrome/browser/page_load_metrics/observers/google_captcha_observer.h
index 473b9e9..c093f12 100644
--- a/chrome/browser/page_load_metrics/observers/google_captcha_observer.h
+++ b/chrome/browser/page_load_metrics/observers/google_captcha_observer.h
@@ -16,19 +16,15 @@ bool IsGoogleCaptcha(const GURL& url);
 class GoogleCaptchaObserver
     : public page_load_metrics::PageLoadMetricsObserver {
  public:
-  explicit GoogleCaptchaObserver(
-      page_load_metrics::PageLoadMetricsObservable* metrics);
+  GoogleCaptchaObserver();
 
   // page_load_metrics::PageLoadMetricsObserver implementation:
   void OnCommit(content::NavigationHandle* navigation_handle) override;
   void OnRedirect(content::NavigationHandle* navigation_handle) override;
-  void OnPageLoadMetricsGoingAway() override;
 
  private:
   bool saw_solution_;
-  page_load_metrics::PageLoadMetricsObservable* const metrics_;
   DISALLOW_COPY_AND_ASSIGN(GoogleCaptchaObserver);
-
 };
 
 }  // namespace google_captcha_observer
diff --git a/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc b/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc
index da6a7a4..cdcf590 100644
--- a/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc
+++ b/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc
@@ -19,32 +19,43 @@ const char kHistogramNameFromGWSFirstTextPaint[] =
 
 }  // namespace
 
-class TestPageLoadMetricsEmbedderInterface
-    : public page_load_metrics::PageLoadMetricsEmbedderInterface {
- public:
-  TestPageLoadMetricsEmbedderInterface() {}
-  rappor::RapporService* GetRapporService() override { return nullptr; }
-  bool IsPrerendering(content::WebContents* web_contents) override {
-    return false;
-  }
-};
-
 class TestFromGWSPageLoadMetricsObserver
     : public FromGWSPageLoadMetricsObserver {
  public:
-  explicit TestFromGWSPageLoadMetricsObserver(
-      page_load_metrics::PageLoadMetricsObservable* metrics)
-      : FromGWSPageLoadMetricsObserver(metrics) {}
+  explicit TestFromGWSPageLoadMetricsObserver(const content::Referrer& referrer)
+      : FromGWSPageLoadMetricsObserver(), referrer_(referrer) {}
   void OnCommit(content::NavigationHandle* navigation_handle) override {
     const GURL& url = navigation_handle->GetURL();
     SetCommittedURLAndReferrer(
         url, content::Referrer::SanitizeForRequest(url, referrer_));
   }
 
+ private:
+  const content::Referrer referrer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFromGWSPageLoadMetricsObserver);
+};
+
+class TestPageLoadMetricsEmbedderInterface
+    : public page_load_metrics::PageLoadMetricsEmbedderInterface {
+ public:
+  TestPageLoadMetricsEmbedderInterface()
+      : referrer_(content::Referrer(GURL("https://www.google.com"),
+                                    blink::WebReferrerPolicyDefault)) {}
+  rappor::RapporService* GetRapporService() override { return nullptr; }
+  bool IsPrerendering(content::WebContents* web_contents) override {
+    return false;
+  }
+  void RegisterObservers(page_load_metrics::PageLoadTracker* tracker) override {
+    tracker->AddObserver(
+        make_scoped_ptr(new TestFromGWSPageLoadMetricsObserver(referrer_)));
+  }
   void set_referrer(const content::Referrer& referrer) { referrer_ = referrer; }
 
  private:
   content::Referrer referrer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestPageLoadMetricsEmbedderInterface);
 };
 
 class PageLoadMetricsObserverTest : public ChromeRenderViewHostTestHarness {
@@ -55,23 +66,17 @@ class PageLoadMetricsObserverTest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::SetUp();
     SetContents(CreateTestWebContents());
     NavigateAndCommit(GURL("http://www.google.com"));
+    embedder_interface_ = new TestPageLoadMetricsEmbedderInterface();
     observer_ =
         make_scoped_ptr(new page_load_metrics::MetricsWebContentsObserver(
-            web_contents(),
-            make_scoped_ptr(new TestPageLoadMetricsEmbedderInterface())));
+            web_contents(), make_scoped_ptr(embedder_interface_)));
     observer_->WasShown();
-
-    // Add PageLoadMetricsObservers here.
-    gws_observer_ = new TestFromGWSPageLoadMetricsObserver(observer_.get());
-    observer_->AddObserver(gws_observer_);
   }
 
   base::HistogramTester histogram_tester_;
+  TestPageLoadMetricsEmbedderInterface* embedder_interface_;
   scoped_ptr<page_load_metrics::MetricsWebContentsObserver> observer_;
 
-  // PageLoadMetricsObservers:
-  TestFromGWSPageLoadMetricsObserver* gws_observer_;
-
   DISALLOW_COPY_AND_ASSIGN(PageLoadMetricsObserverTest);
 };
 
@@ -99,7 +104,7 @@ TEST_F(PageLoadMetricsObserverTest, ReferralsFromGWSHTTPToHTTPS) {
   timing.navigation_start = base::Time::FromDoubleT(1);
   timing.first_text_paint = base::TimeDelta::FromMilliseconds(1);
   // HTTPS google.com referral  to HTTP example.com.
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://www.google.com"), blink::WebReferrerPolicyOrigin));
   NavigateAndCommit(GURL("http://www.example.com"));
 
@@ -120,7 +125,7 @@ TEST_F(PageLoadMetricsObserverTest, ReferralFromGWS) {
   timing.navigation_start = base::Time::FromDoubleT(1);
   timing.first_text_paint = base::TimeDelta::FromMilliseconds(1);
 
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://www.google.com/url"), blink::WebReferrerPolicyDefault));
   NavigateAndCommit(GURL("https://www.example.com"));
 
@@ -141,7 +146,7 @@ TEST_F(PageLoadMetricsObserverTest, ReferralFromGWSBackgroundLater) {
   timing.navigation_start = base::Time::FromDoubleT(1);
   timing.first_text_paint = base::TimeDelta::FromMicroseconds(1);
 
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://www.google.com/url"), blink::WebReferrerPolicyDefault));
   NavigateAndCommit(GURL("https://www.example.com"));
 
@@ -164,7 +169,7 @@ TEST_F(PageLoadMetricsObserverTest, ReferralsFromCaseInsensitive) {
   timing.navigation_start = base::Time::FromDoubleT(1);
   timing.first_text_paint = base::TimeDelta::FromMilliseconds(1);
   // HTTPS google.com referral  to HTTP example.com.
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://wWw.GoOGlE.cOm/webhp"), blink::WebReferrerPolicyOrigin));
   NavigateAndCommit(GURL("https://www.example.com"));
 
@@ -185,7 +190,7 @@ TEST_F(PageLoadMetricsObserverTest, ReferralsFromGWSOrigin) {
   timing.navigation_start = base::Time::FromDoubleT(1);
   timing.first_text_paint = base::TimeDelta::FromMilliseconds(1);
   // HTTPS google.com referral  to HTTP example.com.
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://www.google.com"), blink::WebReferrerPolicyOrigin));
   NavigateAndCommit(GURL("https://www.example.com"));
 
@@ -197,7 +202,7 @@ TEST_F(PageLoadMetricsObserverTest, ReferralsFromGWSOrigin) {
   timing2.navigation_start = base::Time::FromDoubleT(10);
   timing2.first_text_paint = base::TimeDelta::FromMilliseconds(100);
   // HTTPS google.com referral  to HTTP example.com.
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://www.google.co.in"), blink::WebReferrerPolicyOrigin));
   NavigateAndCommit(GURL("https://www.example2.com"));
 
@@ -219,9 +224,9 @@ TEST_F(PageLoadMetricsObserverTest, ReferralNotFromGWS) {
   page_load_metrics::PageLoadTiming timing;
   timing.navigation_start = base::Time::FromDoubleT(1);
   timing.first_text_paint = base::TimeDelta::FromMilliseconds(1);
-  NavigateAndCommit(GURL("https://www.example.com"));
-  gws_observer_->set_referrer(content::Referrer(
+  embedder_interface_->set_referrer(content::Referrer(
       GURL("https://www.anothersite.com"), blink::WebReferrerPolicyDefault));
+  NavigateAndCommit(GURL("https://www.example.com"));
 
   observer_->OnMessageReceived(
       PageLoadMetricsMsg_TimingUpdated(observer_->routing_id(), timing),
diff --git a/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc
index cfa4fbb..58df2d1 100644
--- a/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc
+++ b/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc
@@ -11,9 +11,8 @@
 
 namespace chrome {
 
-StaleWhileRevalidateMetricsObserver::StaleWhileRevalidateMetricsObserver(
-    page_load_metrics::PageLoadMetricsObservable* metrics)
-    : is_interesting_domain_(false), metrics_(metrics) {}
+StaleWhileRevalidateMetricsObserver::StaleWhileRevalidateMetricsObserver()
+    : is_interesting_domain_(false) {}
 
 void StaleWhileRevalidateMetricsObserver::OnCommit(
     content::NavigationHandle* navigation_handle) {
@@ -47,9 +46,4 @@ void StaleWhileRevalidateMetricsObserver::OnComplete(
   }
 }
 
-void StaleWhileRevalidateMetricsObserver::OnPageLoadMetricsGoingAway() {
-  metrics_->RemoveObserver(this);
-  delete this;
-}
-
 }  // namespace chrome
diff --git a/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.h b/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.h
index d60aeb9..19fb188 100644
--- a/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.h
+++ b/chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.h
@@ -13,20 +13,17 @@ namespace chrome {
 class StaleWhileRevalidateMetricsObserver
     : public page_load_metrics::PageLoadMetricsObserver {
  public:
-  explicit StaleWhileRevalidateMetricsObserver(
-      page_load_metrics::PageLoadMetricsObservable* metrics);
+  StaleWhileRevalidateMetricsObserver();
   // page_load_metrics::PageLoadMetricsObserver implementation:
   void OnCommit(content::NavigationHandle* navigation_handle) override;
   void OnComplete(
       const page_load_metrics::PageLoadTiming& timing,
       const page_load_metrics::PageLoadExtraInfo& extra_info) override;
-  void OnPageLoadMetricsGoingAway() override;
 
  private:
   // True if the committed URL is one of the domains of interest to the
   // stale-while-revalidate experiment.
   bool is_interesting_domain_;
-  page_load_metrics::PageLoadMetricsObservable* const metrics_;
 
   DISALLOW_COPY_AND_ASSIGN(StaleWhileRevalidateMetricsObserver);
 };
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
index 9dcbb11..21a990a 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
+++ b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
@@ -13,40 +13,30 @@
 #include "components/rappor/rappor_service.h"
 #include "content/public/browser/web_contents.h"
 
-namespace {
-
-void RegisterPageLoadMetricsObservers(
-    page_load_metrics::PageLoadMetricsObservable* metrics) {
-  // Attach observers scoped to the web contents here.
-
-  // This is a self-destruct class, and will delete itself when triggered by
-  // OnPageLoadMetricsGoingAway.
-  metrics->AddObserver(new FromGWSPageLoadMetricsObserver(metrics));
-  metrics->AddObserver(
-      new google_captcha_observer::GoogleCaptchaObserver(metrics));
-  // StaleWhileRevalidateMetricsObserver also deletes itself from
-  // OnPageLoadMetricsGoingAway.
-  // TODO(ricea): Remove this in April 2016 or before. crbug.com/348877
-  metrics->AddObserver(
-      new chrome::StaleWhileRevalidateMetricsObserver(metrics));
-}
-
-}  // namespace
-
 namespace chrome {
 
 void InitializePageLoadMetricsForWebContents(
     content::WebContents* web_contents) {
-  RegisterPageLoadMetricsObservers(
-      page_load_metrics::MetricsWebContentsObserver::CreateForWebContents(
-          web_contents,
-          make_scoped_ptr(new PageLoadMetricsEmbedderInterfaceImpl())));
+  page_load_metrics::MetricsWebContentsObserver::CreateForWebContents(
+      web_contents,
+      make_scoped_ptr(new PageLoadMetricsEmbedder()));
 }
 
-PageLoadMetricsEmbedderInterfaceImpl::~PageLoadMetricsEmbedderInterfaceImpl() {}
+PageLoadMetricsEmbedder::~PageLoadMetricsEmbedder() {}
+
+void PageLoadMetricsEmbedder::RegisterObservers(
+    page_load_metrics::PageLoadTracker* tracker) {
+  // These classes are owned by the metrics.
+  tracker->AddObserver(make_scoped_ptr(new FromGWSPageLoadMetricsObserver()));
+  tracker->AddObserver(
+      make_scoped_ptr(new google_captcha_observer::GoogleCaptchaObserver()));
+  // TODO(ricea): Remove this in April 2016 or before. crbug.com/348877
+  tracker->AddObserver(
+      make_scoped_ptr(new chrome::StaleWhileRevalidateMetricsObserver()));
+}
 
 rappor::RapporService*
-PageLoadMetricsEmbedderInterfaceImpl::GetRapporService() {
+PageLoadMetricsEmbedder::GetRapporService() {
   // During the browser process shutdown path, calling this getter can
   // reinitialize multiple destroyed objects. This alters shutdown ordering.
   if (g_browser_process->IsShuttingDown())
@@ -54,7 +44,7 @@ PageLoadMetricsEmbedderInterfaceImpl::GetRapporService() {
   return g_browser_process->rappor_service();
 }
 
-bool PageLoadMetricsEmbedderInterfaceImpl::IsPrerendering(
+bool PageLoadMetricsEmbedder::IsPrerendering(
     content::WebContents* web_contents) {
   return prerender::PrerenderContents::FromWebContents(web_contents) != nullptr;
 }
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_initialize.h b/chrome/browser/page_load_metrics/page_load_metrics_initialize.h
index f9a78ca..e3aaa79 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_initialize.h
+++ b/chrome/browser/page_load_metrics/page_load_metrics_initialize.h
@@ -20,13 +20,14 @@ namespace chrome {
 void InitializePageLoadMetricsForWebContents(
     content::WebContents* web_contents);
 
-class PageLoadMetricsEmbedderInterfaceImpl
+class PageLoadMetricsEmbedder
     : public page_load_metrics::PageLoadMetricsEmbedderInterface {
  public:
   // PageLoadMetricsEmbedderInterface:
-  ~PageLoadMetricsEmbedderInterfaceImpl() override;
+  ~PageLoadMetricsEmbedder() override;
   rappor::RapporService* GetRapporService() override;
   bool IsPrerendering(content::WebContents* web_contents) override;
+  void RegisterObservers(page_load_metrics::PageLoadTracker* tracker) override;
 };
 
 }  // namespace chrome
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 8136caa..ee52195 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,27 @@ uint64_t RapporHistogramBucketIndex(const base::TimeDelta& time) {
 PageLoadTracker::PageLoadTracker(
     bool in_foreground,
     PageLoadMetricsEmbedderInterface* embedder_interface,
-    content::NavigationHandle* navigation_handle,
-    base::ObserverList<PageLoadMetricsObserver, true>* observers)
-    : has_commit_(false),
+    content::NavigationHandle* navigation_handle)
+    : renderer_tracked_(false),
+      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 (const auto& observer : observers_) {
+    observer->OnStart(navigation_handle);
+  }
+}
 
 PageLoadTracker::~PageLoadTracker() {
   if (has_commit_) {
     RecordTimingHistograms();
     RecordRappor();
   }
+  PageLoadExtraInfo info = GetPageLoadMetricsInfo();
+  for (const auto& observer : observers_) {
+    observer->OnComplete(timing_, info);
+  }
 }
 
 void PageLoadTracker::WebContentsHidden() {
@@ -143,15 +151,18 @@ 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 (renderer_tracked())
+    RecordCommittedEvent(RELEVANT_LOAD_STARTED, !started_in_foreground_);
 
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
-                    OnCommit(navigation_handle));
+  for (const auto& observer : observers_) {
+    observer->OnCommit(navigation_handle);
+  }
 }
 
 void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
-                    OnRedirect(navigation_handle));
+  for (const auto& observer : observers_) {
+    observer->OnRedirect(navigation_handle);
+  }
 }
 
 bool PageLoadTracker::UpdateTiming(const PageLoadTiming& new_timing) {
@@ -173,6 +184,15 @@ bool PageLoadTracker::HasBackgrounded() {
   return !started_in_foreground_ || !background_time_.is_null();
 }
 
+void PageLoadTracker::set_renderer_tracked(bool renderer_tracked) {
+  renderer_tracked_ = renderer_tracked;
+}
+
+void PageLoadTracker::AddObserver(
+    scoped_ptr<PageLoadMetricsObserver> observer) {
+  observers_.push_back(std::move(observer));
+}
+
 PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
   base::TimeDelta first_background_time;
   base::TimeDelta first_foreground_time;
@@ -181,10 +201,10 @@ 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() {
+const GURL& PageLoadTracker::committed_url() {
   DCHECK(has_commit_);
   return url_;
 }
@@ -211,15 +231,14 @@ base::TimeDelta PageLoadTracker::GetBackgroundDelta() {
 
 void PageLoadTracker::RecordTimingHistograms() {
   DCHECK(has_commit_);
+  if (!renderer_tracked())
+    return;
+
   if (timing_.IsEmpty()) {
     RecordInternalError(ERR_NO_IPCS_RECEIVED);
     return;
   }
 
-  PageLoadExtraInfo info = GetPageLoadMetricsInfo();
-  FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
-                    OnComplete(timing_, info));
-
   base::TimeDelta background_delta = GetBackgroundDelta();
 
   if (!timing_.dom_content_loaded_event_start.is_zero()) {
@@ -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,19 +336,19 @@ 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);
   }
 }
 
 void PageLoadTracker::RecordRappor() {
-  DCHECK(!GetCommittedURL().is_empty());
+  DCHECK(!committed_url().is_empty());
   rappor::RapporService* rappor_service =
       embedder_interface_->GetRapporService();
   if (!rappor_service)
@@ -340,8 +359,8 @@ void PageLoadTracker::RecordRappor() {
       first_contentful_paint < GetBackgroundDelta()) {
     scoped_ptr<rappor::Sample> sample =
         rappor_service->CreateSample(rappor::UMA_RAPPOR_TYPE);
-    sample->SetStringField("Domain", rappor::GetDomainAndRegistrySampleFromGURL(
-                                         GetCommittedURL()));
+    sample->SetStringField(
+        "Domain", rappor::GetDomainAndRegistrySampleFromGURL(committed_url()));
     uint64_t bucket_index = RapporHistogramBucketIndex(first_contentful_paint);
     sample->SetFlagsField("Bucket", uint64_t(1) << bucket_index,
                           kNumRapporHistogramBuckets);
@@ -375,23 +394,7 @@ MetricsWebContentsObserver* MetricsWebContentsObserver::CreateForWebContents(
   return metrics;
 }
 
-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);
-}
+MetricsWebContentsObserver::~MetricsWebContentsObserver() {}
 
 bool MetricsWebContentsObserver::OnMessageReceived(
     const IPC::Message& message,
@@ -420,9 +423,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 +459,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_->set_renderer_tracked(
+      IsRelevantNavigation(navigation_handle, browser_url, mime_type));
 
-  committed_load_ = finished_nav.Pass();
   committed_load_->Commit(navigation_handle);
 }
 
@@ -511,8 +511,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_->renderer_tracked()) {
+    RecordInternalError(ERR_IPC_WITH_NO_RELEVANT_LOAD);
     error = true;
   }
 
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.h b/components/page_load_metrics/browser/metrics_web_contents_observer.h
index 6bc2904..524e1c0 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.h
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h
@@ -34,6 +34,8 @@ class RapporService;
 
 namespace page_load_metrics {
 
+class PageLoadTracker;
+
 // These constants are for keeping the tests in sync.
 const char kHistogramFirstLayout[] = "PageLoad.Timing2.NavigationToFirstLayout";
 const char kHistogramFirstTextPaint[] =
@@ -117,8 +119,9 @@ enum ProvisionalLoadEvent {
   PROVISIONAL_LOAD_LAST_ENTRY
 };
 
-// CommittedLoadEvents are events that occur on committed loads that we track.
-// Note that we capture events only for committed loads that:
+// CommittedRelevantLoadEvents are events that occur on committed loads that the
+// MetricsWebContentsObserver tracks. Note events are only captured for
+// committed loads that:
 //  - Are http/https.
 //  - Not same-page navigations.
 //  - Are not navigations to an error page.
@@ -127,21 +130,21 @@ enum ProvisionalLoadEvent {
 // If you add elements to this enum, make sure you update the enum
 // value in histograms.xml. Only add elements to the end to prevent
 // inconsistencies between versions.
-enum CommittedLoadEvent {
-  // When a load that eventually commits started. Note we can't log this until
-  // commit time, but it represents when the actual page load started. Thus, we
-  // only separate this into .Background when a page load starts backgrounded.
-  COMMITTED_LOAD_STARTED,
+enum CommittedRelevantLoadEvent {
+  // When a load that eventually commits started. This cannot be logged until
+  // commit time, but it represents when the actual page load started. Thus, it
+  // only separates into .Background when a page load starts backgrounded.
+  RELEVANT_LOAD_STARTED,
 
   // These two events are disjoint. Sum them to find the total number of
-  // committed loads that we end up tracking.
-  COMMITTED_LOAD_FAILED_BEFORE_FIRST_LAYOUT,
-  COMMITTED_LOAD_SUCCESSFUL_FIRST_LAYOUT,
+  // committed loads that are tracked.
+  RELEVANT_LOAD_FAILED_BEFORE_FIRST_LAYOUT,
+  RELEVANT_LOAD_SUCCESSFUL_FIRST_LAYOUT,
 
   // TODO(csharrison) once first paint metrics are in place, add new events.
 
   // Add values before this final count.
-  COMMITTED_LOAD_LAST_ENTRY
+  RELEVANT_LOAD_LAST_ENTRY
 };
 
 // These errors are internal to the page_load_metrics subsystem and do not
@@ -162,7 +165,7 @@ enum InternalErrorLoadEvent {
   // We received an IPC when we weren't tracking a committed load. This will
   // often happen if we get an IPC from a bad URL scheme (that is, the renderer
   // sent us an IPC from a navigation we don't care about).
-  ERR_IPC_WITH_NO_COMMITTED_LOAD,
+  ERR_IPC_WITH_NO_RELEVANT_LOAD,
 
   // Received a notification from a frame that has been navigated away from.
   ERR_IPC_FROM_WRONG_FRAME,
@@ -170,7 +173,7 @@ enum InternalErrorLoadEvent {
   // We received an IPC even through the last committed url from the browser
   // was not http/s. This can happen with the renderer sending IPCs for the
   // new tab page. This will often come paired with
-  // ERR_IPC_WITH_NO_COMMITTED_LOAD.
+  // ERR_IPC_WITH_NO_RELEVANT_LOAD.
   ERR_IPC_FROM_BAD_URL_SCHEME,
 
   // If we track a navigation, but the renderer sends us no IPCs. This could
@@ -188,6 +191,7 @@ class PageLoadMetricsEmbedderInterface {
   virtual ~PageLoadMetricsEmbedderInterface() {}
   virtual rappor::RapporService* GetRapporService() = 0;
   virtual bool IsPrerendering(content::WebContents* web_contents) = 0;
+  virtual void RegisterObservers(PageLoadTracker* metrics) = 0;
 };
 
 // This class tracks a given page load, starting from navigation start /
@@ -197,12 +201,11 @@ class PageLoadMetricsEmbedderInterface {
 // well as a committed PageLoadTracker.
 class PageLoadTracker {
  public:
-  // Caller must guarantee that the observers and embedder_interface pointers
-  // outlives this class.
+  // Caller must guarantee that the embedder_interface pointer outlives this
+  // class.
   PageLoadTracker(bool in_foreground,
                   PageLoadMetricsEmbedderInterface* embedder_interface,
-                  content::NavigationHandle* navigation_handle,
-                  base::ObserverList<PageLoadMetricsObserver, true>* observers);
+                  content::NavigationHandle* navigation_handle);
   ~PageLoadTracker();
   void Redirect(content::NavigationHandle* navigation_handle);
   void Commit(content::NavigationHandle* navigation_handle);
@@ -212,18 +215,27 @@ class PageLoadTracker {
   // Returns true if the timing was successfully updated.
   bool UpdateTiming(const PageLoadTiming& timing);
   void RecordProvisionalEvent(ProvisionalLoadEvent event);
-  void RecordCommittedEvent(CommittedLoadEvent event, bool backgrounded);
+  void RecordCommittedEvent(CommittedRelevantLoadEvent event,
+                            bool backgrounded);
   bool HasBackgrounded();
 
+  void set_renderer_tracked(bool renderer_tracked);
+  bool renderer_tracked() { return renderer_tracked_; }
+
+  void AddObserver(scoped_ptr<PageLoadMetricsObserver> observer);
+
  private:
   PageLoadExtraInfo GetPageLoadMetricsInfo();
   // Only valid to call post-commit.
-  const GURL& GetCommittedURL();
+  const GURL& committed_url();
 
   base::TimeDelta GetBackgroundDelta();
   void RecordTimingHistograms();
   void RecordRappor();
 
+  // Whether the renderer should be sending timing IPCs to this page load.
+  bool renderer_tracked_;
+
   bool has_commit_;
 
   // The navigation start in TimeTicks, not the wall time reported by Blink.
@@ -242,8 +254,7 @@ class PageLoadTracker {
   // Interface to chrome features. Must outlive the class.
   PageLoadMetricsEmbedderInterface* const embedder_interface_;
 
-  // List of observers. This must outlive the class.
-  base::ObserverList<PageLoadMetricsObserver, true>* observers_;
+  std::vector<scoped_ptr<PageLoadMetricsObserver>> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(PageLoadTracker);
 };
@@ -252,8 +263,7 @@ class PageLoadTracker {
 // IPC messages received from a MetricsRenderFrameObserver.
 class MetricsWebContentsObserver
     : public content::WebContentsObserver,
-      public content::WebContentsUserData<MetricsWebContentsObserver>,
-      public PageLoadMetricsObservable {
+      public content::WebContentsUserData<MetricsWebContentsObserver> {
  public:
   // Note that the returned metrics is owned by the web contents.
   // The caller must guarantee that the RapporService (if non-null) will
@@ -266,9 +276,6 @@ class MetricsWebContentsObserver
       scoped_ptr<PageLoadMetricsEmbedderInterface> embedder_interface);
   ~MetricsWebContentsObserver() override;
 
-  void AddObserver(PageLoadMetricsObserver* observer) override;
-  void RemoveObserver(PageLoadMetricsObserver* observer) override;
-
   // content::WebContentsObserver implementation:
   bool OnMessageReceived(const IPC::Message& message,
                          content::RenderFrameHost* render_frame_host) override;
@@ -292,6 +299,10 @@ class MetricsWebContentsObserver
   // True if the web contents is currently in the foreground.
   bool in_foreground_;
 
+  // The PageLoadTrackers must be deleted before the |embedded_interface_|,
+  // because they hold a pointer to the |embedder_interface_|.
+  scoped_ptr<PageLoadMetricsEmbedderInterface> embedder_interface_;
+
   // This map tracks all of the navigations ongoing that are not committed
   // yet. Once a navigation is committed, it moves from the map to
   // committed_load_. Note that a PageLoadTrackers NavigationHandle is only
@@ -300,9 +311,6 @@ class MetricsWebContentsObserver
       provisional_loads_;
   scoped_ptr<PageLoadTracker> committed_load_;
 
-  scoped_ptr<PageLoadMetricsEmbedderInterface> embedder_interface_;
-  base::ObserverList<PageLoadMetricsObserver, true> observers_;
-
   DISALLOW_COPY_AND_ASSIGN(MetricsWebContentsObserver);
 };
 
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc b/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
index b1f3228..54060a0 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
@@ -37,6 +37,7 @@ class TestPageLoadMetricsEmbedderInterface
   bool IsPrerendering(content::WebContents* web_contents) override {
     return is_prerendering_;
   }
+  void RegisterObservers(PageLoadTracker* tracker) override {}
   void set_is_prerendering(bool is_prerendering) {
     is_prerendering_ = is_prerendering;
   }
@@ -88,7 +89,7 @@ class MetricsWebContentsObserverTest
     }
   }
 
-  void CheckCommittedEvent(CommittedLoadEvent event,
+  void CheckCommittedEvent(CommittedRelevantLoadEvent event,
                            int count,
                            bool background) {
     if (background) {
@@ -506,9 +507,9 @@ TEST_F(MetricsWebContentsObserverTest, DontLogIrrelevantNavigation) {
   web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl));
 
   CheckProvisionalEvent(PROVISIONAL_LOAD_COMMITTED, 2, false);
-  CheckCommittedEvent(COMMITTED_LOAD_STARTED, 1, false);
+  CheckCommittedEvent(RELEVANT_LOAD_STARTED, 1, false);
   CheckErrorEvent(ERR_IPC_FROM_BAD_URL_SCHEME, 1);
-  CheckErrorEvent(ERR_IPC_WITH_NO_COMMITTED_LOAD, 1);
+  CheckErrorEvent(ERR_IPC_WITH_NO_RELEVANT_LOAD, 1);
   CheckTotalEvents();
 }
 
@@ -550,8 +551,8 @@ TEST_F(MetricsWebContentsObserverTest, AbortCommittedLoadBeforeFirstLayout) {
   web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl2));
 
   CheckProvisionalEvent(PROVISIONAL_LOAD_COMMITTED, 2, false);
-  CheckCommittedEvent(COMMITTED_LOAD_STARTED, 2, false);
-  CheckCommittedEvent(COMMITTED_LOAD_FAILED_BEFORE_FIRST_LAYOUT, 1, false);
+  CheckCommittedEvent(RELEVANT_LOAD_STARTED, 2, false);
+  CheckCommittedEvent(RELEVANT_LOAD_FAILED_BEFORE_FIRST_LAYOUT, 1, false);
   CheckTotalEvents();
 }
 
@@ -571,8 +572,8 @@ TEST_F(MetricsWebContentsObserverTest, SuccessfulFirstLayoutInForegroundEvent) {
   web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl2));
 
   CheckProvisionalEvent(PROVISIONAL_LOAD_COMMITTED, 2, false);
-  CheckCommittedEvent(COMMITTED_LOAD_STARTED, 2, false);
-  CheckCommittedEvent(COMMITTED_LOAD_SUCCESSFUL_FIRST_LAYOUT, 1, false);
+  CheckCommittedEvent(RELEVANT_LOAD_STARTED, 2, false);
+  CheckCommittedEvent(RELEVANT_LOAD_SUCCESSFUL_FIRST_LAYOUT, 1, false);
   CheckTotalEvents();
 }
 
@@ -598,9 +599,9 @@ TEST_F(MetricsWebContentsObserverTest,
 
   CheckProvisionalEvent(PROVISIONAL_LOAD_COMMITTED, 1, true);
   CheckProvisionalEvent(PROVISIONAL_LOAD_COMMITTED, 1, false);
-  CheckCommittedEvent(COMMITTED_LOAD_STARTED, 1, true);
-  CheckCommittedEvent(COMMITTED_LOAD_STARTED, 1, false);
-  CheckCommittedEvent(COMMITTED_LOAD_SUCCESSFUL_FIRST_LAYOUT, 1, true);
+  CheckCommittedEvent(RELEVANT_LOAD_STARTED, 1, true);
+  CheckCommittedEvent(RELEVANT_LOAD_STARTED, 1, false);
+  CheckCommittedEvent(RELEVANT_LOAD_SUCCESSFUL_FIRST_LAYOUT, 1, true);
   CheckTotalEvents();
 }
 
@@ -622,7 +623,7 @@ TEST_F(MetricsWebContentsObserverTest, BadIPC) {
       main_rfh());
 
   CheckProvisionalEvent(PROVISIONAL_LOAD_COMMITTED, 1, false);
-  CheckCommittedEvent(COMMITTED_LOAD_STARTED, 1, false);
+  CheckCommittedEvent(RELEVANT_LOAD_STARTED, 1, false);
   CheckErrorEvent(ERR_BAD_TIMING_IPC, 1);
   CheckTotalEvents();
 }
diff --git a/components/page_load_metrics/browser/page_load_metrics_observer.cc b/components/page_load_metrics/browser/page_load_metrics_observer.cc
index 05b3730..436c673 100644
--- a/components/page_load_metrics/browser/page_load_metrics_observer.cc
+++ b/components/page_load_metrics/browser/page_load_metrics_observer.cc
@@ -9,9 +9,11 @@ namespace page_load_metrics {
 PageLoadExtraInfo::PageLoadExtraInfo(
     const base::TimeDelta& first_background_time,
     const base::TimeDelta& first_foreground_time,
-    bool started_in_foreground)
+    bool started_in_foreground,
+    bool has_commit)
     : first_background_time(first_background_time),
       first_foreground_time(first_foreground_time),
-      started_in_foreground(started_in_foreground) {}
+      started_in_foreground(started_in_foreground),
+      has_commit(has_commit) {}
 
 }  // namespace page_load_metrics
diff --git a/components/page_load_metrics/browser/page_load_metrics_observer.h b/components/page_load_metrics/browser/page_load_metrics_observer.h
index 08a35dd..dddfb82 100644
--- a/components/page_load_metrics/browser/page_load_metrics_observer.h
+++ b/components/page_load_metrics/browser/page_load_metrics_observer.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_OBSERVER_H_
 #define COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_OBSERVER_H_
 
+#include "base/macros.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
 #include "content/public/browser/navigation_handle.h"
 
@@ -13,7 +14,8 @@ namespace page_load_metrics {
 struct PageLoadExtraInfo {
   PageLoadExtraInfo(const base::TimeDelta& first_background_time,
                     const base::TimeDelta& first_foreground_time,
-                    bool started_in_foreground);
+                    bool started_in_foreground,
+                    bool has_commit);
 
   // Returns the time to first background if the page load started in the
   // foreground. If the page has not been backgrounded, or the page started in
@@ -27,22 +29,25 @@ struct PageLoadExtraInfo {
 
   // True if the page load started in the foreground.
   const bool started_in_foreground;
+
+  // True if the page load committed and received its first bytes of data.
+  const bool has_commit;
 };
 
-// Interface for PageLoadMetrics observers. Note that it might be possible for
-// OnCommit to be fired without a subsequent OnComplete (i.e. if an error
-// occurs or we don't have any metrics to log). PageLoadMetricsObservers are
-// required to stop observing (call PageLoadMetricsObservable::RemoveObserver)
-// some time before the OnPageLoadMetricsGoingAway trigger finishes. If this
-// class is destroyed before the PageLoadMetricsObservable, it should call
-// RemoveObserver in its destructor.
+// Interface for PageLoadMetrics observers. All instances of this class are
+// owned by the PageLoadTracker tracking a page load. They will be deleted after
+// calling OnComplete.
 class PageLoadMetricsObserver {
  public:
   virtual ~PageLoadMetricsObserver() {}
 
+  // The page load started, with the given navigation handle.
+  virtual void OnStart(content::NavigationHandle* navigation_handle) {}
+
   // OnRedirect is triggered when a page load redirects to another URL.
   // The navigation handle holds relevant data for the navigation, but will
-  // be destroyed soon after this call. Don't hold a reference to it.
+  // be destroyed soon after this call. Don't hold a reference to it. This can
+  // be called multiple times.
   virtual void OnRedirect(content::NavigationHandle* navigation_handle) {}
 
   // OnCommit is triggered when a page load commits, i.e. when we receive the
@@ -52,26 +57,13 @@ class PageLoadMetricsObserver {
   virtual void OnCommit(content::NavigationHandle* navigation_handle) {}
 
   // OnComplete is triggered when we are ready to record metrics for this page
-  // load. This will happen some time after commit, and will be triggered as
-  // long as we've received some data about the page load. The
-  // PageLoadTiming struct contains timing data and the PageLoadExtraInfo struct
-  // contains other useful information about the tab backgrounding/foregrounding
-  // over the course of the page load.
+  // load. This will happen some time after commit. The PageLoadTiming struct
+  // contains timing data and the PageLoadExtraInfo struct contains other useful
+  // data collected over the course of the page load. If the load did not
+  // receive any timing information, |timing.IsEmpty()| will be true.
+  // After this call, the object will be deleted.
   virtual void OnComplete(const PageLoadTiming& timing,
                           const PageLoadExtraInfo& extra_info) {}
-
-  // This is called when the WebContents we are observing is tearing down. No
-  // further callbacks will be triggered.
-  virtual void OnPageLoadMetricsGoingAway() {}
-};
-
-// Class which handles notifying observers when loads commit and complete. It
-// must call OnPageLoadMetricsGoingAway in the destructor.
-class PageLoadMetricsObservable {
- public:
-  virtual ~PageLoadMetricsObservable() {}
-  virtual void AddObserver(PageLoadMetricsObserver* observer) = 0;
-  virtual void RemoveObserver(PageLoadMetricsObserver* observer) = 0;
 };
 
 }  // namespace page_load_metrics
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 389e013..53e7317 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -66590,7 +66590,8 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 <enum name="InternalErrorLoadEvent" type="int">
   <summary>Internal Errors in the page_load_metrics system</summary>
   <int value="0" label="Invalid timing IPC sent from renderer"/>
-  <int value="1" label="IPC received while not tracking a committed load"/>
+  <int value="1"
+      label="IPC received while not tracking a relevant committed load"/>
   <int value="2" label="IPC received from a frame we navigated away from"/>
   <int value="3" label="IPC received from a bad URL scheme"/>
   <int value="4" label="No IPCs received for this navigation"/>