Merge remote-tracking branch 'refs/remotes/origin/master'

Revert "Make LoadMonitoringExtensionHostQueue remove itself as an ExtensionHost observer at the correct time." BUG=467353 This reverts commit 4eefc7b64335c6008a1e5218c230dadd733875a7.

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

6 files changed, 98 insertions, 173 deletions

diff --git a/extensions/browser/extension_host.cc b/extensions/browser/extension_host.cc
index df92489..bbf51ef 100644
--- a/extensions/browser/extension_host.cc
+++ b/extensions/browser/extension_host.cc
@@ -58,7 +58,7 @@ ExtensionHost::ExtensionHost(const Extension* extension,
       extension_id_(extension->id()),
       browser_context_(site_instance->GetBrowserContext()),
       render_view_host_(nullptr),
-      has_loaded_once_(false),
+      did_stop_loading_(false),
       document_element_available_(false),
       initial_url_(url),
       extension_function_dispatcher_(browser_context_, this),
@@ -92,7 +92,6 @@ ExtensionHost::~ExtensionHost() {
     UMA_HISTOGRAM_LONG_TIMES("Extensions.EventPageActiveTime2",
                              load_start_->Elapsed());
   }
-
   content::NotificationService::current()->Notify(
       extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
       content::Source<BrowserContext>(browser_context_),
@@ -102,16 +101,11 @@ ExtensionHost::~ExtensionHost() {
   FOR_EACH_OBSERVER(DeferredStartRenderHostObserver,
                     deferred_start_render_host_observer_list_,
                     OnDeferredStartRenderHostDestroyed(this));
-
-  // Remove ourselves from the queue as late as possible (before effectively
-  // destroying self, but after everything else) so that queues that are
-  // monitoring lifetime get a chance to see stop-loading events.
   delegate_->GetExtensionHostQueue()->Remove(this);
-
-  // Deliberately stop observing |host_contents_| because its destruction
-  // events (like DidStopLoading, it turns out) can call back into
-  // ExtensionHost re-entrantly, when anything declared after |host_contents_|
-  // has already been destroyed.
+  // Immediately stop observing |host_contents_| because its destruction events
+  // (like DidStopLoading, it turns out) can call back into ExtensionHost
+  // re-entrantly, when anything declared after |host_contents_| has already
+  // been destroyed.
   content::WebContentsObserver::Observe(nullptr);
 }
 
@@ -225,7 +219,7 @@ void ExtensionHost::LoadInitialURL() {
 }
 
 bool ExtensionHost::IsBackgroundPage() const {
-  DCHECK_EQ(extension_host_type_, VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
+  DCHECK(extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
   return true;
 }
 
@@ -274,23 +268,35 @@ void ExtensionHost::DidStartLoading(content::RenderViewHost* render_view_host) {
 }
 
 void ExtensionHost::DidStopLoading(content::RenderViewHost* render_view_host) {
-  // Only record UMA for the first load. Subsequent loads will likely behave
-  // quite different, and it's first load we're most interested in.
-  if (!has_loaded_once_)
-    RecordStopLoadingUMA();
-  has_loaded_once_ = true;
+  bool notify = !did_stop_loading_;
+  did_stop_loading_ = true;
   OnDidStopLoading();
-  content::NotificationService::current()->Notify(
-      extensions::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
-      content::Source<BrowserContext>(browser_context_),
-      content::Details<ExtensionHost>(this));
-  FOR_EACH_OBSERVER(DeferredStartRenderHostObserver,
-                    deferred_start_render_host_observer_list_,
-                    OnDeferredStartRenderHostDidStopLoading(this));
+  if (notify) {
+    CHECK(load_start_.get());
+    if (extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
+      if (extension_ && BackgroundInfo::HasLazyBackgroundPage(extension_)) {
+        UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.EventPageLoadTime2",
+                                   load_start_->Elapsed());
+      } else {
+        UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.BackgroundPageLoadTime2",
+                                   load_start_->Elapsed());
+      }
+    } else if (extension_host_type_ == VIEW_TYPE_EXTENSION_POPUP) {
+      UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.PopupLoadTime2",
+                                 load_start_->Elapsed());
+    }
+    content::NotificationService::current()->Notify(
+        extensions::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
+        content::Source<BrowserContext>(browser_context_),
+        content::Details<ExtensionHost>(this));
+    FOR_EACH_OBSERVER(DeferredStartRenderHostObserver,
+                      deferred_start_render_host_observer_list_,
+                      OnDeferredStartRenderHostDidStopLoading(this));
+  }
 }
 
 void ExtensionHost::OnDidStopLoading() {
-  DCHECK_EQ(extension_host_type_, VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
+  DCHECK(extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
   // Nothing to do for background pages.
 }
 
@@ -455,20 +461,4 @@ bool ExtensionHost::IsNeverVisible(content::WebContents* web_contents) {
   return view_type == extensions::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE;
 }
 
-void ExtensionHost::RecordStopLoadingUMA() {
-  CHECK(load_start_.get());
-  if (extension_host_type_ == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-    if (extension_ && BackgroundInfo::HasLazyBackgroundPage(extension_)) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.EventPageLoadTime2",
-                                 load_start_->Elapsed());
-    } else {
-      UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.BackgroundPageLoadTime2",
-                                 load_start_->Elapsed());
-    }
-  } else if (extension_host_type_ == VIEW_TYPE_EXTENSION_POPUP) {
-    UMA_HISTOGRAM_MEDIUM_TIMES("Extensions.PopupLoadTime2",
-                               load_start_->Elapsed());
-  }
-}
-
 }  // namespace extensions
