Switch RenderFrameHostManager to use RenderProcessHostObserver, remove NOTIFICATION_RENDERER_PROCES…

content/browser/frame_host/render_frame_host_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/render_frame_host_manager.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/trace_event/trace_event.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/debug_urls.h"
 #include "content/browser/frame_host/frame_navigation_entry.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/browser/webui/web_ui_impl.h"
 #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"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/referrer.h"
 #include "content/public/common/url_constants.h"
 
 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();
   return true;
 }
 
 // static
 bool RenderFrameHostManager::IsSwappedOutStateForbidden() {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSitePerProcess);
 }
 
 RenderFrameHostManager::RenderFrameHostManager(
     FrameTreeNode* frame_tree_node,
     RenderFrameHostDelegate* render_frame_delegate,
     RenderViewHostDelegate* render_view_delegate,
     RenderWidgetHostDelegate* render_widget_delegate,
     Delegate* delegate)
     : frame_tree_node_(frame_tree_node),
       delegate_(delegate),
       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) {
   DCHECK(frame_tree_node_);
 }
 
 RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_) {
     scoped_ptr<RenderFrameHostImpl> relic = UnsetPendingRenderFrameHost();
     ShutdownProxiesIfLastActiveFrameInSiteInstance(relic.get());
   }
 
   if (speculative_render_frame_host_) {
     scoped_ptr<RenderFrameHostImpl> relic = UnsetSpeculativeRenderFrameHost();
     ShutdownProxiesIfLastActiveFrameInSiteInstance(relic.get());
   }
 
   ShutdownProxiesIfLastActiveFrameInSiteInstance(render_frame_host_.get());
 
   // Delete any RenderFrameProxyHosts and swapped out RenderFrameHosts.
   // It is important to delete those prior to deleting the current
   // 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.
   web_ui_.reset();
 
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
 }
 
 void RenderFrameHostManager::Init(BrowserContext* browser_context,
@@ skipping 10 matching lines @@
   SetRenderFrameHost(CreateRenderFrameHost(site_instance, view_routing_id,
                                            frame_routing_id, flags));
 
   // Notify the delegate of the creation of the current RenderFrameHost.
   // Do this only for subframes, as the main frame case is taken care of by
   // WebContentsImpl::Init.
   if (!frame_tree_node_->IsMainFrame()) {
     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();
 }
 
 RenderWidgetHostView* RenderFrameHostManager::GetRenderWidgetHostView() const {
   if (interstitial_page_)
     return interstitial_page_->GetView();
   if (render_frame_host_)
     return render_frame_host_->GetView();
   return nullptr;
 }
@@ skipping 15 matching lines @@
   FrameTreeNode* outer_contents_frame_tree_node =
       FrameTreeNode::GloballyFindByID(
           delegate_->GetOuterDelegateFrameTreeNodeID());
   return outer_contents_frame_tree_node->parent()
       ->current_frame_host()
       ->render_view_host();
 }
 
 RenderFrameProxyHost* RenderFrameHostManager::GetProxyToParent() {
   if (frame_tree_node_->IsMainFrame())
-    return NULL;
+    return nullptr;
 
-  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 iter->second;
+  return proxy_hosts_->Get(frame_tree_node_->parent()
+                               ->render_manager()
+                               ->current_frame_host()
+                               ->GetSiteInstance()
+                               ->GetId());
 }
 
 RenderFrameProxyHost* RenderFrameHostManager::GetProxyToOuterDelegate() {
   int outer_contents_frame_tree_node_id =
       delegate_->GetOuterDelegateFrameTreeNodeID();
   FrameTreeNode* outer_contents_frame_tree_node =
       FrameTreeNode::GloballyFindByID(outer_contents_frame_tree_node_id);
   if (!outer_contents_frame_tree_node ||
       !outer_contents_frame_tree_node->parent()) {
     return nullptr;
@@ skipping 43 matching lines @@
   // Create a pending RenderFrameHost to use for the navigation.
   RenderFrameHostImpl* dest_render_frame_host = UpdateStateForNavigate(
       dest_url,
       // TODO(creis): Move source_site_instance to FNE.
       entry.source_site_instance(), frame_entry.site_instance(),
       entry.GetTransitionType(),
       entry.restore_type() != NavigationEntryImpl::RESTORE_NONE,
       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
   // its first page.  (Bug 1145340)
   if (dest_render_frame_host != render_frame_host_ &&
       !render_frame_host_->IsRenderFrameLive()) {
     // Note: we don't call InitRenderView here because we are navigating away
     // soon anyway, and we don't have the NavigationEntry for this host.
     delegate_->CreateRenderViewForRenderManager(
         render_frame_host_->render_view_host(), MSG_ROUTING_NONE,
@@ skipping 11 matching lines @@
     // RFH's ServiceRegistry to be initialized (e.g., if the site is a WebUI
     // site that is handled via Mojo, then Mojo WebUI code in //chrome will
     // add a service to this RFH's ServiceRegistry).
     dest_render_frame_host->SetUpMojoIfNeeded();
 
     // Recreate the opener chain.
     CreateOpenerProxiesIfNeeded(dest_render_frame_host->GetSiteInstance());
     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()
     // on it later.
     if (dest_render_frame_host != render_frame_host_) {
       if (dest_render_frame_host->GetView())
         dest_render_frame_host->GetView()->Hide();
     } else {
       // TODO(nasko): This is a very ugly hack. The Chrome extensions process
       // manager still uses NotificationService and expects to see a
@@ skipping 238 matching lines @@
   } else {
     // No one else should be sending us DidNavigate in this state.
     DCHECK(false);
   }
 }
 
 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)
     render_frame_host_->DisownOpener();
 
   if (pending_render_frame_host_ &&
       pending_render_frame_host_.get() != render_frame_host) {
     pending_render_frame_host_->DisownOpener();
   }
 }
 
 void RenderFrameHostManager::CommitPendingSandboxFlags() {
   // Return early if there were no pending sandbox flags updates.
   if (!frame_tree_node_->CommitPendingSandboxFlags())
     return;
 
   // Sandbox flags updates can only happen when the frame has a parent.
   CHECK(frame_tree_node_->parent());
 
   // Notify all of the frame's proxies about updated sandbox flags, excluding
   // 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(),
           frame_tree_node_->current_replication_state().sandbox_flags));
     }
   }
 }
 
 void RenderFrameHostManager::RendererProcessClosing(
     RenderProcessHost* render_process_host) {
   // Remove any swapped out RVHs from this process, so that we don't try to
   // swap them back in while the process is exiting.  Start by finding them,
   // since there could be more than one.
   std::list<int> ids_to_remove;
   // 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();
   }
 
   while (!ids_to_keep.empty()) {
     frame_tree_node_->frame_tree()->ForEach(
         base::Bind(
             &RenderFrameHostManager::ResetProxiesInSiteInstance,
             ids_to_keep.back()));
     ids_to_keep.pop_back();
   }
