content: Introduce a RenderWidgetHostIterator.

RenderWidgetHost::GetRenderWidgetHosts() and GetAllRenderWidgetHosts() returns a
list of RenderWidgetHost's. The caller can then perform some operation that leads
to some of the RenderWidgetHosts in the list being destroyed, leading to reading
from freed memory, when iterating over the list. So instead of sending a list,
introduce RenderWidgetHostIterator, which allows safely iterating over the list
of hosts.

BUG=285307
R=ajwong@chromium.org, creis@chromium.org, jam@chromium.org

Review URL: https://codereview.chromium.org/23453038

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222591 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 193 insertions, 126 deletions

diff --git a/chrome/browser/chromeos/system/timezone_settings.cc b/chrome/browser/chromeos/system/timezone_settings.cc
index eab34e9..5f9b4d8 100644
--- a/chrome/browser/chromeos/system/timezone_settings.cc
+++ b/chrome/browser/chromeos/system/timezone_settings.cc
@@ -21,6 +21,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "third_party/icu/source/i18n/unicode/timezone.h"
 
 using content::BrowserThread;
@@ -352,11 +353,11 @@ const icu::TimeZone* TimezoneSettingsBaseImpl::GetKnownTimezoneOrNull(
 }
 
 void TimezoneSettingsBaseImpl::NotifyRenderers() {
-  content::RenderWidgetHost::List widgets =
-      content::RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (widgets[i]->IsRenderView()) {
-      content::RenderViewHost* view = content::RenderViewHost::From(widgets[i]);
+  scoped_ptr<content::RenderWidgetHostIterator> widgets(
+      content::RenderWidgetHost::GetRenderWidgetHosts());
+  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (widget->IsRenderView()) {
+      content::RenderViewHost* view = content::RenderViewHost::From(widget);
       view->NotifyTimezoneChange();
     }
   }
diff --git a/chrome/browser/extensions/api/processes/processes_api.cc b/chrome/browser/extensions/api/processes/processes_api.cc
index b29f3c6..c92b2db 100644
--- a/chrome/browser/extensions/api/processes/processes_api.cc
+++ b/chrome/browser/extensions/api/processes/processes_api.cc
@@ -31,6 +31,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/result_codes.h"
 #include "extensions/common/error_utils.h"
@@ -111,15 +112,15 @@ base::ListValue* GetTabsForProcess(int process_id) {
   int tab_id = -1;
   // We need to loop through all the RVHs to ensure we collect the set of all
   // tabs using this renderer process.
-  content::RenderWidgetHost::List widgets =
-      content::RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (widgets[i]->GetProcess()->GetID() != process_id)
+  scoped_ptr<content::RenderWidgetHostIterator> widgets(
+      content::RenderWidgetHost::GetRenderWidgetHosts());
+  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (widget->GetProcess()->GetID() != process_id)
       continue;
-    if (!widgets[i]->IsRenderView())
+    if (!widget->IsRenderView())
       continue;
 
-    content::RenderViewHost* host = content::RenderViewHost::From(widgets[i]);
+    content::RenderViewHost* host = content::RenderViewHost::From(widget);
     content::WebContents* contents =
         content::WebContents::FromRenderViewHost(host);
     if (contents) {
diff --git a/chrome/browser/memory_details.cc b/chrome/browser/memory_details.cc
index 643e211..3e24154 100644
--- a/chrome/browser/memory_details.cc
+++ b/chrome/browser/memory_details.cc
@@ -23,6 +23,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/bindings_policy.h"
 #include "extensions/browser/view_type_utils.h"
@@ -218,10 +219,11 @@ void MemoryDetails::CollectChildInfoOnUIThread() {
     ProcessMemoryInformation& process =
         chrome_browser->processes[index];
 
-    RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-    for (size_t i = 0; i < widgets.size(); ++i) {
+    scoped_ptr<content::RenderWidgetHostIterator> widgets(
+        RenderWidgetHost::GetRenderWidgetHosts());
+    while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
       content::RenderProcessHost* render_process_host =
-          widgets[i]->GetProcess();
+          widget->GetProcess();
       DCHECK(render_process_host);
       // Ignore processes that don't have a connection, such as crashed tabs.
       if (!render_process_host->HasConnection() ||
@@ -241,10 +243,10 @@ void MemoryDetails::CollectChildInfoOnUIThread() {
       // The RenderProcessHost may host multiple WebContentses.  Any
       // of them which contain diagnostics information make the whole
       // process be considered a diagnostics process.
-      if (!widgets[i]->IsRenderView())
+      if (!widget->IsRenderView())
         continue;
 
-      RenderViewHost* host = RenderViewHost::From(widgets[i]);
+      RenderViewHost* host = RenderViewHost::From(widget);
       WebContents* contents = WebContents::FromRenderViewHost(host);
       GURL url;
       if (contents) {
diff --git a/chrome/browser/performance_monitor/performance_monitor.cc b/chrome/browser/performance_monitor/performance_monitor.cc
index 5ed870f..b800e7f 100644
--- a/chrome/browser/performance_monitor/performance_monitor.cc
+++ b/chrome/browser/performance_monitor/performance_monitor.cc
@@ -37,6 +37,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "net/url_request/url_request.h"
 
@@ -622,15 +623,15 @@ void PerformanceMonitor::AddRendererClosedEvent(
   // render view, extract the url, and append it to the string, comma-separating
   // the entries.
   std::string url_list;
-  content::RenderWidgetHost::List widgets =
-      content::RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (widgets[i]->GetProcess()->GetID() != host->GetID())
+  scoped_ptr<content::RenderWidgetHostIterator> widgets(
+      content::RenderWidgetHost::GetRenderWidgetHosts());
+  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (widget->GetProcess()->GetID() != host->GetID())
       continue;
-    if (!widgets[i]->IsRenderView())
+    if (!widget->IsRenderView())
       continue;
 
-    content::RenderViewHost* view = content::RenderViewHost::From(widgets[i]);
+    content::RenderViewHost* view = content::RenderViewHost::From(widget);
     std::string url;
     if (!MaybeGetURLFromRenderView(view, &url))
       continue;
diff --git a/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc b/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
index d60e74e..47edba3 100644
--- a/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
+++ b/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
@@ -18,6 +18,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 
@@ -40,13 +41,14 @@ int RenderProcessHostCount() {
 }
 
 RenderViewHost* FindFirstDevToolsHost() {
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (!widgets[i]->GetProcess()->HasConnection())
+  scoped_ptr<content::RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (!widget->GetProcess()->HasConnection())
       continue;
-    if (!widgets[i]->IsRenderView())
+    if (!widget->IsRenderView())
       continue;
-    RenderViewHost* host = RenderViewHost::From(widgets[i]);
+    RenderViewHost* host = RenderViewHost::From(widget);
     WebContents* contents = WebContents::FromRenderViewHost(host);
     GURL url = contents->GetURL();
     if (url.SchemeIs(chrome::kChromeDevToolsScheme))
diff --git a/chrome/browser/task_manager/guest_resource_provider.cc b/chrome/browser/task_manager/guest_resource_provider.cc
index b8bda92..f38f265 100644
--- a/chrome/browser/task_manager/guest_resource_provider.cc
+++ b/chrome/browser/task_manager/guest_resource_provider.cc
@@ -15,6 +15,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
@@ -132,10 +133,11 @@ void GuestResourceProvider::StartUpdating() {
   updating_ = true;
 
   // Add all the existing guest WebContents.
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (widgets[i]->IsRenderView()) {
-      RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
+  scoped_ptr<content::RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (widget->IsRenderView()) {
+      RenderViewHost* rvh = RenderViewHost::From(widget);
       if (rvh->IsSubframe())
         Add(rvh);
     }
diff --git a/content/browser/accessibility/accessibility_ui.cc b/content/browser/accessibility/accessibility_ui.cc
index 585be68..d5a9734 100644
--- a/content/browser/accessibility/accessibility_ui.cc
+++ b/content/browser/accessibility/accessibility_ui.cc
@@ -22,6 +22,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/common/url_constants.h"
@@ -91,15 +92,16 @@ void SendTargetsData(
     const WebUIDataSource::GotDataCallback& callback) {
   scoped_ptr<base::ListValue> rvh_list(new base::ListValue());
 
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
     // Ignore processes that don't have a connection, such as crashed tabs.
-    if (!widgets[i]->GetProcess()->HasConnection())
+    if (!widget->GetProcess()->HasConnection())
       continue;
-    if (!widgets[i]->IsRenderView())
+    if (!widget->IsRenderView())
         continue;
 
-    RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
+    RenderViewHost* rvh = RenderViewHost::From(widget);
     rvh_list->Append(BuildTargetDescriptor(rvh));
   }
 
diff --git a/content/browser/accessibility/browser_accessibility_state_impl.cc b/content/browser/accessibility/browser_accessibility_state_impl.cc
index befd717..3dee7d6 100644
--- a/content/browser/accessibility/browser_accessibility_state_impl.cc
+++ b/content/browser/accessibility/browser_accessibility_state_impl.cc
@@ -10,6 +10,7 @@
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/common/content_switches.h"
 #include "ui/gfx/sys_color_change_listener.h"
 
@@ -131,16 +132,16 @@ void BrowserAccessibilityStateImpl::SetAccessibilityMode(
 
   // Iterate over all RenderWidgetHosts, even swapped out ones in case
   // they become active again.
-  RenderWidgetHost::List widgets =
-      RenderWidgetHostImpl::GetAllRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHostImpl::GetAllRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
     // Ignore processes that don't have a connection, such as crashed tabs.
-    if (!widgets[i]->GetProcess()->HasConnection())
+    if (!widget->GetProcess()->HasConnection())
       continue;
-    if (!widgets[i]->IsRenderView())
+    if (!widget->IsRenderView())
       continue;
 
-    RenderWidgetHostImpl::From(widgets[i])->SetAccessibilityMode(mode);
+    RenderWidgetHostImpl::From(widget)->SetAccessibilityMode(mode);
   }
 }
 
diff --git a/content/browser/devtools/render_view_devtools_agent_host.cc b/content/browser/devtools/render_view_devtools_agent_host.cc
index 461cc36..25b9229 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.cc
+++ b/content/browser/devtools/render_view_devtools_agent_host.cc
@@ -21,6 +21,7 @@
 #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_widget_host_iterator.h"
 
 namespace content {
 
@@ -101,15 +102,16 @@ bool DevToolsAgentHost::IsDebuggerAttached(WebContents* web_contents) {
 //static
 std::vector<RenderViewHost*> DevToolsAgentHost::GetValidRenderViewHosts() {
   std::vector<RenderViewHost*> result;
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
     // Ignore processes that don't have a connection, such as crashed contents.
-    if (!widgets[i]->GetProcess()->HasConnection())
+    if (!widget->GetProcess()->HasConnection())
       continue;
-    if (!widgets[i]->IsRenderView())
+    if (!widget->IsRenderView())
       continue;
 
-    RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
+    RenderViewHost* rvh = RenderViewHost::From(widget);
     WebContents* web_contents = WebContents::FromRenderViewHost(rvh);
     // Don't report a RenderViewHost if it is not the current RenderViewHost
     // for some WebContents.
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index c3cd0fe..4981d60 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -112,6 +112,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host_factory.h"
 #include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_constants.h"
@@ -1637,10 +1638,11 @@ void RenderProcessHostImpl::ProcessDied(bool already_dead) {
 
 int RenderProcessHostImpl::GetActiveViewCount() {
   int num_active_views = 0;
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
     // Count only RenderWidgetHosts in this process.
-    if (widgets[i]->GetProcess()->GetID() == GetID())
+    if (widget->GetProcess()->GetID() == GetID())
       num_active_views++;
   }
   return num_active_views;
@@ -1773,17 +1775,17 @@ void RenderProcessHostImpl::OnCompositorSurfaceBuffersSwappedNoHost(
 
 void RenderProcessHostImpl::OnGpuSwitching() {
   // We are updating all widgets including swapped out ones.
-  RenderWidgetHost::List widgets =
-      RenderWidgetHostImpl::GetAllRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (!widgets[i]->IsRenderView())
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHostImpl::GetAllRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (!widget->IsRenderView())
       continue;
 
     // Skip widgets in other processes.
-    if (widgets[i]->GetProcess()->GetID() != GetID())
+    if (widget->GetProcess()->GetID() != GetID())
       continue;
 
-    RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
+    RenderViewHost* rvh = RenderViewHost::From(widget);
     rvh->UpdateWebkitPreferences(rvh->GetWebkitPreferences());
   }
 }
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 0d8f0e0..dfd9e10 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -51,6 +51,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host_observer.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_constants.h"
@@ -483,9 +484,10 @@ void RenderViewHostImpl::WasSwappedOut() {
 
     // Count the number of active widget hosts for the process, which
     // is equivalent to views using the process as of this writing.
-    RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-    for (size_t i = 0; i < widgets.size(); ++i) {
-      if (widgets[i]->GetProcess()->GetID() == GetProcess()->GetID())
+    scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+    while (RenderWidgetHost* widget = widgets->GetNextHost()) {
+      if (widget->GetProcess()->GetID() == GetProcess()->GetID())
         ++views;
     }
 
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index a3c9a01..ec3387c 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -44,6 +44,7 @@
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
@@ -91,15 +92,50 @@ const int kPaintMsgTimeoutMS = 50;
 base::LazyInstance<std::vector<RenderWidgetHost::CreatedCallback> >
 g_created_callbacks = LAZY_INSTANCE_INITIALIZER;
 
-}  // namespace
-
-
 typedef std::pair<int32, int32> RenderWidgetHostID;
 typedef base::hash_map<RenderWidgetHostID, RenderWidgetHostImpl*>
     RoutingIDWidgetMap;
-static base::LazyInstance<RoutingIDWidgetMap> g_routing_id_widget_map =
+base::LazyInstance<RoutingIDWidgetMap> g_routing_id_widget_map =
     LAZY_INSTANCE_INITIALIZER;
 
+// Implements the RenderWidgetHostIterator interface. It keeps a list of
+// RenderWidgetHosts, and makes sure it returns a live RenderWidgetHost at each
+// iteration (or NULL if there isn't any left).
+class RenderWidgetHostIteratorImpl : public RenderWidgetHostIterator {
+ public:
+  RenderWidgetHostIteratorImpl()
+      : current_index_(0) {
+  }
+
+  virtual ~RenderWidgetHostIteratorImpl() {
+  }
+
+  void Add(RenderWidgetHost* host) {
+    hosts_.push_back(RenderWidgetHostID(host->GetProcess()->GetID(),
+                                        host->GetRoutingID()));
+  }
+
+  // RenderWidgetHostIterator:
+  virtual RenderWidgetHost* GetNextHost() OVERRIDE {
+    RenderWidgetHost* host = NULL;
+    while (current_index_ < hosts_.size() && !host) {
+      RenderWidgetHostID id = hosts_[current_index_];
+      host = RenderWidgetHost::FromID(id.first, id.second);
+      ++current_index_;
+    }
+    return host;
+  }
+
+ private:
+  std::vector<RenderWidgetHostID> hosts_;
+  size_t current_index_;
+
+  DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostIteratorImpl);
+};
+
+}  // namespace
+
+
 // static
 void RenderWidgetHost::RemoveAllBackingStores() {
   BackingStoreManager::RemoveAllBackingStores();
@@ -234,8 +270,8 @@ RenderWidgetHostImpl* RenderWidgetHostImpl::FromID(
 }
 
 // static
-std::vector<RenderWidgetHost*> RenderWidgetHost::GetRenderWidgetHosts() {
-  std::vector<RenderWidgetHost*> hosts;
+scoped_ptr<RenderWidgetHostIterator> RenderWidgetHost::GetRenderWidgetHosts() {
+  RenderWidgetHostIteratorImpl* hosts = new RenderWidgetHostIteratorImpl();
   RoutingIDWidgetMap* widgets = g_routing_id_widget_map.Pointer();
   for (RoutingIDWidgetMap::const_iterator it = widgets->begin();
        it != widgets->end();
@@ -243,28 +279,31 @@ std::vector<RenderWidgetHost*> RenderWidgetHost::GetRenderWidgetHosts() {
     RenderWidgetHost* widget = it->second;
 
     if (!widget->IsRenderView()) {
-      hosts.push_back(widget);
+      hosts->Add(widget);
       continue;
     }
 
     // Add only active RenderViewHosts.
     RenderViewHost* rvh = RenderViewHost::From(widget);
     if (!static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out())
-      hosts.push_back(widget);
+      hosts->Add(widget);
   }
-  return hosts;
+
+  return scoped_ptr<RenderWidgetHostIterator>(hosts);
 }
 
 // static
-std::vector<RenderWidgetHost*> RenderWidgetHostImpl::GetAllRenderWidgetHosts() {
-  std::vector<RenderWidgetHost*> hosts;
+scoped_ptr<RenderWidgetHostIterator>
+RenderWidgetHostImpl::GetAllRenderWidgetHosts() {
+  RenderWidgetHostIteratorImpl* hosts = new RenderWidgetHostIteratorImpl();
   RoutingIDWidgetMap* widgets = g_routing_id_widget_map.Pointer();
   for (RoutingIDWidgetMap::const_iterator it = widgets->begin();
        it != widgets->end();
        ++it) {
-    hosts.push_back(it->second);
+    hosts->Add(it->second);
   }
-  return hosts;
+
+  return scoped_ptr<RenderWidgetHostIterator>(hosts);
 }
 
 // static
diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h
index ba7fca8..5c9c2d6 100644
--- a/content/browser/renderer_host/render_widget_host_impl.h
+++ b/content/browser/renderer_host/render_widget_host_impl.h
@@ -106,14 +106,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl : virtual public RenderWidgetHost,
   // Returns all RenderWidgetHosts including swapped out ones for
   // internal use. The public interface
   // RendgerWidgetHost::GetRenderWidgetHosts only returns active ones.
-  // Keep in mind that there may be dependencies between these
-  // widgets.  If a caller indirectly causes one of the widgets to be
-  // deleted while iterating over the list, the deleted widget will
-  // stay in the list and possibly causes a use-after-free.  Take care
-  // to avoid deleting widgets as you iterate (e.g., see
-  // http://crbug.com/259859).  TODO(nasko): Improve this interface to
-  // better prevent UaFs.
-  static std::vector<RenderWidgetHost*> GetAllRenderWidgetHosts();
+  static scoped_ptr<RenderWidgetHostIterator> GetAllRenderWidgetHosts();
 
   // Use RenderWidgetHostImpl::From(rwh) to downcast a
   // RenderWidgetHost to a RenderWidgetHostImpl.  Internally, this
diff --git a/content/browser/web_contents/render_view_host_manager.cc b/content/browser/web_contents/render_view_host_manager.cc
index 2f62591..0081116 100644
--- a/content/browser/web_contents/render_view_host_manager.cc
+++ b/content/browser/web_contents/render_view_host_manager.cc
@@ -24,6 +24,7 @@
 #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_widget_host_iterator.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents_view.h"
 #include "content/public/browser/web_ui_controller.h"
@@ -823,33 +824,16 @@ void RenderViewHostManager::ShutdownRenderViewHostsInSiteInstance(
   // list.
   swapped_out_hosts_.erase(site_instance_id);
 
-  RenderWidgetHost::List widgets =
-      RenderWidgetHostImpl::GetAllRenderWidgetHosts();
-
-  // Here deleting a RWH in widgets can possibly cause another RWH in
-  // the list to be deleted.  This can result in leaving a dangling
-  // pointer in the widgets list. Our assumption is that a widget
-  // deleted as that sort of side-effect should not be directly
-  // deleted here. Therefore, we first gather only widgets directly to
-  // be deleted so that we don't hit any future dangling pointers in
-  // widgets.
-  std::vector<RenderViewHostImpl*> rvhs_to_be_deleted;
-
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (!widgets[i]->IsRenderView())
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHostImpl::GetAllRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (!widget->IsRenderView())
       continue;
     RenderViewHostImpl* rvh =
-        static_cast<RenderViewHostImpl*>(RenderViewHost::From(widgets[i]));
-    if (site_instance_id == rvh->GetSiteInstance()->GetId()) {
-      DCHECK(rvh->is_swapped_out());
-      rvhs_to_be_deleted.push_back(rvh);
-    }
+        static_cast<RenderViewHostImpl*>(RenderViewHost::From(widget));
+    if (site_instance_id == rvh->GetSiteInstance()->GetId())
+      rvh->Shutdown();
   }
-
-  // Finally we delete the gathered RVHs, which should not indirectly
-  // delete each other.
-  for (size_t i = 0; i < rvhs_to_be_deleted.size(); ++i)
-    rvhs_to_be_deleted[i]->Shutdown();
 }
 
 RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
diff --git a/content/browser/web_contents/render_view_host_manager_unittest.cc b/content/browser/web_contents/render_view_host_manager_unittest.cc
index 77d6533..7d435fa 100644
--- a/content/browser/web_contents/render_view_host_manager_unittest.cc
+++ b/content/browser/web_contents/render_view_host_manager_unittest.cc
@@ -16,6 +16,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host_observer.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/javascript_message_type.h"
@@ -345,12 +346,14 @@ TEST_F(RenderViewHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
   TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
   EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
 
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
   // We know that there is the only one active widget. Another view is
   // now swapped out, so the swapped out view is not included in the
   // list.
-  EXPECT_TRUE(widgets.size() == 1);
-  RenderViewHost* rvh = RenderViewHost::From(widgets[0]);
+  RenderWidgetHost* widget = widgets->GetNextHost();
+  EXPECT_FALSE(widgets->GetNextHost());
+  RenderViewHost* rvh = RenderViewHost::From(widget);
   EXPECT_FALSE(static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out());
 }
 
@@ -364,14 +367,15 @@ TEST_F(RenderViewHostManagerTest,
   TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
   EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
 
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  RenderWidgetHost::List all_widgets =
-      RenderWidgetHostImpl::GetAllRenderWidgetHosts();
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
 
-  for (size_t i = 0; i < widgets.size(); ++i) {
+  while (RenderWidgetHost* w = widgets->GetNextHost()) {
     bool found = false;
-    for (size_t j = 0; j < all_widgets.size(); ++j) {
-      if (widgets[i] == all_widgets[j]) {
+    scoped_ptr<RenderWidgetHostIterator> all_widgets(
+        RenderWidgetHostImpl::GetAllRenderWidgetHosts());
+    while (RenderWidgetHost* widget = all_widgets->GetNextHost()) {
+      if (w == widget) {
         found = true;
         break;
       }
diff --git a/content/browser/worker_host/worker_service_impl.cc b/content/browser/worker_host/worker_service_impl.cc
index 5fb8e62..d2ab272 100644
--- a/content/browser/worker_host/worker_service_impl.cc
+++ b/content/browser/worker_host/worker_service_impl.cc
@@ -21,6 +21,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/web_contents.h"
@@ -110,16 +111,17 @@ void WorkerPrioritySetter::GatherVisibleIDsAndUpdateWorkerPriorities() {
       new std::set<std::pair<int, int> >();
 
   // Gather up all the visible renderer process/view pairs
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
-    if (widgets[i]->GetProcess()->VisibleWidgetCount() == 0)
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
+    if (widget->GetProcess()->VisibleWidgetCount() == 0)
       continue;
 
-    RenderWidgetHostView* render_view = widgets[i]->GetView();
+    RenderWidgetHostView* render_view = widget->GetView();
     if (render_view && render_view->IsShowing()) {
       visible_renderer_ids->insert(
-          std::pair<int, int>(widgets[i]->GetProcess()->GetID(),
-                              widgets[i]->GetRoutingID()));
+          std::pair<int, int>(widget->GetProcess()->GetID(),
+                              widget->GetRoutingID()));
     }
   }
 
diff --git a/content/public/browser/render_widget_host.h b/content/public/browser/render_widget_host.h
index a700266..e54403c 100644
--- a/content/public/browser/render_widget_host.h
+++ b/content/public/browser/render_widget_host.h
@@ -36,6 +36,7 @@ namespace content {
 
 class RenderProcessHost;
 class RenderWidgetHostImpl;
+class RenderWidgetHostIterator;
 class RenderWidgetHostView;
 
 // A RenderWidgetHost manages the browser side of a browser<->renderer
@@ -121,10 +122,9 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
   // Returns NULL if the IDs do not correspond to a live RenderWidgetHost.
   static RenderWidgetHost* FromID(int32 process_id, int32 routing_id);
 
-  typedef std::vector<RenderWidgetHost*> List;
-
-  // Returns the global list of active render widget hosts.
-  static RenderWidgetHost::List GetRenderWidgetHosts();
+  // Returns an iterator to iterate over the global list of active render widget
+  // hosts.
+  static scoped_ptr<RenderWidgetHostIterator> GetRenderWidgetHosts();
 
   virtual ~RenderWidgetHost() {}
 
diff --git a/content/public/browser/render_widget_host_iterator.h b/content/public/browser/render_widget_host_iterator.h
new file mode 100644
index 0000000..508fddb
--- /dev/null
+++ b/content/public/browser/render_widget_host_iterator.h
@@ -0,0 +1,25 @@
+// Copyright (c) 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.
+
+#ifndef CONTENT_PUBLIC_BROWSER_RENDER_WIDGET_HOST_ITERATOR_H_
+#define CONTENT_PUBLIC_BROWSER_RENDER_WIDGET_HOST_ITERATOR_H_
+
+namespace content {
+
+class RenderWidgetHost;
+
+// RenderWidgetHostIterator is used to safely iterate over a list of
+// RenderWidgetHosts.
+class RenderWidgetHostIterator {
+ public:
+  virtual ~RenderWidgetHostIterator() {}
+
+  // Returns the next RenderWidgetHost in the list. Returns NULL if none is
+  // available.
+  virtual RenderWidgetHost* GetNextHost() = 0;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_RENDER_WIDGET_HOST_ITERATOR_H_
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc
index 3846e0b..5e4bd1c 100644
--- a/content/public/test/mock_render_process_host.cc
+++ b/content/public/test/mock_render_process_host.cc
@@ -14,6 +14,7 @@
 #include "content/common/child_process_host_impl.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/storage_partition.h"
 
 namespace content {
@@ -206,10 +207,11 @@ IPC::ChannelProxy* MockRenderProcessHost::GetChannel() {
 
 int MockRenderProcessHost::GetActiveViewCount() {
   int num_active_views = 0;
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
-  for (size_t i = 0; i < widgets.size(); ++i) {
+  scoped_ptr<RenderWidgetHostIterator> widgets(
+      RenderWidgetHost::GetRenderWidgetHosts());
+  while (RenderWidgetHost* widget = widgets->GetNextHost()) {
     // Count only RenderWidgetHosts in this process.
-    if (widgets[i]->GetProcess()->GetID() == GetID())
+    if (widget->GetProcess()->GetID() == GetID())
       num_active_views++;
   }
   return num_active_views;