diff --git a/extensions/browser/extension_host.h b/extensions/browser/extension_host.h
index 5d92335..9926161 100644
--- a/extensions/browser/extension_host.h
+++ b/extensions/browser/extension_host.h
@@ -59,7 +59,7 @@ class ExtensionHost : public DeferredStartRenderHost,
   content::WebContents* host_contents() const { return host_contents_.get(); }
   content::RenderViewHost* render_view_host() const;
   content::RenderProcessHost* render_process_host() const;
-  bool has_loaded_once() const { return has_loaded_once_; }
+  bool did_stop_loading() const { return did_stop_loading_; }
   bool document_element_available() const {
     return document_element_available_;
   }
@@ -131,8 +131,8 @@ class ExtensionHost : public DeferredStartRenderHost,
                            UnloadedExtensionInfo::Reason reason) override;
 
  protected:
-  // Called each time this ExtensionHost completes a load finishes loading,
-  // before any stop-loading notifications or observer methods are called.
+  // Called after the extension page finishes loading but before the
+  // EXTENSION_HOST_DID_STOP_LOADING notification is sent.
   virtual void OnDidStopLoading();
 
   // Navigates to the initial page.
@@ -155,9 +155,6 @@ class ExtensionHost : public DeferredStartRenderHost,
   void OnIncrementLazyKeepaliveCount();
   void OnDecrementLazyKeepaliveCount();
 
-  // Records UMA for load events.
-  void RecordStopLoadingUMA();
-
   // Delegate for functionality that cannot exist in the extensions module.
   scoped_ptr<ExtensionHostDelegate> delegate_;
 
@@ -178,10 +175,8 @@ class ExtensionHost : public DeferredStartRenderHost,
   // host, so we can send messages to it before it finishes loading.
   content::RenderViewHost* render_view_host_;
 
-  // Whether the ExtensionHost has finished loading some content at least once.
-  // There may be subsequent loads - such as reloads and navigations - and this
-  // will not affect its value (it will remain true).
-  bool has_loaded_once_;
+  // Whether the RenderWidget has reported that it has stopped loading.
+  bool did_stop_loading_;
 
   // True if the main frame has finished parsing.
   bool document_element_available_;
