chromeos: Fix UpdateScreenTest and NetworkScreenTest for Debug build.

These tests have been failing since crrev.com/107314 due to the hacky
workaround I added.

Along the way, rework other tests to use MockDBusThreadManager. These
weren't failing but using the mock would make the tests faster and more
reliable.

For signed_settings_unittest.cc to pass, add expectations for StorePolicy().
We didn't need this before, as we were using stub implementation that
always ran callback.Run(true), but we are now using mocks, so we should
add this behavior manually.

BUG=chromium-os:22183
TEST=Run UpdateScreenTest, NetworkScreenTest, ExistingUserControllerTest, SignedSettingsHelperTest with the debug build.

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

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

7 files changed, 78 insertions, 69 deletions

diff --git a/chrome/browser/chromeos/dbus/dbus_thread_manager.cc b/chrome/browser/chromeos/dbus/dbus_thread_manager.cc
index 53ea24c..9d360f5 100644
--- a/chrome/browser/chromeos/dbus/dbus_thread_manager.cc
+++ b/chrome/browser/chromeos/dbus/dbus_thread_manager.cc
@@ -104,12 +104,6 @@ class DBusThreadManagerImpl : public DBusThreadManager {
     return speech_synthesizer_client_.get();
   }
 
-  // DBusThreadManager override.
-  virtual void set_session_manager_client_for_testing(
-      SessionManagerClient* session_manager_client) OVERRIDE {
-    session_manager_client_.reset(session_manager_client);
-  }
-
   scoped_ptr<base::Thread> dbus_thread_;
   scoped_refptr<dbus::Bus> system_bus_;
   scoped_ptr<CrosDBusService> cros_dbus_service_;
