Switch RenderFrameHostManager to use RenderProcessHostObserver, remove NOTIFICATION_RENDERER_PROCES…

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 67f66fa0cd45f9a52296e9c1b475264b7baa3746..21335e35409a13d53ea09b58ab7f9e4bc6dfca01 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/frame_host/render_frame_host_manager.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/command_line.h"
@@ -32,8 +33,7 @@
 #include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/user_metrics.h"
@@ -44,6 +44,122 @@
 
 namespace content {
 
+// A helper class to hold all frame proxies and register as a
+// RenderProcessHostObserver for them.
+class RenderFrameHostManager::RenderFrameProxyHostMap
+    : public RenderProcessHostObserver {
+ public:
+  typedef base::hash_map<int32, RenderFrameProxyHost*> MapType;

[ncarter (slow), 2015/07/21 19:07:14]

using MapType = base::hash_map<int32, RenderFrameProxyHost*>;

(assuming the language about aliases on https://chromium-cpp.appspot.com/ is
normative)

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+
+  RenderFrameProxyHostMap(RenderFrameHostManager* manager);
+  ~RenderFrameProxyHostMap() override;
+
+  // Read-only access to the underlying map of site instance ID to
+  // RenderFrameProxyHosts.
+  MapType::const_iterator begin() { return map_.begin(); }

[ncarter (slow), 2015/07/21 19:07:15]

Should these accessors be const, to fit the flavor of them returning
const_iterators?

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+  MapType::const_iterator end() { return map_.end(); }
+  bool empty() { return map_.empty(); }
+
+  // Returns the proxy with the specified ID, or nullptr if there is no such
+  // one.
+  RenderFrameProxyHost* Get(int32 id);
+
+  // Adds the specified proxy with the specified ID. It is an error (and fatal)
+  // to add more than one proxy with the specified ID.
+  void Add(int32 id, scoped_ptr<RenderFrameProxyHost> proxy);
+
+  // Removes the proxy with the specified site instance ID.
+  void Remove(int32 id);
+
+  // Removes all proxies.
+  void Clear();
+
+  // RenderProcessHostObserver implementation.
+  void RenderProcessWillExit(RenderProcessHost* host) override;
+  void RenderProcessExited(RenderProcessHost* host,
+                           base::TerminationStatus status,
+                           int exit_code) override;
+
+ private:
+  RenderFrameHostManager* manager_;
+  MapType map_;
+};
+
+RenderFrameHostManager::RenderFrameProxyHostMap::RenderFrameProxyHostMap(
+    RenderFrameHostManager* manager)
+    : manager_(manager) {}
+
+RenderFrameHostManager::RenderFrameProxyHostMap::~RenderFrameProxyHostMap() {
+  Clear();
+}
+
+RenderFrameProxyHost* RenderFrameHostManager::RenderFrameProxyHostMap::Get(
+    int32 id) {
+  auto it = map_.find(id);
+  if (it != map_.end())
+    return it->second;
+  return nullptr;
+}
+
+void RenderFrameHostManager::RenderFrameProxyHostMap::Add(
+    int32 id,
+    scoped_ptr<RenderFrameProxyHost> proxy) {
+  CHECK_EQ(0u, map_.count(id)) << "Inserting a duplicate item.";
+
+  // If this is the first proxy that has this process host, observe the
+  // process host.
+  RenderProcessHost* host = proxy->GetProcess();
+  size_t count =
+      std::count_if(begin(), end(), [host](MapType::value_type item) {
+        return item.second->GetProcess() == host;
+      });
+  if (count == 0)
+    host->AddObserver(this);
+
+  map_[id] = proxy.release();
+}
+
+void RenderFrameHostManager::RenderFrameProxyHostMap::Remove(int32 id) {
+  auto it = map_.find(id);
+  if (it == map_.end())
+    return;
+
+  // If this is the last proxy that has this process host, stop observing the
+  // process host.
+  RenderProcessHost* host = it->second->GetProcess();
+  size_t count =
+      std::count_if(begin(), end(), [host](MapType::value_type item) {
+        return item.second->GetProcess() == host;
+      });
+  if (count == 1)
+    host->RemoveObserver(this);
+
+  delete it->second;
+  map_.erase(it);
+}
+
+void RenderFrameHostManager::RenderFrameProxyHostMap::Clear() {
+  std::set<RenderProcessHost*> hosts;
+  for (const auto& pair : map_)
+    hosts.insert(pair.second->GetProcess());
+  for (auto host : hosts)
+    host->RemoveObserver(this);
+
+  STLDeleteValues(&map_);
+}
+
+void RenderFrameHostManager::RenderFrameProxyHostMap::RenderProcessWillExit(
+    RenderProcessHost* host) {
+  manager_->RendererProcessClosing(host);
+}
+
+void RenderFrameHostManager::RenderFrameProxyHostMap::RenderProcessExited(
+    RenderProcessHost* host,
+    base::TerminationStatus status,
+    int exit_code) {
+  manager_->RendererProcessClosing(host);
+}
+
 // static
 bool RenderFrameHostManager::ClearRFHsPendingShutdown(FrameTreeNode* node) {
   node->render_manager()->pending_delete_hosts_.clear();
@@ -67,6 +183,7 @@ RenderFrameHostManager::RenderFrameHostManager(
       render_frame_delegate_(render_frame_delegate),
       render_view_delegate_(render_view_delegate),
       render_widget_delegate_(render_widget_delegate),
+      proxy_hosts_(new RenderFrameProxyHostMap(this)),
       interstitial_page_(nullptr),
       should_reuse_web_ui_(false),
       weak_factory_(this) {
@@ -91,7 +208,7 @@ RenderFrameHostManager::~RenderFrameHostManager() {
   // RenderFrameHost, since the CrossProcessFrameConnector (owned by
   // RenderFrameProxyHost) points to the RenderWidgetHostView associated with
   // the current RenderFrameHost and uses it during its destructor.
-  STLDeleteValues(&proxy_hosts_);
+  ResetProxyHosts();
 
   // Release the WebUI prior to resetting the current RenderFrameHost, as the
   // WebUI accesses the RenderFrameHost during cleanup.
@@ -122,24 +239,17 @@ void RenderFrameHostManager::Init(BrowserContext* browser_context,
     delegate_->NotifySwappedFromRenderManager(
         nullptr, render_frame_host_.get(), false);
   }
-
-  // Keep track of renderer processes as they start to shut down or are
-  // crashed/killed.
-  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                 NotificationService::AllSources());
-  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_CLOSING,
-                 NotificationService::AllSources());
 }
 
 RenderViewHostImpl* RenderFrameHostManager::current_host() const {
   if (!render_frame_host_)
-    return NULL;
+    return nullptr;
   return render_frame_host_->render_view_host();
 }
 
 RenderViewHostImpl* RenderFrameHostManager::pending_render_view_host() const {
   if (!pending_render_frame_host_)
-    return NULL;
+    return nullptr;
   return pending_render_frame_host_->render_view_host();
 }
 
@@ -175,18 +285,13 @@ RenderFrameHostManager::GetOuterRenderWidgetHostForKeyboardInput() {
 
 RenderFrameProxyHost* RenderFrameHostManager::GetProxyToParent() {
   if (frame_tree_node_->IsMainFrame())
-    return NULL;
-
-  RenderFrameProxyHostMap::iterator iter =
-      proxy_hosts_.find(frame_tree_node_->parent()
-                            ->render_manager()
-                            ->current_frame_host()
-                            ->GetSiteInstance()
-                            ->GetId());
-  if (iter == proxy_hosts_.end())
-    return NULL;
+    return nullptr;
 
-  return iter->second;
+  return proxy_hosts_->Get(frame_tree_node_->parent()
+                               ->render_manager()
+                               ->current_frame_host()
+                               ->GetSiteInstance()
+                               ->GetId());
 }
 
 RenderFrameProxyHost* RenderFrameHostManager::GetProxyToOuterDelegate() {
@@ -250,7 +355,7 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
       entry.IsViewSourceMode(), entry.transferred_global_request_id(),
       entry.bindings());
   if (!dest_render_frame_host)
-    return NULL;  // We weren't able to create a pending render frame host.
+    return nullptr;  // We weren't able to create a pending render frame host.
 
   // If the current render_frame_host_ isn't live, we should create it so
   // that we don't show a sad tab while the dest_render_frame_host fetches
@@ -282,7 +387,7 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
     if (!InitRenderView(dest_render_frame_host->render_view_host(),
                         MSG_ROUTING_NONE,
                         frame_tree_node_->IsMainFrame()))
-      return NULL;
+      return nullptr;
 
     // Now that we've created a new renderer, be sure to hide it if it isn't
     // our primary one.  Otherwise, we might crash if we try to call Show()
@@ -541,12 +646,10 @@ void RenderFrameHostManager::DidDisownOpener(
     RenderFrameHost* render_frame_host) {
   // Notify all RenderFrameHosts but the one that notified us. This is necessary
   // in case a process swap has occurred while the message was in flight.
-  for (RenderFrameProxyHostMap::iterator iter = proxy_hosts_.begin();
-       iter != proxy_hosts_.end();
-       ++iter) {
-    DCHECK_NE(iter->second->GetSiteInstance(),
+  for (const auto& pair : *proxy_hosts_) {
+    DCHECK_NE(pair.second->GetSiteInstance(),
               current_frame_host()->GetSiteInstance());
-    iter->second->DisownOpener();
+    pair.second->DisownOpener();
   }
 
   if (render_frame_host_.get() != render_frame_host)
@@ -570,7 +673,7 @@ void RenderFrameHostManager::CommitPendingSandboxFlags() {
   // the parent process since it already knows the latest flags.
   SiteInstance* parent_site_instance =
       frame_tree_node_->parent()->current_frame_host()->GetSiteInstance();
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     if (pair.second->GetSiteInstance() != parent_site_instance) {
       pair.second->Send(new FrameMsg_DidUpdateSandboxFlags(
           pair.second->GetRoutingID(),
@@ -588,25 +691,22 @@ void RenderFrameHostManager::RendererProcessClosing(
   // Do not remove proxies in the dead process that still have active frame
   // count though, we just reset them to be uninitialized.
   std::list<int> ids_to_keep;
-  for (RenderFrameProxyHostMap::iterator iter = proxy_hosts_.begin();
-       iter != proxy_hosts_.end();
-       ++iter) {
-    RenderFrameProxyHost* proxy = iter->second;
+  for (const auto& pair : *proxy_hosts_) {

[ncarter (slow), 2015/07/21 19:07:14]

This is cool.

[Avi (use Gerrit), 2015/07/21 20:45:16]

Thanks!

+    RenderFrameProxyHost* proxy = pair.second;
     if (proxy->GetProcess() != render_process_host)
       continue;
 
     if (static_cast<SiteInstanceImpl*>(proxy->GetSiteInstance())
             ->active_frame_count() >= 1U) {
-      ids_to_keep.push_back(iter->first);
+      ids_to_keep.push_back(pair.first);
     } else {
-      ids_to_remove.push_back(iter->first);
+      ids_to_remove.push_back(pair.first);
     }
   }
 
   // Now delete them.
   while (!ids_to_remove.empty()) {
-    delete proxy_hosts_[ids_to_remove.back()];
-    proxy_hosts_.erase(ids_to_remove.back());
+    proxy_hosts_->Remove(ids_to_remove.back());
     ids_to_remove.pop_back();
   }
 
@@ -655,19 +755,18 @@ void RenderFrameHostManager::SwapOutOldFrame(
     ShutdownProxiesIfLastActiveFrameInSiteInstance(old_render_frame_host.get());
 
     // Tell the old RenderFrameHost to swap out, with no proxy to replace it.
-    old_render_frame_host->SwapOut(NULL, true);
+    old_render_frame_host->SwapOut(nullptr, true);
     MoveToPendingDeleteHosts(old_render_frame_host.Pass());
     return;
   }
 
   // Otherwise there are active views and we need a proxy for the old RFH.
   // (There should not be one yet.)
-  CHECK(!GetRenderFrameProxyHost(old_render_frame_host->GetSiteInstance()));
-  RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
+  scoped_ptr<RenderFrameProxyHost> scoped_proxy(new RenderFrameProxyHost(
       old_render_frame_host->GetSiteInstance(),
-      old_render_frame_host->render_view_host(), frame_tree_node_);
-  CHECK(proxy_hosts_.insert(std::make_pair(old_site_instance_id, proxy)).second)
-      << "Inserting a duplicate item.";
+      old_render_frame_host->render_view_host(), frame_tree_node_));
+  RenderFrameProxyHost* proxy = scoped_proxy.get();

[ncarter (slow), 2015/07/21 19:07:14]

You could simplify this a little by eliminating the |scoped_proxy| temporary,
assigning the result of "new RFPH()"  to |proxy|, and passing
make_scoped_ptr(proxy) to Add(). Honestly it's a no-win situation here and I'm
okay with what you have too. But my bias tips very slightly toward avoiding a
nulled-out |scoped_proxy| variable, since we've had exactly that kind of
use-scopedptr-after-Pass() bug in this function before ( my fault:
https://codereview.chromium.org/1074843002/diff/60001/content/browser/frame_h...
).

[Avi (use Gerrit), 2015/07/21 20:45:16]

That's a little cleaner; I'll go with that. I just want to ensure that Add()
takes a scoped_ptr to make its ownership transfer explicit.

+  proxy_hosts_->Add(old_site_instance_id, scoped_proxy.Pass());
 
   // Tell the old RenderFrameHost to swap out and be replaced by the proxy.
   old_render_frame_host->SwapOut(proxy, true);
@@ -705,10 +804,11 @@ void RenderFrameHostManager::DiscardUnusedFrame(
     // Any currently suspended navigations are no longer needed.
     render_frame_host->CancelSuspendedNavigations();
 
-    CHECK(!GetRenderFrameProxyHost(site_instance));
-    RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
-        site_instance, render_frame_host->render_view_host(), frame_tree_node_);
-    proxy_hosts_[site_instance->GetId()] = proxy;
+    scoped_ptr<RenderFrameProxyHost> scoped_proxy(new RenderFrameProxyHost(
+        site_instance, render_frame_host->render_view_host(),
+        frame_tree_node_));
+    RenderFrameProxyHost* proxy = scoped_proxy.get();

[ncarter (slow), 2015/07/21 19:07:14]

Ditto.

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+    proxy_hosts_->Add(site_instance->GetId(), scoped_proxy.Pass());
 
     // Check if the RenderFrameHost is already swapped out, to avoid swapping it
     // out again.
@@ -760,7 +860,7 @@ bool RenderFrameHostManager::DeleteFromPendingList(
 }
 
 void RenderFrameHostManager::ResetProxyHosts() {
-  STLDeleteValues(&proxy_hosts_);
+  proxy_hosts_->Clear();
 }
 
 // PlzNavigate
@@ -913,14 +1013,14 @@ RenderFrameHostManager::UnsetSpeculativeRenderFrameHost() {
 }
 
 void RenderFrameHostManager::OnDidStartLoading() {
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     pair.second->Send(
         new FrameMsg_DidStartLoading(pair.second->GetRoutingID()));
   }
 }
 
 void RenderFrameHostManager::OnDidStopLoading() {
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     pair.second->Send(new FrameMsg_DidStopLoading(pair.second->GetRoutingID()));
   }
 }
@@ -933,7 +1033,7 @@ void RenderFrameHostManager::OnDidUpdateName(const std::string& name) {
           switches::kSitePerProcess))
     return;
 
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     pair.second->Send(
         new FrameMsg_DidUpdateName(pair.second->GetRoutingID(), name));
   }
@@ -944,28 +1044,12 @@ void RenderFrameHostManager::OnDidUpdateOrigin(
   if (!IsSwappedOutStateForbidden())
     return;
 
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     pair.second->Send(
         new FrameMsg_DidUpdateOrigin(pair.second->GetRoutingID(), origin));
   }
 }
 
-void RenderFrameHostManager::Observe(
-    int type,
-    const NotificationSource& source,
-    const NotificationDetails& details) {
-  switch (type) {
-    case NOTIFICATION_RENDERER_PROCESS_CLOSED:
-    case NOTIFICATION_RENDERER_PROCESS_CLOSING:
-      RendererProcessClosing(
-          Source<RenderProcessHost>(source).ptr());
-      break;
-
-    default:
-      NOTREACHED();
-  }
-}
-
 RenderFrameHostManager::SiteInstanceDescriptor::SiteInstanceDescriptor(
     BrowserContext* browser_context,
     GURL dest_url,
@@ -979,10 +1063,9 @@ RenderFrameHostManager::SiteInstanceDescriptor::SiteInstanceDescriptor(
 bool RenderFrameHostManager::ClearProxiesInSiteInstance(
     int32 site_instance_id,
     FrameTreeNode* node) {
-  RenderFrameProxyHostMap::iterator iter =
-      node->render_manager()->proxy_hosts_.find(site_instance_id);
-  if (iter != node->render_manager()->proxy_hosts_.end()) {
-    RenderFrameProxyHost* proxy = iter->second;
+  RenderFrameProxyHost* proxy =
+      node->render_manager()->proxy_hosts_->Get(site_instance_id);
+  if (proxy) {
     // Delete the proxy.  If it is for a main frame (and thus the RFH is stored
     // in the proxy) and it was still pending swap out, move the RFH to the
     // pending deletion list first.
@@ -995,8 +1078,7 @@ bool RenderFrameHostManager::ClearProxiesInSiteInstance(
           proxy->PassFrameHostOwnership();
       node->render_manager()->MoveToPendingDeleteHosts(swapped_out_rfh.Pass());
     }
-    delete proxy;
-    node->render_manager()->proxy_hosts_.erase(site_instance_id);
+    node->render_manager()->proxy_hosts_->Remove(site_instance_id);
   }
 
   return true;
@@ -1005,10 +1087,10 @@ bool RenderFrameHostManager::ClearProxiesInSiteInstance(
 // static.
 bool RenderFrameHostManager::ResetProxiesInSiteInstance(int32 site_instance_id,
                                                         FrameTreeNode* node) {
-  RenderFrameProxyHostMap::iterator iter =
-      node->render_manager()->proxy_hosts_.find(site_instance_id);
-  if (iter != node->render_manager()->proxy_hosts_.end())
-    iter->second->set_render_frame_proxy_created(false);
+  RenderFrameProxyHost* proxy =
+      node->render_manager()->proxy_hosts_->Get(site_instance_id);
+  if (proxy)
+    proxy->set_render_frame_proxy_created(false);
 
   return true;
 }
@@ -1555,8 +1637,7 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
       new_render_frame_host = proxy->PassFrameHostOwnership();
       new_render_frame_host->GetProcess()->AddPendingView();
 
-      proxy_hosts_.erase(instance->GetId());
-      delete proxy;
+      proxy_hosts_->Remove(instance->GetId());

[ncarter (slow), 2015/07/21 19:07:14]

Is it worth adding a "// deletes |proxy|" or "proxy = NULL;" here?

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

     }
   } else {
     // Create a new RenderFrameHost if we don't find an existing one.
@@ -1571,10 +1652,11 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     if (!swapped_out) {
       new_render_frame_host->GetProcess()->AddPendingView();
     } else {
-      proxy = new RenderFrameProxyHost(
+      scoped_ptr<RenderFrameProxyHost> scoped_proxy(new RenderFrameProxyHost(
           new_render_frame_host->GetSiteInstance(),
-          new_render_frame_host->render_view_host(), frame_tree_node_);
-      proxy_hosts_[instance->GetId()] = proxy;
+          new_render_frame_host->render_view_host(), frame_tree_node_));
+      proxy = scoped_proxy.get();

[ncarter (slow), 2015/07/21 19:07:14]

Ditto

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+      proxy_hosts_->Add(instance->GetId(), scoped_proxy.Pass());
       proxy_routing_id = proxy->GetRoutingID();
       proxy->TakeFrameHostOwnership(new_render_frame_host.Pass());
     }
@@ -1658,9 +1740,10 @@ int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
     return proxy->GetRoutingID();
 
   if (!proxy) {
-    proxy = new RenderFrameProxyHost(
-        instance, render_view_host, frame_tree_node_);
-    proxy_hosts_[instance->GetId()] = proxy;
+    scoped_ptr<RenderFrameProxyHost> scoped_proxy(
+        new RenderFrameProxyHost(instance, render_view_host, frame_tree_node_));
+    proxy = scoped_proxy.get();

[ncarter (slow), 2015/07/21 19:07:14]

Ditto

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+    proxy_hosts_->Add(instance->GetId(), scoped_proxy.Pass());
   }
 
   if (RenderFrameHostManager::IsSwappedOutStateForbidden() &&
@@ -1675,7 +1758,7 @@ int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
 }
 
 void RenderFrameHostManager::CreateProxiesForChildFrame(FrameTreeNode* child) {
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     child->render_manager()->CreateRenderFrameProxy(
         pair.second->GetSiteInstance());
   }
@@ -1707,9 +1790,10 @@ void RenderFrameHostManager::CreateOuterDelegateProxy(
     RenderFrameHostImpl* render_frame_host) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSitePerProcess));
-  RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
-      outer_contents_site_instance, nullptr, frame_tree_node_);
-  proxy_hosts_[outer_contents_site_instance->GetId()] = proxy;
+  scoped_ptr<RenderFrameProxyHost> scoped_proxy(new RenderFrameProxyHost(
+      outer_contents_site_instance, nullptr, frame_tree_node_));
+  RenderFrameProxyHost* proxy = scoped_proxy.get();

[ncarter (slow), 2015/07/21 19:07:14]

Ditto

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+  proxy_hosts_->Add(outer_contents_site_instance->GetId(), scoped_proxy.Pass());
 
   // Swap the outer WebContents's frame with the proxy to inner WebContents.
   //
@@ -1972,12 +2056,7 @@ void RenderFrameHostManager::CommitPending() {
   if (RenderFrameHostManager::IsSwappedOutStateForbidden()) {
     // Since the new RenderFrameHost is now committed, there must be no proxies
     // for its SiteInstance. Delete any existing ones.
-    RenderFrameProxyHostMap::iterator iter =
-        proxy_hosts_.find(render_frame_host_->GetSiteInstance()->GetId());
-    if (iter != proxy_hosts_.end()) {
-      delete iter->second;
-      proxy_hosts_.erase(iter);
-    }
+    proxy_hosts_->Remove(render_frame_host_->GetSiteInstance()->GetId());
   }
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -1996,8 +2075,8 @@ void RenderFrameHostManager::CommitPending() {
 
   // After all is done, there must never be a proxy in the list which has the
   // same SiteInstance as the current RenderFrameHost.
-  CHECK(proxy_hosts_.find(render_frame_host_->GetSiteInstance()->GetId()) ==
-        proxy_hosts_.end());
+  CHECK(proxy_hosts_->Get(render_frame_host_->GetSiteInstance()->GetId()) ==

[ncarter (slow), 2015/07/21 19:07:14]

CHECK(!proxy_hosts->Get(...)) ?

[Avi (use Gerrit), 2015/07/21 20:45:16]

Done.

+        nullptr);
 }
 
 void RenderFrameHostManager::ShutdownProxiesIfLastActiveFrameInSiteInstance(
@@ -2147,7 +2226,7 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
   // original site).  In that case, we have a proxy for the current RFH but
   // haven't deleted it yet.  The new navigation will swap it back in, so we can
   // delete the proxy.
-  DeleteRenderFrameProxyHost(new_instance.get());
+  proxy_hosts_->Remove(new_instance.get()->GetId());
 
   if (ShouldReuseWebUI(current_entry, dest_url)) {
     pending_web_ui_.reset();
@@ -2245,12 +2324,12 @@ bool RenderFrameHostManager::IsOnSwappedOutList(
   if (!rfh->GetSiteInstance())
     return false;
 
-  RenderFrameProxyHostMap::const_iterator iter = proxy_hosts_.find(
-      rfh->GetSiteInstance()->GetId());
-  if (iter == proxy_hosts_.end())
+  RenderFrameProxyHost* host =
+      proxy_hosts_->Get(rfh->GetSiteInstance()->GetId());
+  if (!host)
     return false;
 
-  return iter->second->render_frame_host() == rfh;
+  return host->render_frame_host() == rfh;
 }
 
 RenderViewHostImpl* RenderFrameHostManager::GetSwappedOutRenderViewHost(
@@ -2258,26 +2337,19 @@ RenderViewHostImpl* RenderFrameHostManager::GetSwappedOutRenderViewHost(
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
   if (proxy)
     return proxy->GetRenderViewHost();
-  return NULL;
+  return nullptr;
 }
 
 RenderFrameProxyHost* RenderFrameHostManager::GetRenderFrameProxyHost(
     SiteInstance* instance) const {
-  RenderFrameProxyHostMap::const_iterator iter =
-      proxy_hosts_.find(instance->GetId());
-  if (iter != proxy_hosts_.end())
-    return iter->second;
-
-  return NULL;
+  return proxy_hosts_->Get(instance->GetId());
 }
 
-void RenderFrameHostManager::DeleteRenderFrameProxyHost(
-    SiteInstance* instance) {
-  RenderFrameProxyHostMap::iterator iter = proxy_hosts_.find(instance->GetId());
-  if (iter != proxy_hosts_.end()) {
-    delete iter->second;
-    proxy_hosts_.erase(iter);
-  }
+std::map<int, RenderFrameProxyHost*>
+RenderFrameHostManager::GetAllProxyHostsForTesting() {
+  std::map<int, RenderFrameProxyHost*> result;
+  result.insert(proxy_hosts_->begin(), proxy_hosts_->end());
+  return result;
 }
 
 void RenderFrameHostManager::CreateOpenerProxiesIfNeeded(