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

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_;

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.

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() {

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_;

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);

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();

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);