diff --git a/extensions/browser/lazy_background_task_queue.cc b/extensions/browser/lazy_background_task_queue.cc
index 6c9b410..754e592 100644
--- a/extensions/browser/lazy_background_task_queue.cc
+++ b/extensions/browser/lazy_background_task_queue.cc
@@ -49,7 +49,7 @@ bool LazyBackgroundTaskQueue::ShouldEnqueueTask(
     ProcessManager* pm = ProcessManager::Get(browser_context);
     ExtensionHost* background_host =
         pm->GetBackgroundHostForExtension(extension->id());
-    if (!background_host || !background_host->has_loaded_once())
+    if (!background_host || !background_host->did_stop_loading())
       return true;
     if (pm->IsBackgroundHostClosing(extension->id()))
       pm->CancelSuspend(extension);
@@ -142,7 +142,7 @@ void LazyBackgroundTaskQueue::Observe(
       ExtensionHost* host =
           content::Details<ExtensionHost>(details).ptr();
       if (host->extension_host_type() == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-        CHECK(host->has_loaded_once());
+        CHECK(host->did_stop_loading());
         ProcessPendingTasks(host, host->browser_context(), host->extension());
       }
       break;
diff --git a/extensions/browser/load_monitoring_extension_host_queue.cc b/extensions/browser/load_monitoring_extension_host_queue.cc
index 3ec5307b..1b8f870 100644
--- a/extensions/browser/load_monitoring_extension_host_queue.cc
+++ b/extensions/browser/load_monitoring_extension_host_queue.cc
@@ -27,7 +27,7 @@ LoadMonitoringExtensionHostQueue::LoadMonitoringExtensionHostQueue(
       started_(false),
       num_queued_(0u),
       num_loaded_(0u),
-      max_awaiting_loading_(0u),
+      max_in_queue_(0u),
       max_active_loading_(0u),
       weak_ptr_factory_(this) {
 }
@@ -56,17 +56,16 @@ void LoadMonitoringExtensionHostQueue::StartMonitoring() {
 void LoadMonitoringExtensionHostQueue::Add(DeferredStartRenderHost* host) {
   StartMonitoring();
   delegate_->Add(host);
-  host->AddDeferredStartRenderHostObserver(this);
-  if (awaiting_loading_.insert(host).second) {
+  if (in_queue_.insert(host).second) {
     ++num_queued_;
-    max_awaiting_loading_ =
-        std::max(max_awaiting_loading_, awaiting_loading_.size());
+    max_in_queue_ = std::max(max_in_queue_, in_queue_.size());
+    host->AddDeferredStartRenderHostObserver(this);
   }
 }
 
 void LoadMonitoringExtensionHostQueue::Remove(DeferredStartRenderHost* host) {
   delegate_->Remove(host);
-  host->RemoveDeferredStartRenderHostObserver(this);
+  RemoveFromQueue(host);
 }
 
 void LoadMonitoringExtensionHostQueue::OnDeferredStartRenderHostDidStartLoading(
@@ -86,10 +85,10 @@ void LoadMonitoringExtensionHostQueue::OnDeferredStartRenderHostDestroyed(
 
 void LoadMonitoringExtensionHostQueue::StartMonitoringHost(
     const DeferredStartRenderHost* host) {
-  awaiting_loading_.erase(host);
   if (active_loading_.insert(host).second) {
     max_active_loading_ = std::max(max_active_loading_, active_loading_.size());
   }
+  RemoveFromQueue(host);
 }
 
 void LoadMonitoringExtensionHostQueue::FinishMonitoringHost(
@@ -99,6 +98,20 @@ void LoadMonitoringExtensionHostQueue::FinishMonitoringHost(
   }
 }
 
+void LoadMonitoringExtensionHostQueue::RemoveFromQueue(
+    const DeferredStartRenderHost* const_host) {
+  // This odd code is needed because StartMonitoringHost() gives us a const
+  // host, but we need a non-const one for
+  // RemoveDeferredStartRenderHostObserver().
+  for (DeferredStartRenderHost* host : in_queue_) {
+    if (host == const_host) {
+      host->RemoveDeferredStartRenderHostObserver(this);
+      in_queue_.erase(host);  // uhoh, iterator invalidated!
+      break;
+    }
+  }
+}
+
 void LoadMonitoringExtensionHostQueue::FinishMonitoring() {
   CHECK(started_);
   UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionHostMonitoring.NumQueued",
@@ -106,12 +119,12 @@ void LoadMonitoringExtensionHostQueue::FinishMonitoring() {
   UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionHostMonitoring.NumLoaded",
                            num_loaded_);
   UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionHostMonitoring.MaxInQueue",
-                           max_awaiting_loading_);
+                           max_in_queue_);
   UMA_HISTOGRAM_COUNTS_100(
       "Extensions.ExtensionHostMonitoring.MaxActiveLoading",
       max_active_loading_);
   if (!finished_callback_.is_null()) {
-    finished_callback_.Run(num_queued_, num_loaded_, max_awaiting_loading_,
+    finished_callback_.Run(num_queued_, num_loaded_, max_in_queue_,
                            max_active_loading_);
   }
 }
diff --git a/extensions/browser/load_monitoring_extension_host_queue.h b/extensions/browser/load_monitoring_extension_host_queue.h
index 6876399..cef3242 100644
--- a/extensions/browser/load_monitoring_extension_host_queue.h
+++ b/extensions/browser/load_monitoring_extension_host_queue.h
@@ -29,7 +29,7 @@ class LoadMonitoringExtensionHostQueue
   // monitoring has finished (timeout has elapsed and UMA is logged).
   using FinishedCallback = base::Callback<void(size_t,  // num_queued
                                                size_t,  // max_loaded
-                                               size_t,  // max_awaiting_loading
+                                               size_t,  // max_in_queue
                                                size_t   // max_active_loading
                                                )>;
   LoadMonitoringExtensionHostQueue(scoped_ptr<ExtensionHostQueue> delegate,
@@ -72,6 +72,9 @@ class LoadMonitoringExtensionHostQueue
   void StartMonitoringHost(const DeferredStartRenderHost* host);
   void FinishMonitoringHost(const DeferredStartRenderHost* host);
 
+  // Removes |host| from the internal |in_queue_| tracking.
+  void RemoveFromQueue(const DeferredStartRenderHost* host);
+
   // Called when monitoring should finish. Metrics are recorded, and from this
   // point on no monitoring will take place.
   void FinishMonitoring();
@@ -86,8 +89,8 @@ class LoadMonitoringExtensionHostQueue
   // A callback to run when monitoring has finished. Intended for testing.
   FinishedCallback finished_callback_;
 
-  // The hosts which are waiting to start loading.
-  std::set<const DeferredStartRenderHost*> awaiting_loading_;
+  // The hosts which are in the delegate's queue.
+  std::set<DeferredStartRenderHost*> in_queue_;
   // The hosts which are currently loading.
   std::set<const DeferredStartRenderHost*> active_loading_;
 
@@ -99,8 +102,8 @@ class LoadMonitoringExtensionHostQueue
   size_t num_queued_;
   // The total number of hosts that started loading.
   size_t num_loaded_;
-  // The maximum number of hosts waiting to load at the same time.
-  size_t max_awaiting_loading_;
+  // The maximum number of hosts in the queue at any time.
+  size_t max_in_queue_;
   // The maximum number of hosts that were loading at the same time.
   size_t max_active_loading_;
 
diff --git a/extensions/browser/load_monitoring_extension_host_queue_unittest.cc b/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
index ef1a1c1..1157ecd 100644
--- a/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
+++ b/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
@@ -16,25 +16,12 @@ namespace extensions {
 namespace {
 
 class StubDeferredStartRenderHost : public DeferredStartRenderHost {
- public:
-  // Returns true if this host is being observed by |observer|.
-  bool IsObservedBy(DeferredStartRenderHostObserver* observer) {
-    return observers_.count(observer) > 0;
-  }
-
  private:
-  // DeferredStartRenderHost:
   void AddDeferredStartRenderHostObserver(
-      DeferredStartRenderHostObserver* observer) override {
-    observers_.insert(observer);
-  }
+      DeferredStartRenderHostObserver* observer) override {}
   void RemoveDeferredStartRenderHostObserver(
-      DeferredStartRenderHostObserver* observer) override {
-    observers_.erase(observer);
-  }
+      DeferredStartRenderHostObserver* observer) override {}
   void CreateRenderViewNow() override {}
-
-  std::set<DeferredStartRenderHostObserver*> observers_;
 };
 
 const size_t g_invalid_size_t = std::numeric_limits<size_t>::max();
@@ -48,7 +35,7 @@ class LoadMonitoringExtensionHostQueueTest : public ExtensionsTest {
         // Arbitrary choice of an invalid size_t.
         num_queued_(g_invalid_size_t),
         num_loaded_(g_invalid_size_t),
-        max_awaiting_loading_(g_invalid_size_t),
+        max_in_queue_(g_invalid_size_t),
         max_active_loading_(g_invalid_size_t) {}
 
   void SetUp() override {
@@ -63,7 +50,7 @@ class LoadMonitoringExtensionHostQueueTest : public ExtensionsTest {
  protected:
   // Creates a new DeferredStartRenderHost. Ownership is held by this class,
   // not passed to caller.
-  StubDeferredStartRenderHost* CreateHost() {
+  DeferredStartRenderHost* CreateHost() {
     StubDeferredStartRenderHost* stub = new StubDeferredStartRenderHost();
     stubs_.push_back(stub);
     return stub;
@@ -73,26 +60,25 @@ class LoadMonitoringExtensionHostQueueTest : public ExtensionsTest {
   LoadMonitoringExtensionHostQueue* queue() { return queue_.get(); }
 
   // Returns true if the queue has finished monitoring.
-  bool finished() const { return finished_; }
+  bool finished() { return finished_; }
 
   // These are available after the queue has finished (in which case finished()
   // will return true).
   size_t num_queued() { return num_queued_; }
   size_t num_loaded() { return num_loaded_; }
-  size_t max_awaiting_loading() { return max_awaiting_loading_; }
+  size_t max_in_queue() { return max_in_queue_; }
   size_t max_active_loading() { return max_active_loading_; }
 
  private:
   // Callback when queue has finished monitoring.
   void Finished(size_t num_queued,
                 size_t num_loaded,
-                size_t max_awaiting_loading,
+                size_t max_in_queue,
                 size_t max_active_loading) {
-    CHECK(!finished_);
     finished_ = true;
     num_queued_ = num_queued;
     num_loaded_ = num_loaded;
-    max_awaiting_loading_ = max_awaiting_loading;
+    max_in_queue_ = max_in_queue;
     max_active_loading_ = max_active_loading;
   }
 
@@ -104,7 +90,7 @@ class LoadMonitoringExtensionHostQueueTest : public ExtensionsTest {
   bool finished_;
   size_t num_queued_;
   size_t num_loaded_;
-  size_t max_awaiting_loading_;
+  size_t max_in_queue_;
   size_t max_active_loading_;
 };
 
@@ -123,7 +109,7 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, NoHosts) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(0u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(0u, max_awaiting_loading());
+  EXPECT_EQ(0u, max_in_queue());
   EXPECT_EQ(0u, max_active_loading());
 }
 
@@ -135,7 +121,7 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(0u, max_active_loading());
 }
 
@@ -150,7 +136,7 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndRemoveOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(0u, max_active_loading());
 }
 
@@ -164,8 +150,12 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndStartOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(1u, max_active_loading());
+
+  // Sanity check: stopping/destroying at this point doesn't crash.
+  queue()->OnDeferredStartRenderHostDidStopLoading(host);
+  queue()->OnDeferredStartRenderHostDestroyed(host);
 }
 
 // Tests adding and destroying a single host without starting it.
@@ -178,8 +168,12 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndDestroyOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(0u, max_active_loading());
+
+  // Sanity check: stopping/destroying at this point doesn't crash.
+  queue()->OnDeferredStartRenderHostDidStopLoading(host);
+  queue()->OnDeferredStartRenderHostDestroyed(host);
 }
 
 // Tests adding, starting, and stopping a single host.
@@ -193,28 +187,11 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndStartAndStopOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(1u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(1u, max_active_loading());
-}
-
-// Tests adding, starting, and stopping a single host - twice.
-TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndStartAndStopOneHostTwice) {
-  DeferredStartRenderHost* host = CreateHost();
-  queue()->Add(host);
-  queue()->OnDeferredStartRenderHostDidStartLoading(host);
-  queue()->OnDeferredStartRenderHostDidStopLoading(host);
 
-  // Re-starting loading should also be recorded fine (e.g. navigations could
-  // trigger this).
-  queue()->OnDeferredStartRenderHostDidStartLoading(host);
-  queue()->OnDeferredStartRenderHostDidStopLoading(host);
-
-  base::RunLoop().RunUntilIdle();
-  ASSERT_TRUE(finished());
-  EXPECT_EQ(1u, num_queued());
-  EXPECT_EQ(2u, num_loaded());  // 2 loaded this time, because we ran twice
-  EXPECT_EQ(1u, max_awaiting_loading());
-  EXPECT_EQ(1u, max_active_loading());
+  // Sanity check: destroying at this point doesn't crash.
+  queue()->OnDeferredStartRenderHostDestroyed(host);
 }
 
 // Tests adding, starting, and destroying (i.e. an implicit stop) a single host.
@@ -228,7 +205,7 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndStartAndDestroyOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(1u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(1u, max_active_loading());
 }
 
@@ -243,7 +220,7 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, AddAndStartAndRemoveOneHost) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
+  EXPECT_EQ(1u, max_in_queue());
   EXPECT_EQ(1u, max_active_loading());
 }
 
@@ -281,10 +258,11 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, Sequence) {
   ASSERT_TRUE(finished());
   EXPECT_EQ(6u, num_queued());
   EXPECT_EQ(2u, num_loaded());
-  EXPECT_EQ(4u, max_awaiting_loading());
+  EXPECT_EQ(4u, max_in_queue());
   EXPECT_EQ(3u, max_active_loading());
 
-  // Complete a realistic sequence by stopping and/or destroying all hosts.
+  // Sanity check: complete a realistic sequence by stopping and/or destroying
+  // all of the hosts. It shouldn't crash.
   queue()->OnDeferredStartRenderHostDestroyed(host1);
   queue()->OnDeferredStartRenderHostDidStopLoading(host2);
   queue()->OnDeferredStartRenderHostDestroyed(host2);
@@ -295,58 +273,4 @@ TEST_F(LoadMonitoringExtensionHostQueueTest, Sequence) {
   queue()->OnDeferredStartRenderHostDestroyed(host6);  // never started/stopped
 }
 
-// Tests that the queue is observing Hosts from adding them through to being
-// removed - that the load sequence itself is irrelevant.
-//
-// This is an unfortunate implementation-style test, but it used to be a bug
-// and difficult to catch outside of a proper test framework - one in which we
-// don't have to trigger events by hand.
-TEST_F(LoadMonitoringExtensionHostQueueTest, ObserverLifetime) {
-  StubDeferredStartRenderHost* host1 = CreateHost();
-  StubDeferredStartRenderHost* host2 = CreateHost();
-  StubDeferredStartRenderHost* host3 = CreateHost();
-  StubDeferredStartRenderHost* host4 = CreateHost();
-
-  EXPECT_FALSE(host1->IsObservedBy(queue()));
-  EXPECT_FALSE(host2->IsObservedBy(queue()));
-  EXPECT_FALSE(host3->IsObservedBy(queue()));
-  EXPECT_FALSE(host4->IsObservedBy(queue()));
-
-  queue()->Add(host1);
-  queue()->Add(host2);
-  queue()->Add(host3);
-  queue()->Add(host4);
-
-  EXPECT_TRUE(host1->IsObservedBy(queue()));
-  EXPECT_TRUE(host2->IsObservedBy(queue()));
-  EXPECT_TRUE(host3->IsObservedBy(queue()));
-  EXPECT_TRUE(host4->IsObservedBy(queue()));
-
-  queue()->OnDeferredStartRenderHostDidStartLoading(host1);
-  queue()->OnDeferredStartRenderHostDidStartLoading(host2);
-  queue()->OnDeferredStartRenderHostDidStartLoading(host3);
-  // host4 will test that we Remove before Starting - so don't start.
-
-  EXPECT_TRUE(host1->IsObservedBy(queue()));
-  EXPECT_TRUE(host2->IsObservedBy(queue()));
-  EXPECT_TRUE(host3->IsObservedBy(queue()));
-  EXPECT_TRUE(host4->IsObservedBy(queue()));
-
-  queue()->OnDeferredStartRenderHostDidStopLoading(host1);
-  queue()->OnDeferredStartRenderHostDestroyed(host2);
-
-  EXPECT_TRUE(host1->IsObservedBy(queue()));
-  EXPECT_TRUE(host2->IsObservedBy(queue()));
-
-  queue()->Remove(host1);
-  queue()->Remove(host2);
-  queue()->Remove(host3);
-  queue()->Remove(host4);
-
-  EXPECT_FALSE(host1->IsObservedBy(queue()));
-  EXPECT_FALSE(host2->IsObservedBy(queue()));
-  EXPECT_FALSE(host3->IsObservedBy(queue()));
-  EXPECT_FALSE(host4->IsObservedBy(queue()));
-}
-
 }  // namespace extensions