Changed the semantics of RenderWidgetHost::GetRenderWidgetHosts() to filter out swapped-out ones.

Changed the semantics of RenderWidgetHost::GetRenderWidgetHosts() so
that it only returns *active* RenderWidgetHosts which are not swapped
out. We (creis/dsjang) checked all the callsites of this function.

This change fixes the bug that swapped-out views are shown (they are
not supposed to be shown) on 1) chrome://memory, 2)
chrome://performance (in an event of killing a view), 3)
chrome://accessibility.

BUG=137990

Review URL: https://chromiumcodereview.appspot.com/17941005

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

10 files changed, 87 insertions, 33 deletions

diff --git a/content/browser/accessibility/browser_accessibility_state_impl.cc b/content/browser/accessibility/browser_accessibility_state_impl.cc
index 8e1f1e0..befd717 100644
--- a/content/browser/accessibility/browser_accessibility_state_impl.cc
+++ b/content/browser/accessibility/browser_accessibility_state_impl.cc
@@ -129,7 +129,10 @@ void BrowserAccessibilityStateImpl::SetAccessibilityMode(
     return;
   accessibility_mode_ = mode;
 
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
+  // 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) {
     // Ignore processes that don't have a connection, such as crashed tabs.
     if (!widgets[i]->GetProcess()->HasConnection())
diff --git a/content/browser/devtools/render_view_devtools_agent_host.cc b/content/browser/devtools/render_view_devtools_agent_host.cc
index 531a023..e4812d3 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.cc
+++ b/content/browser/devtools/render_view_devtools_agent_host.cc
@@ -129,10 +129,6 @@ std::vector<RenderViewHost*> DevToolsAgentHost::GetValidRenderViewHosts() {
       continue;
 
     RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
-    // Don't report swapped out views.
-    if (static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out())
-      continue;
-
     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 631ad81..3389374 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -85,6 +85,7 @@
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_helper.h"
+#include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/socket_stream_dispatcher_host.h"
 #include "content/browser/renderer_host/text_input_client_message_filter.h"
 #include "content/browser/resolve_proxy_msg_helper.h"
@@ -1633,18 +1634,7 @@ int RenderProcessHostImpl::GetActiveViewCount() {
   RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
   for (size_t i = 0; i < widgets.size(); ++i) {
     // Count only RenderWidgetHosts in this process.
-    if (widgets[i]->GetProcess()->GetID() != GetID())
-      continue;
-
-    // All RenderWidgetHosts are swapped in.
-    if (!widgets[i]->IsRenderView()) {
-      num_active_views++;
-      continue;
-    }
-
-    // Don't count swapped out views.
-    RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
-    if (!static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out())
+    if (widgets[i]->GetProcess()->GetID() == GetID())
       num_active_views++;
   }
   return num_active_views;
@@ -1776,7 +1766,9 @@ void RenderProcessHostImpl::OnCompositorSurfaceBuffersSwappedNoHost(
 }
 
 void RenderProcessHostImpl::OnGpuSwitching() {
-  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
+  // 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())
       continue;
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index cc98a8b..c2b0584 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -475,8 +475,8 @@ void RenderViewHostImpl::WasSwappedOut() {
     base::ProcessHandle process_handle = GetProcess()->GetHandle();
     int views = 0;
 
-    // Count the number of widget hosts for the process, which is equivalent to
-    // views using the process as of this writing.
+    // 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())
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 456f6df..4d1a04a 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -264,6 +264,28 @@ std::vector<RenderWidgetHost*> RenderWidgetHost::GetRenderWidgetHosts() {
   for (RoutingIDWidgetMap::const_iterator it = widgets->begin();
        it != widgets->end();
        ++it) {
+    RenderWidgetHost* widget = it->second;
+
+    if (!widget->IsRenderView()) {
+      hosts.push_back(widget);
+      continue;
+    }
+
+    // Add only active RenderViewHosts.
+    RenderViewHost* rvh = RenderViewHost::From(widget);
+    if (!static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out())
+      hosts.push_back(widget);
+  }
+  return hosts;
+}
+
+// static
+std::vector<RenderWidgetHost*> RenderWidgetHostImpl::GetAllRenderWidgetHosts() {
+  std::vector<RenderWidgetHost*> hosts;
+  RoutingIDWidgetMap* widgets = g_routing_id_widget_map.Pointer();
+  for (RoutingIDWidgetMap::const_iterator it = widgets->begin();
+       it != widgets->end();
+       ++it) {
     hosts.push_back(it->second);
   }
   return hosts;
diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h
index 5aa6a96..c581cdd 100644
--- a/content/browser/renderer_host/render_widget_host_impl.h
+++ b/content/browser/renderer_host/render_widget_host_impl.h
@@ -97,6 +97,11 @@ class CONTENT_EXPORT RenderWidgetHostImpl : virtual public RenderWidgetHost,
   // Similar to RenderWidgetHost::FromID, but returning the Impl object.
   static RenderWidgetHostImpl* FromID(int32 process_id, int32 routing_id);
 
+  // Returns all RenderWidgetHosts including swapped out ones for
+  // internal use. The public interface
+  // RendgerWidgetHost::GetRenderWidgetHosts only returns active ones.
+  static std::vector<RenderWidgetHost*> GetAllRenderWidgetHosts();
+
   // Use RenderWidgetHostImpl::From(rwh) to downcast a
   // RenderWidgetHost to a RenderWidgetHostImpl.  Internally, this
   // uses RenderWidgetHost::AsRenderWidgetHostImpl().
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 05be4fc..741230e 100644
--- a/content/browser/web_contents/render_view_host_manager_unittest.cc
+++ b/content/browser/web_contents/render_view_host_manager_unittest.cc
@@ -322,6 +322,47 @@ TEST_F(RenderViewHostManagerTest, WhiteListDidActivateAcceleratedCompositing) {
   EXPECT_TRUE(swapped_out_rvh->is_accelerated_compositing_active());
 }
 
+// Test if RenderViewHost::GetRenderWidgetHosts() only returns active
+// widgets.
+TEST_F(RenderViewHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
+  TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
+  EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
+
+  RenderWidgetHost::List 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]);
+  EXPECT_FALSE(static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out());
+}
+
+// Test if RenderViewHost::GetRenderWidgetHosts() returns a subset of
+// RenderViewHostImpl::GetAllRenderWidgetHosts().
+// RenderViewHost::GetRenderWidgetHosts() returns only active widgets, but
+// RenderViewHostImpl::GetAllRenderWidgetHosts() returns everything
+// including swapped out ones.
+TEST_F(RenderViewHostManagerTest,
+       GetRenderWidgetHostsWithinGetAllRenderWidgetHosts) {
+  TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
+  EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
+
+  RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
+  RenderWidgetHost::List all_widgets =
+      RenderWidgetHostImpl::GetAllRenderWidgetHosts();
+
+  for (size_t i = 0; i < widgets.size(); ++i) {
+    bool found = false;
+    for (size_t j = 0; j < all_widgets.size(); ++j) {
+      if (widgets[i] == widgets[j]) {
+        found = true;
+        break;
+      }
+    }
+    EXPECT_TRUE(found);
+  }
+}
+
 // When there is an error with the specified page, renderer exits view-source
 // mode. See WebFrameImpl::DidFail(). We check by this test that
 // EnableViewSourceMode message is sent on every navigation regardless
diff --git a/content/public/browser/render_widget_host.h b/content/public/browser/render_widget_host.h
index 5a4918d..ca727b7 100644
--- a/content/public/browser/render_widget_host.h
+++ b/content/public/browser/render_widget_host.h
@@ -122,7 +122,8 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
   static RenderWidgetHost* FromID(int32 process_id, int32 routing_id);
 
   typedef std::vector<RenderWidgetHost*> List;
-  // Returns the global list of render widget hosts.
+
+  // Returns the global list of active render widget hosts.
   static RenderWidgetHost::List GetRenderWidgetHosts();
 
   virtual ~RenderWidgetHost() {}
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc
index 1a20910..77608af 100644
--- a/content/public/test/mock_render_process_host.cc
+++ b/content/public/test/mock_render_process_host.cc
@@ -209,18 +209,7 @@ int MockRenderProcessHost::GetActiveViewCount() {
   RenderWidgetHost::List widgets = RenderWidgetHost::GetRenderWidgetHosts();
   for (size_t i = 0; i < widgets.size(); ++i) {
     // Count only RenderWidgetHosts in this process.
-    if (widgets[i]->GetProcess()->GetID() != GetID())
-      continue;
-
-    // All RenderWidgetHosts are swapped in.
-    if (!widgets[i]->IsRenderView()) {
-      num_active_views++;
-      continue;
-    }
-
-    // Don't count swapped out views.
-    RenderViewHost* rvh = RenderViewHost::From(widgets[i]);
-    if (!static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out())
+    if (widgets[i]->GetProcess()->GetID() == GetID())
       num_active_views++;
   }
   return num_active_views;
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 54e4e4c..98c2b69 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1320,6 +1320,11 @@ void RenderViewImpl::OnNavigate(const ViewMsg_Navigate_Params& params) {
     // navigation to accomplish that.
     is_reload = false;
 
+    // We refresh timezone when a view is swapped in since timezone
+    // can get out of sync when the system timezone is updated while
+    // the view is swapped out.
+    NotifyTimezoneChange(webview()->mainFrame());
+
     SetSwappedOut(false);
   }