Switch ChromeTestSuite to the same convention as ContentTestSuite:

the test suite implicitly provides global resources for each test,
but they're re-initialized between each test.

The performance overhead is negligible.

We need that to continue moving tests from unit_tests
to content_unittests. Because of shared test fixtures
the test suites need to be compatible.

BUG=90443

Review URL: http://codereview.chromium.org/7744039

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

10 files changed, 131 insertions, 139 deletions

diff --git a/chrome/test/base/browser_with_test_window_test.cc b/chrome/test/base/browser_with_test_window_test.cc
index 1bdc229..3de42a4 100644
--- a/chrome/test/base/browser_with_test_window_test.cc
+++ b/chrome/test/base/browser_with_test_window_test.cc
@@ -29,7 +29,7 @@ BrowserWithTestWindowTest::BrowserWithTestWindowTest()
 }
 
 void BrowserWithTestWindowTest::SetUp() {
-  TestingBrowserProcessTest::SetUp();
+  testing::Test::SetUp();
 
   profile_.reset(CreateProfile());
   browser_.reset(new Browser(Browser::TYPE_TABBED, profile()));
diff --git a/chrome/test/base/browser_with_test_window_test.h b/chrome/test/base/browser_with_test_window_test.h
index 9615b6c..4f32979 100644
--- a/chrome/test/base/browser_with_test_window_test.h
+++ b/chrome/test/base/browser_with_test_window_test.h
@@ -9,7 +9,6 @@
 #include "base/message_loop.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/test_browser_window.h"
-#include "chrome/test/base/testing_browser_process_test.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/renderer_host/test_render_view_host.h"
@@ -39,7 +38,7 @@ class NavigationController;
 //
 // Subclasses must invoke BrowserWithTestWindowTest::SetUp as it is responsible
 // for creating the various objects of this class.
-class BrowserWithTestWindowTest : public TestingBrowserProcessTest {
+class BrowserWithTestWindowTest : public testing::Test {
  public:
   BrowserWithTestWindowTest();
   virtual ~BrowserWithTestWindowTest();
diff --git a/chrome/test/base/chrome_test_suite.cc b/chrome/test/base/chrome_test_suite.cc
index 9661bf7..fa02aa9 100644
--- a/chrome/test/base/chrome_test_suite.cc
+++ b/chrome/test/base/chrome_test_suite.cc
@@ -5,21 +5,29 @@
 #include "chrome/test/base/chrome_test_suite.h"
 
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
+#include "base/memory/ref_counted.h"
 #include "base/metrics/stats_table.h"
+#include "base/path_service.h"
 #include "base/process_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "content/common/content_paths.h"
+#include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/ui_base_paths.h"
+
 #if defined(TOOLKIT_VIEWS)
 #include "views/view.h"
 #endif
@@ -33,7 +41,9 @@
 #include "base/shared_memory.h"
 #endif
 
-static void RemoveSharedMemoryFile(const std::string& filename) {
+namespace {
+
+void RemoveSharedMemoryFile(const std::string& filename) {
   // Stats uses SharedMemory under the hood. On posix, this results in a file
   // on disk.
 #if defined(OS_POSIX)
@@ -42,49 +52,108 @@ static void RemoveSharedMemoryFile(const std::string& filename) {
 #endif
 }
 
-LocalHostResolverProc::LocalHostResolverProc()
-    : HostResolverProc(NULL) {
-}
+// In many cases it may be not obvious that a test makes a real DNS lookup.
+// We generally don't want to rely on external DNS servers for our tests,
+// so this host resolver procedure catches external queries and returns a failed
+// lookup result.
+class LocalHostResolverProc : public net::HostResolverProc {
+ public:
+  LocalHostResolverProc() : HostResolverProc(NULL) {
+  }
 
-LocalHostResolverProc::~LocalHostResolverProc() {
-}
+  virtual int Resolve(const std::string& host,
+                      net::AddressFamily address_family,
+                      net::HostResolverFlags host_resolver_flags,
+                      net::AddressList* addrlist,
+                      int* os_error) {
+    const char* kLocalHostNames[] = {"localhost", "127.0.0.1", "::1"};
+    bool local = false;
+
+    if (host == net::GetHostName()) {
+      local = true;
+    } else {
+      for (size_t i = 0; i < arraysize(kLocalHostNames); i++)
+        if (host == kLocalHostNames[i]) {
+          local = true;
+          break;
+        }
+    }
+
+    // To avoid depending on external resources and to reduce (if not preclude)
+    // network interactions from tests, we simulate failure for non-local DNS
+    // queries, rather than perform them.
+    // If you really need to make an external DNS query, use
+    // net::RuleBasedHostResolverProc and its AllowDirectLookup method.
+    if (!local) {
+      DVLOG(1) << "To avoid external dependencies, simulating failure for "
+          "external DNS lookup of " << host;
+      return net::ERR_NOT_IMPLEMENTED;
+    }
+
+    return ResolveUsingPrevious(host, address_family, host_resolver_flags,
+                                addrlist, os_error);
+  }
+};
+
+class ChromeTestSuiteInitializer : public testing::EmptyTestEventListener {
+ public:
+  ChromeTestSuiteInitializer() {
+  }
+
+  virtual void OnTestStart(const testing::TestInfo& test_info) OVERRIDE {
+    DCHECK(!g_browser_process);
+    g_browser_process = new TestingBrowserProcess;
 
-int LocalHostResolverProc::Resolve(const std::string& host,
-                                   net::AddressFamily address_family,
-                                   net::HostResolverFlags host_resolver_flags,
-                                   net::AddressList* addrlist,
-                                   int* os_error) {
-  const char* kLocalHostNames[] = {"localhost", "127.0.0.1", "::1"};
-  bool local = false;
-
-  if (host == net::GetHostName()) {
-    local = true;
-  } else {
-    for (size_t i = 0; i < arraysize(kLocalHostNames); i++)
-      if (host == kLocalHostNames[i]) {
-        local = true;
-        break;
-      }
+    DCHECK(!content::GetContentClient());
+    content_client_.reset(new chrome::ChromeContentClient);
+    browser_content_client_.reset(new chrome::ChromeContentBrowserClient());
+    content_client_->set_browser(browser_content_client_.get());
+    content::SetContentClient(content_client_.get());
+
+    SetUpHostResolver();
   }
 
-  // To avoid depending on external resources and to reduce (if not preclude)
-  // network interactions from tests, we simulate failure for non-local DNS
-  // queries, rather than perform them.
-  // If you really need to make an external DNS query, use
-  // net::RuleBasedHostResolverProc and its AllowDirectLookup method.
-  if (!local) {
-    DVLOG(1) << "To avoid external dependencies, simulating failure for "
-                "external DNS lookup of " << host;
-    return net::ERR_NOT_IMPLEMENTED;
+  virtual void OnTestEnd(const testing::TestInfo& test_info) OVERRIDE {
+    if (g_browser_process) {
+      delete g_browser_process;
+      g_browser_process = NULL;
+    }
+
+    DCHECK_EQ(content_client_.get(), content::GetContentClient());
+    browser_content_client_.reset();
+    content_client_.reset();
+    content::SetContentClient(NULL);
+
+    TearDownHostResolver();
   }
 
-  return ResolveUsingPrevious(host, address_family, host_resolver_flags,
-                              addrlist, os_error);
-}
+ private:
+  void SetUpHostResolver() {
+    host_resolver_proc_ = new LocalHostResolverProc;
+    scoped_host_resolver_proc_.reset(
+        new net::ScopedDefaultHostResolverProc(host_resolver_proc_.get()));
+  }
+
+  void TearDownHostResolver() {
+    scoped_host_resolver_proc_.reset();
+    host_resolver_proc_ = NULL;
+  }
+
+  scoped_ptr<BrowserProcess> browser_process_;
+
+  scoped_ptr<chrome::ChromeContentClient> content_client_;
+  scoped_ptr<chrome::ChromeContentBrowserClient> browser_content_client_;
+
+  scoped_refptr<LocalHostResolverProc> host_resolver_proc_;
+  scoped_ptr<net::ScopedDefaultHostResolverProc> scoped_host_resolver_proc_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeTestSuiteInitializer);
+};
+
+}  // namespace
 
 ChromeTestSuite::ChromeTestSuite(int argc, char** argv)
-    : base::TestSuite(argc, argv),
-      stats_table_(NULL) {
+    : base::TestSuite(argc, argv) {
 }
 
 ChromeTestSuite::~ChromeTestSuite() {
@@ -99,13 +168,7 @@ void ChromeTestSuite::Initialize() {
 
   base::TestSuite::Initialize();
 
-  // Initialize the content client which that code uses to talk to Chrome.
-  content::SetContentClient(&chrome_content_client_);
-  content::GetContentClient()->set_browser(&chrome_browser_content_client_);
-
   chrome::RegisterChromeSchemes();
-  host_resolver_proc_ = new LocalHostResolverProc();
-  scoped_host_resolver_proc_.Init(host_resolver_proc_.get());
 
   chrome::RegisterPathProvider();
   content::RegisterPathProvider();
@@ -133,19 +196,20 @@ void ChromeTestSuite::Initialize() {
       resources_pack_path.Append(FILE_PATH_LITERAL("resources.pak"));
   ResourceBundle::AddDataPackToSharedInstance(resources_pack_path);
 
-  // initialize the global StatsTable for unit_tests (make sure the file
-  // doesn't exist before opening it so the test gets a clean slate)
-  stats_filename_ = "unit_tests";
-  std::string pid_string = base::StringPrintf("-%d", base::GetCurrentProcId());
-  stats_filename_ += pid_string;
-  RemoveSharedMemoryFile(stats_filename_);
-  stats_table_ = new base::StatsTable(stats_filename_, 20, 200);
-  base::StatsTable::set_current(stats_table_);
-
 #if defined(TOOLKIT_VIEWS) && defined(OS_LINUX)
   // Turn of GPU compositing in browser during unit tests.
   views::View::set_use_acceleration_when_possible(false);
 #endif
+
+  stats_filename_ = base::StringPrintf("unit_tests-%d",
+                                       base::GetCurrentProcId());
+  RemoveSharedMemoryFile(stats_filename_);
+  stats_table_.reset(new base::StatsTable(stats_filename_, 20, 200));
+  base::StatsTable::set_current(stats_table_.get());
+
+  testing::TestEventListeners& listeners =
+      testing::UnitTest::GetInstance()->listeners();
+  listeners.Append(new ChromeTestSuiteInitializer);
 }
 
 void ChromeTestSuite::Shutdown() {
@@ -155,9 +219,8 @@ void ChromeTestSuite::Shutdown() {
   base::mac::SetOverrideAppBundle(NULL);
 #endif
 
-  // Tear down shared StatsTable; prevents unit_tests from leaking it.
   base::StatsTable::set_current(NULL);
-  delete stats_table_;
+  stats_table_.reset();
   RemoveSharedMemoryFile(stats_filename_);
 
   base::TestSuite::Shutdown();
diff --git a/chrome/test/base/chrome_test_suite.h b/chrome/test/base/chrome_test_suite.h
index 1a52ece..14f2f7b 100644
--- a/chrome/test/base/chrome_test_suite.h
+++ b/chrome/test/base/chrome_test_suite.h
@@ -8,39 +8,15 @@
 
 #include <string>
 
-#include "build/build_config.h"
-
-#include "base/file_util.h"
-#include "base/memory/ref_counted.h"
-#include "base/path_service.h"
+#include "base/file_path.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/test/test_suite.h"
 #include "chrome/app/scoped_ole_initializer.h"
-#include "chrome/browser/chrome_content_browser_client.h"
-#include "chrome/common/chrome_content_client.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "net/base/mock_host_resolver.h"
-#include "net/base/net_util.h"
 
 namespace base {
 class StatsTable;
 }
 
-// In many cases it may be not obvious that a test makes a real DNS lookup.
-// We generally don't want to rely on external DNS servers for our tests,
-// so this host resolver procedure catches external queries and returns a failed
-// lookup result.
-class LocalHostResolverProc : public net::HostResolverProc {
- public:
-  LocalHostResolverProc();
-  virtual ~LocalHostResolverProc();
-
-  virtual int Resolve(const std::string& host,
-                      net::AddressFamily address_family,
-                      net::HostResolverFlags host_resolver_flags,
-                      net::AddressList* addrlist,
-                      int* os_error);
-};
-
 class ChromeTestSuite : public base::TestSuite {
  public:
   ChromeTestSuite(int argc, char** argv);
@@ -54,22 +30,13 @@ class ChromeTestSuite : public base::TestSuite {
     browser_dir_ = browser_dir;
   }
 
-  // Client for embedding content in Chrome.
-  chrome::ChromeContentClient chrome_content_client_;
-  chrome::ChromeContentBrowserClient chrome_browser_content_client_;
-
-  base::StatsTable* stats_table_;
-
-  // The name used for the stats file so it can be cleaned up on posix during
-  // test shutdown.
-  std::string stats_filename_;
-
   // Alternative path to browser binaries.
   FilePath browser_dir_;
 
+  std::string stats_filename_;
+  scoped_ptr<base::StatsTable> stats_table_;
+
   ScopedOleInitializer ole_initializer_;
-  scoped_refptr<LocalHostResolverProc> host_resolver_proc_;
-  net::ScopedDefaultHostResolverProc scoped_host_resolver_proc_;
 };
 
 #endif  // CHROME_TEST_BASE_CHROME_TEST_SUITE_H_
diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc
index 52cebdd6..d8d621f 100644
--- a/chrome/test/base/in_process_browser_test.cc
+++ b/chrome/test/base/in_process_browser_test.cc
@@ -95,7 +95,11 @@ void InProcessBrowserTest::SetUp() {
   ASSERT_TRUE(CreateUserDataDirectory())
       << "Could not create user data directory.";
 
-  DCHECK(!g_browser_process);
+  // Undo TestingBrowserProcess creation in ChromeTestSuite.
+  // TODO(phajdan.jr): Extract a smaller test suite so we don't need this.
+  DCHECK(g_browser_process);
+  delete g_browser_process;
+  g_browser_process = NULL;
 
   // Allow subclasses the opportunity to make changes to the default user data
   // dir before running any tests.
diff --git a/chrome/test/base/testing_browser_process.cc b/chrome/test/base/testing_browser_process.cc
index 315c2c2..f86f6d5 100644
--- a/chrome/test/base/testing_browser_process.cc
+++ b/chrome/test/base/testing_browser_process.cc
@@ -271,13 +271,3 @@ void TestingBrowserProcess::SetIOThread(IOThread* io_thread) {
 void TestingBrowserProcess::SetDevToolsManager(DevToolsManager* manager) {
   devtools_manager_.reset(manager);
 }
-
-ScopedTestingBrowserProcess::ScopedTestingBrowserProcess() {
-  DCHECK(!g_browser_process);
-  g_browser_process = &browser_process_;
-}
-
-ScopedTestingBrowserProcess::~ScopedTestingBrowserProcess() {
-  DCHECK_EQ(&browser_process_, g_browser_process);
-  g_browser_process = NULL;
-}
diff --git a/chrome/test/base/testing_browser_process.h b/chrome/test/base/testing_browser_process.h
index 57eeda8c..5659875 100644
--- a/chrome/test/base/testing_browser_process.h
+++ b/chrome/test/base/testing_browser_process.h
@@ -148,20 +148,4 @@ class TestingBrowserProcess : public BrowserProcess {
   DISALLOW_COPY_AND_ASSIGN(TestingBrowserProcess);
 };
 
-// Scoper to put a TestingBrowserProcess in |g_browser_process|.
-class ScopedTestingBrowserProcess {
- public:
-  ScopedTestingBrowserProcess();
-  ~ScopedTestingBrowserProcess();
-
-  TestingBrowserProcess* get() {
-    return &browser_process_;
-  }
-
- private:
-  TestingBrowserProcess browser_process_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedTestingBrowserProcess);
-};
-
 #endif  // CHROME_TEST_BASE_TESTING_BROWSER_PROCESS_H_
diff --git a/chrome/test/base/testing_browser_process_test.h b/chrome/test/base/testing_browser_process_test.h
deleted file mode 100644
index eb37025..0000000
--- a/chrome/test/base/testing_browser_process_test.h
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright (c) 2011 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 CHROME_TEST_BASE_TESTING_BROWSER_PROCESS_TEST_H_
-#define CHROME_TEST_BASE_TESTING_BROWSER_PROCESS_TEST_H_
-#pragma once
-
-#include "chrome/test/base/testing_browser_process.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-// Base class for tests that need |g_browser_process| to be initialized.
-class TestingBrowserProcessTest : public testing::Test {
- protected:
-  ScopedTestingBrowserProcess testing_browser_process_;
-};
-
-#endif  // CHROME_TEST_BASE_TESTING_BROWSER_PROCESS_TEST_H_
diff --git a/chrome/test/pyautolib/pyautolib.cc b/chrome/test/pyautolib/pyautolib.cc
index fcccbf5..d9fadfc 100644
--- a/chrome/test/pyautolib/pyautolib.cc
+++ b/chrome/test/pyautolib/pyautolib.cc
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/base_paths.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
diff --git a/chrome/test/ui/ui_test_suite.cc b/chrome/test/ui/ui_test_suite.cc
index 917b500..e0967c4 100644
--- a/chrome/test/ui/ui_test_suite.cc
+++ b/chrome/test/ui/ui_test_suite.cc
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
 #include "chrome/common/chrome_switches.h"