@@ skipping 28 matching lines @@
   // RenderFrameHost once it runs its unload handler, without replacing it with
   // a proxy.
   size_t active_frame_count =
       old_render_frame_host->GetSiteInstance()->active_frame_count();
   if (active_frame_count <= 1) {
     // Clear out any proxies from this SiteInstance, in case this was the
     // last one keeping other proxies alive.
     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);
 
   // SwapOut creates a RenderFrameProxy, so set the proxy to be initialized.
   proxy->set_render_frame_proxy_created(true);
 
   if (RenderFrameHostManager::IsSwappedOutStateForbidden()) {
     // In --site-per-process, frames delete their RFH rather than storing it
     // in the proxy.  Schedule it for deletion once the SwapOutACK comes in.
@@ skipping 17 matching lines @@
 
   // If the SiteInstance for the pending RFH is being used by others don't
   // delete the RFH. Just swap it out and it can be reused at a later point.
   // In --site-per-process, RenderFrameHosts are not kept around and are
   // deleted when not used, replaced by RenderFrameProxyHosts.
   SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
   if (site_instance->HasSite() && site_instance->active_frame_count() > 1) {
     // 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.
     if (!render_frame_host->is_swapped_out())
       render_frame_host->SwapOut(proxy, false);
 
     if (!RenderFrameHostManager::IsSwappedOutStateForbidden()) {
       DCHECK(frame_tree_node_->IsMainFrame());
       proxy->TakeFrameHostOwnership(render_frame_host.Pass());
     }
@@ skipping 31 matching lines @@
        iter++) {
     if (*iter == render_frame_host) {
       pending_delete_hosts_.erase(iter);
       return true;
     }
   }
   return false;
 }
 
 void RenderFrameHostManager::ResetProxyHosts() {
-  STLDeleteValues(&proxy_hosts_);
+  proxy_hosts_->Clear();
 }
 
 // PlzNavigate
 void RenderFrameHostManager::DidCreateNavigationRequest(
     const NavigationRequest& request) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
   // Clean up any state in case there's an ongoing navigation.
   // TODO(carlosk): remove this cleanup here once we properly cancel ongoing
   // navigations.
