Don't run BrowserPolicyConnector::Init from TestingBrowserProcess by default.

Tests that need internal stuff from the connector should run Init() from SetUp, or should be built as an InProcessBrowserTest instead.

BUG=141255,66054

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

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

chrome/browser/chromeos/login/login_utils_browsertest.cc

@@ -203,6 +203,7 @@ class LoginUtilsTest : public testing::Test,
     browser_process_->SetProfileManager(
         new ProfileManagerWithoutInit(scoped_temp_dir_.path()));
     connector_ = browser_process_->browser_policy_connector();
+    connector_->Init();
 
     RunAllPending();
   }

chrome/browser/policy/browser_policy_connector.cc

@@ -128,10 +128,7 @@ BrowserPolicyConnector::~BrowserPolicyConnector() {
 void BrowserPolicyConnector::Init() {
   DCHECK(!device_management_service_.get()) <<
       "BrowserPolicyConnector::Init() called twice.";
-  // Don't create platform providers if running in a unit test, since
-  // AsyncPlatformLoader requires deletion on the FILE thread.
-  if (MessageLoop::current())
-    platform_provider_.reset(CreatePlatformProvider());
+  platform_provider_.reset(CreatePlatformProvider());
 
   device_management_service_.reset(
       new DeviceManagementService(GetDeviceManagementUrl()));
@@ -149,10 +146,6 @@ void BrowserPolicyConnector::Init() {
 
   InitializeDevicePolicy();
 
-  // Don't bother updating the cache if this is a unit test.
-  if (!MessageLoop::current())
-    return;
-
   // Create the AppPackUpdater to start updating the cache. It requires the
   // system request context, which isn't available yet; therefore it is
   // created only once the loops are running.
@@ -546,10 +539,6 @@ void BrowserPolicyConnector::InitializeDevicePolicy() {
           device_policy_cache,
           GetDeviceManagementUrl()));
 
-      // Skip the final initialization if this is a unit test.
-      if (!MessageLoop::current())
-        return;
-
       // Initialize the subsystem once the message loops are spinning.
       MessageLoop::current()->PostTask(
           FROM_HERE,

chrome/browser/policy/config_dir_policy_loader.cc

@@ -13,11 +13,9 @@
 #include "base/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
-#include "base/message_loop.h"
 #include "base/platform_file.h"
 #include "base/stl_util.h"
 #include "chrome/browser/policy/policy_bundle.h"
-#include "content/public/browser/browser_thread.h"
 
 namespace policy {
 
@@ -38,13 +36,6 @@ ConfigDirPolicyLoader::ConfigDirPolicyLoader(const FilePath& config_dir,
 ConfigDirPolicyLoader::~ConfigDirPolicyLoader() {}
 
 void ConfigDirPolicyLoader::InitOnFile() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
-  // Some unit tests create a File thread that shares a MessageLoop with the
-  // UI thread, and this causes DCHECKs because IO operations aren't allowed on
-  // that thread. So if this is running in the context of one of those tests,
-  // just exit.
-  if (!MessageLoop::current()->IsType(MessageLoop::TYPE_IO))
-    return;
   base::files::FilePathWatcher::Callback callback =
       base::Bind(&ConfigDirPolicyLoader::OnFileUpdated, base::Unretained(this));
   mandatory_watcher_.Watch(config_dir_.Append(kMandatoryConfigDir), callback);

chrome/browser/policy/user_cloud_policy_store_unittest.cc

@@ -102,6 +102,7 @@ TEST_F(UserCloudPolicyStoreTest, Store) {
 
   // Store a simple policy and make sure it ends up as the currently active
   // policy.
+  EXPECT_CALL(observer_, OnStoreLoaded(store_.get()));
   store_->Store(policy_.policy());
   base::RunLoop run_loop;
   run_loop.RunUntilIdle();

chrome/browser/policy/user_policy_signin_service_unittest.cc

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/mock_cloud_policy_store.h"
 #include "chrome/browser/policy/user_cloud_policy_manager.h"
 #include "chrome/browser/policy/user_policy_signin_service.h"
@@ -24,9 +26,6 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using testing::_;
-using testing::Return;
-
 namespace policy {
 
 namespace {
@@ -35,10 +34,12 @@ class UserPolicySigninServiceTest : public testing::Test {
  public:
   UserPolicySigninServiceTest()
       : loop_(MessageLoop::TYPE_UI),
-        ui_thread_(content::BrowserThread::UI, &loop_) {
-  }
+        ui_thread_(content::BrowserThread::UI, &loop_),
+        file_thread_(content::BrowserThread::FILE, &loop_) {}
 
   virtual void SetUp() OVERRIDE {
+    g_browser_process->browser_policy_connector()->Init();
+
     local_state_.reset(new TestingPrefService);
     chrome::RegisterLocalState(local_state_.get());
     static_cast<TestingBrowserProcess*>(g_browser_process)->SetLocalState(
@@ -65,11 +66,15 @@ class UserPolicySigninServiceTest : public testing::Test {
   virtual void TearDown() OVERRIDE {
     // Free the profile before we clear out the browser prefs.
     profile_.reset();
-    static_cast<TestingBrowserProcess*>(g_browser_process)->SetLocalState(NULL);
+    TestingBrowserProcess* testing_browser_process =
+        static_cast<TestingBrowserProcess*>(g_browser_process);
+    testing_browser_process->SetLocalState(NULL);
     local_state_.reset();
+    testing_browser_process->SetBrowserPolicyConnector(NULL);
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
   }
 
-
   scoped_ptr<TestingProfile> profile_;
   // Weak pointer to a MockCloudPolicyStore - lifetime is managed by the
   // UserCloudPolicyManager.
@@ -79,6 +84,7 @@ class UserPolicySigninServiceTest : public testing::Test {
   // asynchronously via tasks, so create a fake thread here.
   MessageLoop loop_;
   content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
 
   scoped_ptr<TestingPrefService> local_state_;
 };

chrome/test/base/testing_browser_process.cc

@@ -73,10 +73,8 @@ chrome_variations::VariationsService*
 policy::BrowserPolicyConnector*
     TestingBrowserProcess::browser_policy_connector() {
 #if defined(ENABLE_CONFIGURATION_POLICY)
-  if (!browser_policy_connector_.get()) {
+  if (!browser_policy_connector_.get())
     browser_policy_connector_.reset(new policy::BrowserPolicyConnector());
-    browser_policy_connector_->Init();
-  }
 #endif
   return browser_policy_connector_.get();
 }

tools/heapcheck/suppressions.txt

@@ -650,25 +650,6 @@
    fun:std::vector::push_back
    fun:IPC::ChannelProxy::Context::OnAddFilter
 }
-{
-   bug_66054_a
-   Heapcheck:Leak
-   ...
-   fun:policy::CreateSequencedTestDictionaryActionP::gmock_Impl::gmock_PerformImpl
-   fun:testing::internal::ActionHelper::Perform
-   fun:policy::CreateSequencedTestDictionaryActionP::gmock_Impl::Perform
-   fun:testing::Action::Perform
-   fun:testing::internal::FunctionMockerBase::InvokeWith
-   fun:testing::internal::FunctionMocker::Invoke
-   fun:policy::ProviderDelegateMock::Load
-   fun:policy::AsynchronousPolicyLoader::Reload
-   fun:policy::AsynchronousPolicyLoaderTest_ProviderNotificationOnPolicyChange_Test::TestBody
-}
-{
-   bug_66054_b
-   Heapcheck:Leak
-   fun:policy::AsynchronousPolicyTestBase_ProviderRefresh_Test::TestBody
-}
 {
    bug_67524
    Heapcheck:Leak
@@ -1508,75 +1489,6 @@
    fun:ReloadButton::ReloadButton
    fun:ReloadButtonTest::ReloadButtonTest
 }
-{
-   bug_141255_a
-   Heapcheck:Leak
-   ...
-   fun:policy::BrowserPolicyConnector::CreatePlatformProvider
-   fun:policy::BrowserPolicyConnector::Init
-   fun:TestingBrowserProcess::browser_policy_connector
-}
-{
-   bug_141255_b
-   Heapcheck:Leak
-   fun:base::internal::WeakReferenceOwner::GetRef
-   fun:base::WeakPtrFactory::GetWeakPtr
-   fun:policy::AsyncPolicyLoader::ScheduleNextReload
-   fun:policy::AsyncPolicyLoader::Init
-}
-{
-   bug_141255_c
-   Heapcheck:Leak
-   ...
-   fun:policy::ConfigDirPolicyLoader::InitOnFile
-   fun:policy::AsyncPolicyLoader::Init
-}
-{
-   bug_141255_d
-   Heapcheck:Leak
-   fun:base::Bind
-   fun:policy::AsyncPolicyProvider::InitWithLoopsReady
-}
-{
-   bug_141255_e
-   Heapcheck:Leak
-   fun:MessageLoop::MessageLoop
-   fun:ExtensionServiceTestBase::ExtensionServiceTestBase
-}
-{
-   bug_141255_f
-   Heapcheck:Leak
-   fun:MessageLoop::MessageLoop
-   fun:ExtensionUITest::ExtensionUITest
-   fun:ExtensionUITest_*
-}
-{
-   bug_141255_g
-   Heapcheck:Leak
-   fun:MessageLoop::MessageLoop
-   fun:MessageLoopForUI::MessageLoopForUI
-   fun:RenderViewHostTestHarness
-   fun:ChromeRenderViewHostTestHarness::ChromeRenderViewHostTestHarness
-   fun:TabContentsTestHarness::TabContentsTestHarness
-}
-{
-   bug_141255_h
-   Heapcheck:Leak
-   fun:MessageLoop::MessageLoop
-   fun:ExtensionServiceTestSimple_Enabledness_Test::TestBody
-}
-{
-   bug_141255_i
-   Heapcheck:Leak
-   fun:MessageLoop::MessageLoop
-   fun:ChromeLauncherControllerTest::ChromeLauncherControllerTest
-}
-{
-   bug_141255_j
-   Heapcheck:Leak
-   fun:MessageLoop::MessageLoop
-   fun:ExtensionUpdaterTest
-}
 {
    bug_141322
    Heapcheck:Leak

tools/valgrind/memcheck/suppressions.txt

@@ -5821,14 +5821,6 @@
    fun:_ZN12ReloadButtonC1EP15LocationBarViewP14CommandUpdater
    fun:_ZN16ReloadButtonTestC2Ev
 }
-{
-   bug_141255
-   Memcheck:Leak
-   ...
-   fun:_ZN6policy22BrowserPolicyConnector22CreatePlatformProviderEv
-   fun:_ZN6policy22BrowserPolicyConnector4InitEv
-   fun:_ZN21TestingBrowserProcess24browser_policy_connectorEv
-}
 
 #-----------------------------------------------------------------------
 # 4. These only occur on our Google workstations