diff --git a/chrome/browser/chromeos/dbus/dbus_thread_manager.h b/chrome/browser/chromeos/dbus/dbus_thread_manager.h
index d3aae9b..fdac529 100644
--- a/chrome/browser/chromeos/dbus/dbus_thread_manager.h
+++ b/chrome/browser/chromeos/dbus/dbus_thread_manager.h
@@ -91,14 +91,6 @@ class DBusThreadManager {
   // down.
   virtual SessionManagerClient* session_manager_client() = 0;
 
-  // Sets the session manager client. Takes the ownership.
-  // The function is exported for testing.
-  //
-  // TODO(satorux): Remove this once we convert tests to use
-  // MockDBusThreadManager.
-  virtual void set_session_manager_client_for_testing(
-      SessionManagerClient* session_manager_client) = 0;
-
   // Returns the speech synthesizer client, owned by DBusThreadManager.
   // Do not cache this pointer and use it after DBusThreadManager is shut
   // down.
diff --git a/chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h b/chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h
index 66464785..91f1ef4 100644
--- a/chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h
+++ b/chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h
@@ -32,8 +32,6 @@ class MockDBusThreadManager : public DBusThreadManager {
   MOCK_METHOD0(power_manager_client, PowerManagerClient*(void));
   MOCK_METHOD0(sensors_client, SensorsClient*(void));
   MOCK_METHOD0(session_manager_client, SessionManagerClient*(void));
-  MOCK_METHOD1(set_session_manager_client_for_testing,
-               void(SessionManagerClient*));
   MOCK_METHOD0(speech_synthesizer_client, SpeechSynthesizerClient*(void));
 
   MockBluetoothAdapterClient* mock_bluetooth_adapter_client() {
diff --git a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
index bf9a5ea..0aabc15 100644
--- a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
@@ -8,7 +8,7 @@
 #include "chrome/browser/chromeos/cros/cros_in_process_browser_test.h"
 #include "chrome/browser/chromeos/cros/mock_cryptohome_library.h"
 #include "chrome/browser/chromeos/cros/mock_network_library.h"
-#include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
+#include "chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h"
 #include "chrome/browser/chromeos/dbus/mock_session_manager_client.h"
 #include "chrome/browser/chromeos/login/existing_user_controller.h"
 #include "chrome/browser/chromeos/login/helper.h"
@@ -104,7 +104,6 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
  protected:
   ExistingUserControllerTest()
       : mock_cryptohome_library_(NULL),
-        mock_session_manager_client_(NULL),
         mock_network_library_(NULL),
         mock_login_display_(NULL),
         mock_login_display_host_(NULL) {
@@ -115,18 +114,19 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
   }
 
   virtual void SetUpInProcessBrowserTestFixture() {
-    DBusThreadManager::Initialize();
+    MockDBusThreadManager* mock_dbus_thread_manager =
+        new MockDBusThreadManager;
+    DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
     CrosInProcessBrowserTest::SetUpInProcessBrowserTestFixture();
     cros_mock_->InitStatusAreaMocks();
     cros_mock_->SetStatusAreaMocksExpectations();
 
     mock_network_library_ = cros_mock_->mock_network_library();
-    mock_session_manager_client_ = new MockSessionManagerClient();
-    DBusThreadManager::Get()->set_session_manager_client_for_testing(
-        mock_session_manager_client_);
-    EXPECT_CALL(*mock_session_manager_client_, EmitLoginPromptReady())
+    MockSessionManagerClient* mock_session_manager_client =
+        mock_dbus_thread_manager->mock_session_manager_client();
+    EXPECT_CALL(*mock_session_manager_client, EmitLoginPromptReady())
         .Times(1);
-    EXPECT_CALL(*mock_session_manager_client_, RetrievePolicy(_))
+    EXPECT_CALL(*mock_session_manager_client, RetrievePolicy(_))
         .Times(AnyNumber());
 
     cros_mock_->InitMockCryptohomeLibrary();
@@ -165,7 +165,6 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
 
   // These mocks are owned by CrosLibrary class.
   MockCryptohomeLibrary* mock_cryptohome_library_;
-  MockSessionManagerClient* mock_session_manager_client_;
   MockNetworkLibrary* mock_network_library_;
 
   scoped_ptr<MockLoginDisplay> mock_login_display_;
diff --git a/chrome/browser/chromeos/login/network_screen_browsertest.cc b/chrome/browser/chromeos/login/network_screen_browsertest.cc
index a17745d..e01087d 100644
--- a/chrome/browser/chromeos/login/network_screen_browsertest.cc
+++ b/chrome/browser/chromeos/login/network_screen_browsertest.cc
@@ -10,7 +10,7 @@
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
-#include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
+#include "chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h"
 #include "chrome/browser/chromeos/dbus/mock_session_manager_client.h"
 #include "chrome/browser/chromeos/cros/mock_network_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
@@ -47,22 +47,22 @@ class DummyButtonListener : public views::ButtonListener {
 class NetworkScreenTest : public WizardInProcessBrowserTest {
  public:
   NetworkScreenTest(): WizardInProcessBrowserTest("network"),
-                       mock_session_manager_client_(NULL),
                        mock_network_library_(NULL) {
   }
 
  protected:
   virtual void SetUpInProcessBrowserTestFixture() {
-    DBusThreadManager::Initialize();
+    MockDBusThreadManager* mock_dbus_thread_manager =
+        new MockDBusThreadManager;
+    DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
     cros_mock_->InitStatusAreaMocks();
     mock_network_library_ = cros_mock_->mock_network_library();
-    mock_session_manager_client_ = new MockSessionManagerClient();
-    DBusThreadManager::Get()->set_session_manager_client_for_testing(
-        mock_session_manager_client_);
+    MockSessionManagerClient* mock_session_manager_client =
+        mock_dbus_thread_manager->mock_session_manager_client();
     cellular_.reset(new NetworkDevice("cellular"));
-    EXPECT_CALL(*mock_session_manager_client_, EmitLoginPromptReady())
+    EXPECT_CALL(*mock_session_manager_client, EmitLoginPromptReady())
         .Times(1);
-    EXPECT_CALL(*mock_session_manager_client_, RetrievePolicy(_))
+    EXPECT_CALL(*mock_session_manager_client, RetrievePolicy(_))
         .Times(AnyNumber());
 
     // Minimal set of expectations needed on NetworkScreen initialization.
@@ -148,13 +148,10 @@ class NetworkScreenTest : public WizardInProcessBrowserTest {
   }
 
   scoped_ptr<MockScreenObserver> mock_screen_observer_;
-  MockSessionManagerClient* mock_session_manager_client_;
   MockNetworkLibrary* mock_network_library_;
   scoped_ptr<NetworkDevice> cellular_;
   NetworkScreen* network_screen_;
   ViewsNetworkScreenActor* actor_;
-  // DBusThreadManager needs a message loop.
-  MessageLoopForUI message_loop_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkScreenTest);
diff --git a/chrome/browser/chromeos/login/signed_settings_unittest.cc b/chrome/browser/chromeos/login/signed_settings_unittest.cc
index b546dae..1f79d380 100644
--- a/chrome/browser/chromeos/login/signed_settings_unittest.cc
+++ b/chrome/browser/chromeos/login/signed_settings_unittest.cc
@@ -11,7 +11,7 @@
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/mock_library_loader.h"
 #include "chrome/browser/chromeos/cros_settings_names.h"
-#include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
+#include "chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h"
 #include "chrome/browser/chromeos/dbus/mock_session_manager_client.h"
 #include "chrome/browser/chromeos/login/mock_owner_key_utils.h"
 #include "chrome/browser/chromeos/login/mock_ownership_service.h"
@@ -109,14 +109,15 @@ class SignedSettingsTest : public testing::Test {
         ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE),
         mock_(new MockKeyUtils),
-        injector_(mock_) /* injector_ takes ownership of mock_ */ {
+        injector_(mock_) /* injector_ takes ownership of mock_ */,
+        mock_dbus_thread_manager_(new MockDBusThreadManager) {
   }
 
   virtual ~SignedSettingsTest() {}
 
   virtual void SetUp() {
     file_thread_.Start();
-    DBusThreadManager::Initialize();
+    DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager_);
   }
 
   virtual void TearDown() {
@@ -283,10 +284,15 @@ class SignedSettingsTest : public testing::Test {
 
   MockKeyUtils* mock_;
   MockInjector injector_;
+  MockDBusThreadManager* mock_dbus_thread_manager_;
 
   ScopedStubCrosEnabler stub_cros_enabler_;
 };
 
+ACTION_P(Retrieve, policy_blob) { arg0.Run(policy_blob); }
+ACTION_P(Store, success) { arg1.Run(success); }
+ACTION_P(FinishKeyOp, s) { arg2->OnKeyOpComplete(OwnerManager::SUCCESS, s); }
+
 TEST_F(SignedSettingsTest, CheckWhitelist) {
   NormalDelegate<bool> d(true);
   d.expect_success();
@@ -357,6 +363,12 @@ TEST_F(SignedSettingsTest, Whitelist) {
   em::PolicyData out_pol;
   ExpectWhitelistOp(s.get(), &in_pol, &out_pol);
 
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
+  EXPECT_CALL(*client, StorePolicy(_, _))
+      .WillOnce(Store(true))
+      .RetiresOnSaturation();
+
   s->Execute();
   s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>());
   message_loop_.RunAllPending();
@@ -374,6 +386,12 @@ TEST_F(SignedSettingsTest, AddToExistingWhitelist) {
   em::PolicyData out_pol;
   ExpectWhitelistOp(s.get(), &in_pol, &out_pol);
 
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
+  EXPECT_CALL(*client, StorePolicy(_, _))
+      .WillOnce(Store(true))
+      .RetiresOnSaturation();
+
   s->Execute();
   s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>());
   message_loop_.RunAllPending();
@@ -391,6 +409,12 @@ TEST_F(SignedSettingsTest, Unwhitelist) {
   em::PolicyData out_pol;
   ExpectWhitelistOp(s.get(), &in_pol, &out_pol);
 
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
+  EXPECT_CALL(*client, StorePolicy(_, _))
+      .WillOnce(Store(true))
+      .RetiresOnSaturation();
+
   s->Execute();
   s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>());
   message_loop_.RunAllPending();
@@ -410,6 +434,12 @@ TEST_F(SignedSettingsTest, RemoveFromExistingWhitelist) {
   em::PolicyData out_pol;
   ExpectWhitelistOp(s.get(), &in_pol, &out_pol);
 
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
+  EXPECT_CALL(*client, StorePolicy(_, _))
+      .WillOnce(Store(true))
+      .RetiresOnSaturation();
+
   s->Execute();
   s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>());
   message_loop_.RunAllPending();
@@ -438,6 +468,12 @@ TEST_F(SignedSettingsTest, StoreProperty) {
   EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>()))
       .WillOnce(SaveArg<0>(&out_pol));
 
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
+  EXPECT_CALL(*client, StorePolicy(_, _))
+      .WillOnce(Store(true))
+      .RetiresOnSaturation();
+
   s->Execute();
   s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>());
   message_loop_.RunAllPending();
@@ -513,10 +549,6 @@ TEST_F(SignedSettingsTest, RetrievePropertyNotFound) {
   message_loop_.RunAllPending();
 }
 
-ACTION_P(Retrieve, policy_blob) { arg0.Run(policy_blob); }
-ACTION_P(Store, success) { arg1.Run(success); }
-ACTION_P(FinishKeyOp, s) { arg2->OnKeyOpComplete(OwnerManager::SUCCESS, s); }
-
 TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) {
   NormalDelegate<std::string> d(fake_value_);
   d.expect_success();
@@ -529,8 +561,8 @@ TEST_F(SignedSettingsTest, RetrievePolicyToRetrieveProperty) {
   em::PolicyFetchResponse signed_policy = BuildProto(data,
                                                      fake_value_,
                                                      &signed_serialized);
-  MockSessionManagerClient* client = new MockSessionManagerClient;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, RetrievePolicy(_))
       .WillOnce(Retrieve(signed_serialized))
       .RetiresOnSaturation();
@@ -591,8 +623,8 @@ TEST_F(SignedSettingsTest, SignAndStorePolicy) {
   std::vector<uint8> fake_sig(fake_value_.c_str(),
                               fake_value_.c_str() + fake_value_.length());
 
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, StorePolicy(signed_serialized, _))
       .WillOnce(Store(true))
       .RetiresOnSaturation();
@@ -612,8 +644,8 @@ TEST_F(SignedSettingsTest, StoreSignedPolicy) {
                                                      &signed_serialized);
   scoped_refptr<SignedSettings> s(
       SignedSettings::CreateStorePolicyOp(&signed_policy, &d));
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, StorePolicy(signed_serialized, _))
       .WillOnce(Store(true))
       .RetiresOnSaturation();
@@ -661,8 +693,8 @@ TEST_F(SignedSettingsTest, RetrievePolicy) {
   d.expect_success();
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, RetrievePolicy(_))
       .WillOnce(Retrieve(signed_serialized))
       .RetiresOnSaturation();
@@ -689,8 +721,8 @@ TEST_F(SignedSettingsTest, RetrieveNullPolicy) {
   d.expect_failure(SignedSettings::NOT_FOUND);
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, RetrievePolicy(_))
       .WillOnce(Retrieve(""))
       .RetiresOnSaturation();
@@ -706,8 +738,8 @@ TEST_F(SignedSettingsTest, RetrieveEmptyPolicy) {
   d.expect_failure(SignedSettings::NOT_FOUND);
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, RetrievePolicy(_))
       .WillOnce(Retrieve(""))
       .RetiresOnSaturation();
@@ -725,8 +757,8 @@ TEST_F(SignedSettingsTest, RetrieveUnsignedPolicy) {
   d.expect_failure(SignedSettings::BAD_SIGNATURE);
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, RetrievePolicy(_))
       .WillOnce(Retrieve(serialized))
       .RetiresOnSaturation();
@@ -744,8 +776,8 @@ TEST_F(SignedSettingsTest, RetrieveMalsignedPolicy) {
   d.expect_failure(SignedSettings::BAD_SIGNATURE);
   scoped_refptr<SignedSettings> s(SignedSettings::CreateRetrievePolicyOp(&d));
 
-  MockSessionManagerClient* client = new MockSessionManagerClient;;
-  DBusThreadManager::Get()->set_session_manager_client_for_testing(client);
+  MockSessionManagerClient* client =
+      mock_dbus_thread_manager_->mock_session_manager_client();
   EXPECT_CALL(*client, RetrievePolicy(_))
       .WillOnce(Retrieve(signed_serialized))
       .RetiresOnSaturation();
diff --git a/chrome/browser/chromeos/login/update_screen_browsertest.cc b/chrome/browser/chromeos/login/update_screen_browsertest.cc
index a88e205..53f64bf 100644
--- a/chrome/browser/chromeos/login/update_screen_browsertest.cc
+++ b/chrome/browser/chromeos/login/update_screen_browsertest.cc
@@ -5,7 +5,7 @@
 #include "base/message_loop.h"
 #include "chrome/browser/chromeos/cros/mock_network_library.h"
 #include "chrome/browser/chromeos/cros/mock_update_library.h"
-#include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
+#include "chrome/browser/chromeos/dbus/mock_dbus_thread_manager.h"
 #include "chrome/browser/chromeos/dbus/mock_session_manager_client.h"
 #include "chrome/browser/chromeos/login/mock_screen_observer.h"
 #include "chrome/browser/chromeos/login/update_screen.h"
@@ -28,22 +28,22 @@ static void RequestUpdateCheckSuccess(UpdateCallback callback, void* userdata) {
 class UpdateScreenTest : public WizardInProcessBrowserTest {
  public:
   UpdateScreenTest() : WizardInProcessBrowserTest("update"),
-                       mock_session_manager_client_(NULL),
                        mock_update_library_(NULL),
                        mock_network_library_(NULL) {}
 
  protected:
   virtual void SetUpInProcessBrowserTestFixture() {
-    DBusThreadManager::Initialize();
+    MockDBusThreadManager* mock_dbus_thread_manager =
+        new MockDBusThreadManager;
+    DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
     WizardInProcessBrowserTest::SetUpInProcessBrowserTestFixture();
     cros_mock_->InitStatusAreaMocks();
     cros_mock_->SetStatusAreaMocksExpectations();
     ASSERT_TRUE(CrosLibrary::Get()->EnsureLoaded());
 
-    mock_session_manager_client_ = new MockSessionManagerClient();
-    DBusThreadManager::Get()->set_session_manager_client_for_testing(
-        mock_session_manager_client_);
-    EXPECT_CALL(*mock_session_manager_client_, EmitLoginPromptReady())
+    MockSessionManagerClient* mock_session_manager_client
+        = mock_dbus_thread_manager->mock_session_manager_client();
+    EXPECT_CALL(*mock_session_manager_client, EmitLoginPromptReady())
         .Times(1);
 
     mock_update_library_ = new MockUpdateLibrary();
@@ -90,14 +90,11 @@ class UpdateScreenTest : public WizardInProcessBrowserTest {
     DBusThreadManager::Shutdown();
   }
 
-  MockSessionManagerClient* mock_session_manager_client_;
   MockUpdateLibrary* mock_update_library_;
   MockNetworkLibrary* mock_network_library_;
 
   scoped_ptr<MockScreenObserver> mock_screen_observer_;
   UpdateScreen* update_screen_;
-  // DBusThreadManager needs a message loop.
-  MessageLoopForUI message_loop_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(UpdateScreenTest);