@@ skipping 132 matching lines @@
 // PlzNavigate
 scoped_ptr<RenderFrameHostImpl>
 RenderFrameHostManager::UnsetSpeculativeRenderFrameHost() {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
   speculative_render_frame_host_->GetProcess()->RemovePendingView();
   return speculative_render_frame_host_.Pass();
 }
 
 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()));
   }
 }
 
 void RenderFrameHostManager::OnDidUpdateName(const std::string& name) {
   // The window.name message may be sent outside of --site-per-process when
   // report_frame_name_changes renderer preference is set (used by
   // WebView).  Don't send the update to proxies in those cases.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           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));
   }
 }
 
 void RenderFrameHostManager::OnDidUpdateOrigin(
     const url::DeprecatedSerializedOrigin& origin) {
   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,
     bool related_to_current)
     : existing_site_instance(nullptr),
       new_is_related_to_current(related_to_current) {
   new_site_url = SiteInstance::GetSiteForURL(browser_context, dest_url);
 }
 
 // static
 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.
     if (node->IsMainFrame() &&
         proxy->render_frame_host() &&
         proxy->render_frame_host()->rfh_state() ==
             RenderFrameHostImpl::STATE_PENDING_SWAP_OUT) {
       DCHECK(!RenderFrameHostManager::IsSwappedOutStateForbidden());
       scoped_ptr<RenderFrameHostImpl> swapped_out_rfh =
           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;
 }
 
 // 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;
 }
 
 bool RenderFrameHostManager::ShouldTransitionCrossSite() {
   // True for --site-per-process, which overrides both kSingleProcess and
   // kProcessPerTab.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSitePerProcess))
     return true;
@@ skipping 526 matching lines @@
   if (proxy && proxy->render_frame_host()) {
     CHECK(!swapped_out_forbidden);
     if (view_routing_id_ptr)
       *view_routing_id_ptr = proxy->GetRenderViewHost()->GetRoutingID();
     // Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
     // Prevent the process from exiting while we're trying to use it.
     if (!swapped_out) {
       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.
     new_render_frame_host = CreateRenderFrameHost(instance, MSG_ROUTING_NONE,
                                                   MSG_ROUTING_NONE, flags);
     RenderViewHostImpl* render_view_host =
         new_render_frame_host->render_view_host();
     int proxy_routing_id = MSG_ROUTING_NONE;
 
     // Prevent the process from exiting while we're trying to navigate in it.
     // Otherwise, if the new RFH is swapped out already, store it.
     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());
     }
 
     success = InitRenderView(render_view_host, proxy_routing_id,
                              !!(flags & CREATE_RF_FOR_MAIN_FRAME_NAVIGATION));
     if (success) {
       // Remember that InitRenderView also created the RenderFrameProxy.
       if (swapped_out)
         proxy->set_render_frame_proxy_created(true);
@@ skipping 63 matching lines @@
       render_view_host = frame_tree_node_->frame_tree()->CreateRenderViewHost(
           instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE, true, true);
     }
   }
 
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
   if (proxy && proxy->is_render_frame_proxy_live())
     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() &&
       frame_tree_node_->IsMainFrame()) {
     InitRenderView(render_view_host, proxy->GetRoutingID(), true);
     proxy->set_render_frame_proxy_created(true);
   } else {
     proxy->InitRenderFrameProxy();
   }
 
   return proxy->GetRoutingID();
 }
 
 void RenderFrameHostManager::CreateProxiesForChildFrame(FrameTreeNode* child) {
-  for (const auto& pair : proxy_hosts_) {
+  for (const auto& pair : *proxy_hosts_) {
     child->render_manager()->CreateRenderFrameProxy(
         pair.second->GetSiteInstance());
   }
 }
 
 void RenderFrameHostManager::EnsureRenderViewInitialized(
     RenderViewHostImpl* render_view_host,
     SiteInstance* instance) {
   DCHECK(frame_tree_node_->IsMainFrame());
 
@@ skipping 11 matching lines @@
 
   InitRenderView(render_view_host, proxy->GetRoutingID(), false);
   proxy->set_render_frame_proxy_created(true);
 }
 
 void RenderFrameHostManager::CreateOuterDelegateProxy(
     SiteInstance* outer_contents_site_instance,
     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.
   //
   // We are in the outer WebContents, and its FrameTree would never see
   // a load start for any of its inner WebContents. Eventually, that also makes
   // the FrameTree never see the matching load stop. Therefore, we always pass
   // false to |is_loading| below.
   // TODO(lazyboy): This |is_loading| behavior might not be what we want,
   // investigate and fix.
   render_frame_host->Send(new FrameMsg_SwapOut(
@@ skipping 242 matching lines @@
   // This will swap it out and then put it on the proxy list (if there are other
   // active views in its SiteInstance) or schedule it for deletion when the swap
   // out ack arrives (or immediately if the process isn't live).
   // In the --site-per-process case, old subframe RFHs are not kept alive inside
   // the proxy.
   SwapOutOldFrame(old_render_frame_host.Pass());
 
   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(
           switches::kSitePerProcess)) {
     // If this is a subframe, it should have a CrossProcessFrameConnector
     // created already.  Use it to link the new RFH's view to the proxy that
     // belongs to the parent frame's SiteInstance. If this navigation causes
     // an out-of-process frame to return to the same process as its parent, the
     // proxy would have been removed from proxy_hosts_ above.
     // Note: We do this after swapping out the old RFH because that may create
     // the proxy we're looking for.
     RenderFrameProxyHost* proxy_to_parent = GetProxyToParent();
     if (proxy_to_parent)
       proxy_to_parent->SetChildRWHView(render_frame_host_->GetView());
   }
 
   // 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(
     RenderFrameHostImpl* render_frame_host) {
   if (!render_frame_host)
     return;
   if (!RenderFrameHostImpl::IsRFHStateActive(render_frame_host->rfh_state()))
     return;
   if (render_frame_host->GetSiteInstance()->active_frame_count() > 1U)
     return;
@@ skipping 129 matching lines @@
     return pending_render_frame_host_.get();
   }
 
   // Otherwise the same SiteInstance can be used.  Navigate render_frame_host_.
 
   // It's possible to swap out the current RFH and then decide to navigate in it
   // anyway (e.g., a cross-process navigation that redirects back to the
   // 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();
     pending_and_current_web_ui_ = web_ui_->AsWeakPtr();
   } else {
     SetPendingWebUI(dest_url, bindings);
     // Make sure the new RenderViewHost has the right bindings.
     if (pending_web_ui() &&
         !render_frame_host_->GetProcess()->IsForGuestsOnly()) {
       render_frame_host_->render_view_host()->AllowBindings(
@@ skipping 77 matching lines @@
   if (!proxy->render_frame_host())
     return false;
   return IsOnSwappedOutList(proxy->render_frame_host());
 }
 
 bool RenderFrameHostManager::IsOnSwappedOutList(
     RenderFrameHostImpl* rfh) const {
   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(
    SiteInstance* instance) const {
   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(
     SiteInstance* instance) {
   FrameTreeNode* opener = frame_tree_node_->opener();
   if (!opener)
     return;
 
   // Create proxies for the opener chain.
   opener->render_manager()->CreateOpenerProxies(instance);
@@ skipping 48 matching lines @@
 int RenderFrameHostManager::GetOpenerRoutingID(SiteInstance* instance) {
   if (!frame_tree_node_->opener())
     return MSG_ROUTING_NONE;
 
   return frame_tree_node_->opener()
       ->render_manager()
       ->GetRoutingIdForSiteInstance(instance);
 }
 
 }  // namespace content