Revert of Make LoadMonitoringExtensionHostQueue remove itself as an ExtensionHost observer at the...

extensions/browser/load_monitoring_extension_host_queue_unittest.cc

Index: extensions/browser/load_monitoring_extension_host_queue_unittest.cc
diff --git a/extensions/browser/load_monitoring_extension_host_queue_unittest.cc b/extensions/browser/load_monitoring_extension_host_queue_unittest.cc
index ef1a1c1c92e62af480d9f4ebef7ccc9c57ebc64e..1157ecda46825b09a3f0b19c3abd2eea71850201 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 {
 
 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 @@
         // 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 @@
  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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
-  EXPECT_EQ(1u, max_active_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 @@
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(0u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
-  EXPECT_EQ(0u, max_active_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 @@
   ASSERT_TRUE(finished());
   EXPECT_EQ(1u, num_queued());
   EXPECT_EQ(1u, num_loaded());
-  EXPECT_EQ(1u, max_awaiting_loading());
-  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());
+  EXPECT_EQ(1u, max_in_queue());
+  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 @@
   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 @@
   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 @@
   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 @@
   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