Make LoadMonitoringExtensionHostQueue remove itself as an ExtensionHost observer at the correct tim…

extensions/browser/load_monitoring_extension_host_queue.cc

Index: extensions/browser/load_monitoring_extension_host_queue.cc
diff --git a/extensions/browser/load_monitoring_extension_host_queue.cc b/extensions/browser/load_monitoring_extension_host_queue.cc
index 1b8f870524b397ecd5789cd0be00eb2f73ea5b0d..02521d54881986f2d8185817a2c42e74d6f7d23c 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_in_queue_(0u),
+      max_waiting_loading_(0u),
       max_active_loading_(0u),
       weak_ptr_factory_(this) {
 }
@@ -56,16 +56,17 @@ void LoadMonitoringExtensionHostQueue::StartMonitoring() {
 void LoadMonitoringExtensionHostQueue::Add(DeferredStartRenderHost* host) {
   StartMonitoring();
   delegate_->Add(host);
-  if (in_queue_.insert(host).second) {
+  host->AddDeferredStartRenderHostObserver(this);

[not at google - send to devlin, 2015/03/10 22:26:07]

I moved this out of the "if (waiting_loading_.insert(host).second)" block
because it seems irrelevant to whether or not it's being waited for - the
reality is, we unconditionally observe on Add and stop observing on Remove.

[Yoyo Zhou, 2015/03/10 22:48:13]

sounds good.

+  if (waiting_loading_.insert(host).second) {
     ++num_queued_;
-    max_in_queue_ = std::max(max_in_queue_, in_queue_.size());
-    host->AddDeferredStartRenderHostObserver(this);
+    max_waiting_loading_ =
+        std::max(max_waiting_loading_, waiting_loading_.size());
   }
 }
 
 void LoadMonitoringExtensionHostQueue::Remove(DeferredStartRenderHost* host) {
   delegate_->Remove(host);
-  RemoveFromQueue(host);
+  host->RemoveDeferredStartRenderHostObserver(this);
 }
 
 void LoadMonitoringExtensionHostQueue::OnDeferredStartRenderHostDidStartLoading(
@@ -85,10 +86,10 @@ void LoadMonitoringExtensionHostQueue::OnDeferredStartRenderHostDestroyed(
 
 void LoadMonitoringExtensionHostQueue::StartMonitoringHost(
     const DeferredStartRenderHost* host) {
+  waiting_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(
@@ -98,20 +99,6 @@ 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",
@@ -119,12 +106,12 @@ void LoadMonitoringExtensionHostQueue::FinishMonitoring() {
   UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionHostMonitoring.NumLoaded",
                            num_loaded_);
   UMA_HISTOGRAM_COUNTS_100("Extensions.ExtensionHostMonitoring.MaxInQueue",
-                           max_in_queue_);
+                           max_waiting_loading_);
   UMA_HISTOGRAM_COUNTS_100(
       "Extensions.ExtensionHostMonitoring.MaxActiveLoading",
       max_active_loading_);
   if (!finished_callback_.is_null()) {
-    finished_callback_.Run(num_queued_, num_loaded_, max_in_queue_,
+    finished_callback_.Run(num_queued_, num_loaded_, max_waiting_loading_,
                            max_active_loading_);
   }
 }