A quick fix for Bug 52834.

It seems some unit tests do not delete MockRenderProcessHost objects created via the MockRenderProcessHostFactory class. This change adds a list of MockRenderProcessHost objects to the MockRenderProcessHostFactory class so it can delete them in its destructor when tests do not delete them.

BUG=52834
TEST=make the "Linux Test (valgrind)(1)" bot green without suppressions.
Review URL: http://codereview.chromium.org/3431006

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

3 files changed, 54 insertions, 42 deletions

diff --git a/chrome/browser/renderer_host/mock_render_process_host.cc b/chrome/browser/renderer_host/mock_render_process_host.cc
index 63272f5..cdecb6f 100644
--- a/chrome/browser/renderer_host/mock_render_process_host.cc
+++ b/chrome/browser/renderer_host/mock_render_process_host.cc
@@ -9,7 +9,8 @@
 MockRenderProcessHost::MockRenderProcessHost(Profile* profile)
     : RenderProcessHost(profile),
       transport_dib_(NULL),
-      bad_msg_count_(0) {
+      bad_msg_count_(0),
+      factory_(NULL) {
   // Child process security operations can't be unit tested unless we add
   // ourselves as an existing child process.
   ChildProcessSecurityPolicy::GetInstance()->Add(id());
@@ -18,6 +19,8 @@ MockRenderProcessHost::MockRenderProcessHost(Profile* profile)
 MockRenderProcessHost::~MockRenderProcessHost() {
   ChildProcessSecurityPolicy::GetInstance()->Remove(id());
   delete transport_dib_;
+  if (factory_)
+    factory_->Remove(this);
 }
 
 bool MockRenderProcessHost::Init(
@@ -115,3 +118,32 @@ void MockRenderProcessHost::OnMessageReceived(const IPC::Message& msg) {
 
 void MockRenderProcessHost::OnChannelConnected(int32 peer_pid) {
 }
+
+MockRenderProcessHostFactory::~MockRenderProcessHostFactory() {
+  // Detach this object from MockRenderProcesses to prevent STLDeleteElements()
+  // from calling MockRenderProcessHostFactory::Remove().
+  for (ScopedVector<MockRenderProcessHost>::iterator it = processes_.begin();
+       it != processes_.end(); ++it) {
+    (*it)->SetFactory(NULL);
+  }
+}
+
+RenderProcessHost* MockRenderProcessHostFactory::CreateRenderProcessHost(
+    Profile* profile) const {
+  MockRenderProcessHost* host = new MockRenderProcessHost(profile);
+  if (host) {
+    processes_.push_back(host);
+    host->SetFactory(this);
+  }
+  return host;
+}
+
+void MockRenderProcessHostFactory::Remove(MockRenderProcessHost* host) const {
+  for (ScopedVector<MockRenderProcessHost>::iterator it = processes_.begin();
+       it != processes_.end(); ++it) {
+    if (*it == host) {
+      processes_.weak_erase(it);
+      break;
+    }
+  }
+}
diff --git a/chrome/browser/renderer_host/mock_render_process_host.h b/chrome/browser/renderer_host/mock_render_process_host.h
index 34f00f9..33a6516 100644
--- a/chrome/browser/renderer_host/mock_render_process_host.h
+++ b/chrome/browser/renderer_host/mock_render_process_host.h
@@ -7,9 +7,11 @@
 #pragma once
 
 #include "base/basictypes.h"
+#include "base/scoped_vector.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/common/ipc_test_sink.h"
 
+class MockRenderProcessHostFactory;
 class TransportDIB;
 class URLRequestContextGetter;
 
@@ -62,11 +64,18 @@ class MockRenderProcessHost : public RenderProcessHost {
   virtual void OnMessageReceived(const IPC::Message& msg);
   virtual void OnChannelConnected(int32 peer_pid);
 
+  // Attaches the factory object so we can remove this object in its destructor
+  // and prevent MockRenderProcessHostFacotry from deleting it.
+  void SetFactory(const MockRenderProcessHostFactory* factory) {
+    factory_ = factory;
+  }
+
  private:
   // Stores IPC messages that would have been sent to the renderer.
   IPC::TestSink sink_;
   TransportDIB* transport_dib_;
   int bad_msg_count_;
+  const MockRenderProcessHostFactory* factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MockRenderProcessHost);
 };
@@ -74,14 +83,21 @@ class MockRenderProcessHost : public RenderProcessHost {
 class MockRenderProcessHostFactory : public RenderProcessHostFactory {
  public:
   MockRenderProcessHostFactory() {}
-  virtual ~MockRenderProcessHostFactory() {}
+  virtual ~MockRenderProcessHostFactory();
 
-  virtual RenderProcessHost* CreateRenderProcessHost(
-      Profile* profile) const {
-    return new MockRenderProcessHost(profile);
-  }
+  virtual RenderProcessHost* CreateRenderProcessHost(Profile* profile) const;
+
+  // Removes the given MockRenderProcessHost from the MockRenderProcessHost list
+  // without deleting it. When a test deletes a MockRenderProcessHost, we need
+  // to remove it from |processes_| to prevent it from being deleted twice.
+  void Remove(MockRenderProcessHost* host) const;
 
  private:
+  // A list of MockRenderProcessHosts created by this object. This list is used
+  // for deleting all MockRenderProcessHosts that have not deleted by a test in
+  // the destructor and prevent them from being leaked.
+  mutable ScopedVector<MockRenderProcessHost> processes_;
+
   DISALLOW_COPY_AND_ASSIGN(MockRenderProcessHostFactory);
 };
 
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index 8dfc818..4795e7f 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -2547,42 +2547,6 @@
    fun:_ZN11MessageLoop10RunHandlerEv
 }
 {
-   bug_52834_a
-   Memcheck:Leak
-   fun:_Znw*
-   ...
-   fun:_ZNK28MockRenderProcessHostFactory23CreateRenderProcessHostEP7Profile
-   fun:_ZN12SiteInstance10GetProcessEv
-   fun:_ZN14RenderViewHostC2EP12SiteInstanceP22RenderViewHostDelegateix
-}
-{
-   bug_52834_b
-   Memcheck:Leak
-   fun:_Znw*
-   ...
-   fun:_ZN5IDMapIN3IPC7Channel8ListenerEL23IDMapOwnershipSemantics0EE9AddWithIDEPS2_i
-   fun:_ZN17RenderProcessHost6AttachEPN3IPC7Channel8ListenerEi
-   fun:_ZN16RenderWidgetHostC2EP17RenderProcessHosti
-   ...
-   fun:_ZN26RenderWidgetFullscreenHostC1EP17RenderProcessHosti
-}
-{
-   bug_52834_c
-   Memcheck:Leak
-   fun:_Znw*
-   fun:_ZNK28MockRenderProcessHostFactory23CreateRenderProcessHostEP7Profile
-   fun:_ZN12SiteInstance10GetProcessEv
-   fun:_ZN14RenderViewHostC2EP12SiteInstanceP22RenderViewHostDelegateiP23SessionStorageNamespace
-   ...
-   fun:_ZN18TestRenderViewHostC1EP12SiteInstanceP22RenderViewHostDelegatei
-   fun:_ZN25TestRenderViewHostFactory20CreateRenderViewHostEP12SiteInstanceP22RenderViewHostDelegateiP23SessionStorageNamespace
-   fun:_ZN21RenderViewHostFactory6CreateEP12SiteInstanceP22RenderViewHostDelegateiP23SessionStorageNamespace
-   fun:_ZN21RenderViewHostManager4InitEP7ProfileP12SiteInstancei
-   fun:_ZN11TabContentsC2EP7ProfileP12SiteInstanceiPKS_P23SessionStorageNamespace
-   ...
-   fun:_ZN25RenderViewHostTestHarness5SetUpEv
-}
-{
    bug_52837
    Memcheck:Leak
    fun:_Znw*