Switch to direct use of OwnerSettingsServiceChromeOS::Set() in tests.

chrome/browser/chromeos/login/kiosk_browsertest.cc

Index: chrome/browser/chromeos/login/kiosk_browsertest.cc
diff --git a/chrome/browser/chromeos/login/kiosk_browsertest.cc b/chrome/browser/chromeos/login/kiosk_browsertest.cc
index 890a1b7f17fad43bc683b178fb587a483f8c27d0..f74aeeb3f9b69ecbdd1fedba96e624ed5ec2fb0a 100644
--- a/chrome/browser/chromeos/login/kiosk_browsertest.cc
+++ b/chrome/browser/chromeos/login/kiosk_browsertest.cc
@@ -35,13 +35,15 @@
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
 #include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
+#include "chrome/browser/chromeos/ownership/fake_owner_settings_service.h"
+#include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/policy/device_policy_cros_browser_test.h"
-#include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile_impl.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -403,74 +405,14 @@ class AppDataLoadWaiter : public KioskAppManagerObserver {
   DISALLOW_COPY_AND_ASSIGN(AppDataLoadWaiter);
 };
 
-class CrosSettingsPermanentlyUntrustedMaker :
-    public DeviceSettingsService::Observer {
- public:
-  CrosSettingsPermanentlyUntrustedMaker();
-
-  // DeviceSettingsService::Observer:
-  void OwnershipStatusChanged() override;
-  void DeviceSettingsUpdated() override;
-  void OnDeviceSettingsServiceShutdown() override;
-
- private:
-  bool untrusted_check_running_;
-  base::RunLoop run_loop_;
-
-  void CheckIfUntrusted();
-
-  DISALLOW_COPY_AND_ASSIGN(CrosSettingsPermanentlyUntrustedMaker);
-};
-
-CrosSettingsPermanentlyUntrustedMaker::CrosSettingsPermanentlyUntrustedMaker()
-    : untrusted_check_running_(false) {
-  DeviceSettingsService::Get()->AddObserver(this);
-
-  policy::DevicePolicyCrosTestHelper().InstallOwnerKey();
-  DeviceSettingsService::Get()->OwnerKeySet(true);
-
-  run_loop_.Run();
-}
-
-void CrosSettingsPermanentlyUntrustedMaker::OwnershipStatusChanged() {
-  if (untrusted_check_running_)
-    return;
-
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&CrosSettingsPermanentlyUntrustedMaker::CheckIfUntrusted,
-                 base::Unretained(this)));
-}
-
-void CrosSettingsPermanentlyUntrustedMaker::DeviceSettingsUpdated() {
-}
-
-void CrosSettingsPermanentlyUntrustedMaker::OnDeviceSettingsServiceShutdown() {
-}
-
-void CrosSettingsPermanentlyUntrustedMaker::CheckIfUntrusted() {
-  untrusted_check_running_ = true;
-  const CrosSettingsProvider::TrustedStatus trusted_status =
-      CrosSettings::Get()->PrepareTrustedValues(
-          base::Bind(&CrosSettingsPermanentlyUntrustedMaker::CheckIfUntrusted,
-                     base::Unretained(this)));
-  if (trusted_status == CrosSettingsProvider::TEMPORARILY_UNTRUSTED)
-    return;
-  untrusted_check_running_ = false;
-
-  if (trusted_status == CrosSettingsProvider::TRUSTED)
-    return;
-
-  DeviceSettingsService::Get()->RemoveObserver(this);
-  run_loop_.Quit();
-}
-
 }  // namespace
 
 class KioskTest : public OobeBaseTest {
  public:
-  KioskTest() : use_consumer_kiosk_mode_(true),
-                fake_cws_(new FakeCWS) {
+  KioskTest()
+      : settings_helper_(false),
+        use_consumer_kiosk_mode_(true),
+        fake_cws_(new FakeCWS) {
     set_exit_when_last_browser_closes(false);
   }
 
@@ -500,9 +442,13 @@ class KioskTest : public OobeBaseTest {
     // Needed to avoid showing Gaia screen instead of owner signin for
     // consumer network down test cases.
     StartupUtils::MarkDeviceRegistered(base::Closure());
+    settings_helper_.ReplaceProvider(kAccountsPrefDeviceLocalAccounts);
+    owner_settings_service_.reset(settings_helper_.CreateOwnerSettingsService(
+        ProfileManager::GetPrimaryUserProfile()));
   }
 
   void TearDownOnMainThread() override {
+    settings_helper_.RestoreProvider();
     AppLaunchController::SetNetworkTimeoutCallbackForTesting(NULL);
     AppLaunchSigninScreen::SetUserManagerForTesting(NULL);
 
@@ -530,8 +476,9 @@ class KioskTest : public OobeBaseTest {
     SetupTestAppUpdateCheck();
 
     // Remove then add to ensure NOTIFICATION_KIOSK_APPS_LOADED fires.
-    KioskAppManager::Get()->RemoveApp(test_app_id_);
-    KioskAppManager::Get()->AddApp(test_app_id_);
+    KioskAppManager::Get()->RemoveApp(test_app_id_,
+                                      owner_settings_service_.get());
+    KioskAppManager::Get()->AddApp(test_app_id_, owner_settings_service_.get());
   }
 
   void FireKioskAppSettingsChanged() {
@@ -548,8 +495,9 @@ class KioskTest : public OobeBaseTest {
   void ReloadAutolaunchKioskApps() {
     SetupTestAppUpdateCheck();
 
-    KioskAppManager::Get()->AddApp(test_app_id_);
-    KioskAppManager::Get()->SetAutoLaunchApp(test_app_id_);
+    KioskAppManager::Get()->AddApp(test_app_id_, owner_settings_service_.get());
+    KioskAppManager::Get()->SetAutoLaunchApp(test_app_id_,
+                                             owner_settings_service_.get());
   }
 
   void StartUIForAppLaunch() {
@@ -793,6 +741,9 @@ class KioskTest : public OobeBaseTest {
     use_consumer_kiosk_mode_ = use;
   }
 
+  ScopedCrosSettingsTestHelper settings_helper_;
+  scoped_ptr<FakeOwnerSettingsService> owner_settings_service_;
+
  private:
   bool use_consumer_kiosk_mode_;
   std::string test_app_id_;
@@ -1207,7 +1158,8 @@ IN_PROC_BROWSER_TEST_F(KioskTest, DoNotLaunchWhenUntrusted) {
   SimulateNetworkOnline();
 
   // Make cros settings untrusted.
-  CrosSettingsPermanentlyUntrustedMaker();
+  settings_helper_.SetTrustedStatus(
+      CrosSettingsProvider::PERMANENTLY_UNTRUSTED);
 
   // Check that the attempt to start a kiosk app fails with an error.
   LaunchApp(test_app_id(), false);
@@ -1246,7 +1198,8 @@ IN_PROC_BROWSER_TEST_F(KioskTest, NoConsumerAutoLaunchWhenUntrusted) {
       base::FundamentalValue(true));
 
   // Make cros settings untrusted.
-  CrosSettingsPermanentlyUntrustedMaker();
+  settings_helper_.SetTrustedStatus(
+      CrosSettingsProvider::PERMANENTLY_UNTRUSTED);
 
   // Check that the attempt to auto-launch a kiosk app fails with an error.
   OobeScreenWaiter(OobeDisplay::SCREEN_ERROR_MESSAGE).Wait();
@@ -1259,7 +1212,8 @@ IN_PROC_BROWSER_TEST_F(KioskTest, NoEnterpriseAutoLaunchWhenUntrusted) {
   SimulateNetworkOnline();
 
   // Make cros settings untrusted.
-  CrosSettingsPermanentlyUntrustedMaker();
+  settings_helper_.SetTrustedStatus(
+      CrosSettingsProvider::PERMANENTLY_UNTRUSTED);
 
   // Trigger the code that handles auto-launch on enterprise devices. This would
   // normally be called from ShowLoginWizard(), which runs so early that it is
@@ -1291,7 +1245,15 @@ class KioskUpdateTest : public KioskTest {
     KioskTest::TearDown();
   }
 
-  void SetUpOnMainThread() override { KioskTest::SetUpOnMainThread(); }
+  void SetUpOnMainThread() override {
+    settings_helper_.CopyStoredValue(kAccountsPrefDeviceLocalAccounts);

[Mattias Nissler (ping if slow), 2015/04/08 11:38:17]

Can you please put a comment here explaining why this is needed, also below in
TearDownOnMainThread()?

Also, it's a bit weird to rely on the actual device settings implementation to
persist the value across PRE_ test phases, even though you've mocked out the
owner settings side of things. Why not just have TearDownOnMainThread() capture
the value and re-inject it for the next SetUpOnMainThread() as long as we're in
the same test group?

[Ivan Podogov, 2015/04/08 11:59:42]

Because the values are stored so that they would be used long before the next
SetUpOnMainThread() actually happens: the default CrosSettings provider is
created and used earlier, by KioskAppManager::UpdateAppData, somewhere around
its creation. If I don't have the values ready by that time (and I don't really
have control over that, since it would require changing the base classes'
behavior), then the external extension cache will be empty on creation, and the
whole PRE part (which only does app caching) becomes meaningless, and the test
breaks.

Another (and arguably better) way to do this would be by moving the default
settings provider creation out of the CrosSettings constructor (it's somewhat
clumsy right now) by setting some global variable, to be able to inject a custom
provider right from the test's constructor (or, better yet, from the
ScopedCrosSettingsTestHelper's constructor).

Still, I would require something to store my settings to a file in
TearDownOnMainThread, or maybe still use StoreCachedDeviceSetting, as I do now,
just by reading those values from the cache in constructor.

+    KioskTest::SetUpOnMainThread();
+  }
+
+  void TearDownOnMainThread() override {
+    settings_helper_.StoreCachedDeviceSetting(kAccountsPrefDeviceLocalAccounts);
+    KioskTest::TearDownOnMainThread();
+  }
 
   void PreCacheApp(const std::string& app_id,
                    const std::string& version,
@@ -1733,8 +1695,9 @@ class KioskEnterpriseTest : public KioskTest {
   }
 
   void SetUpInProcessBrowserTestFixture() override {
-    device_policy_test_helper_.MarkAsEnterpriseOwned();
-    device_policy_test_helper_.InstallOwnerKey();
+    policy::DevicePolicyCrosTestHelper::MarkAsEnterpriseOwnedBy(
+        kTestOwnerEmail);
+    settings_helper_.SetCurrentUserIsOwner(false);
 
     KioskTest::SetUpInProcessBrowserTestFixture();
   }
@@ -1777,41 +1740,21 @@ class KioskEnterpriseTest : public KioskTest {
     base::RunLoop().RunUntilIdle();
   }
 
-  static void StorePolicyCallback(const base::Closure& callback, bool result) {
-    ASSERT_TRUE(result);
-    callback.Run();
-  }
-
   void ConfigureKioskAppInPolicy(const std::string& account_id,
                                  const std::string& app_id,
                                  const std::string& update_url) {
-    em::DeviceLocalAccountsProto* accounts =
-        device_policy_test_helper_.device_policy()->payload()
-            .mutable_device_local_accounts();
-    em::DeviceLocalAccountInfoProto* account = accounts->add_account();
-    account->set_account_id(account_id);
-    account->set_type(
-        em::DeviceLocalAccountInfoProto::ACCOUNT_TYPE_KIOSK_APP);
-    account->mutable_kiosk_app()->set_app_id(app_id);
-    if (!update_url.empty())
-      account->mutable_kiosk_app()->set_update_url(update_url);
-    accounts->set_auto_login_id(account_id);
-    em::PolicyData& policy_data =
-        device_policy_test_helper_.device_policy()->policy_data();
-    policy_data.set_service_account_identity(kTestEnterpriseServiceAccountId);
-    device_policy_test_helper_.device_policy()->Build();
-
-    base::RunLoop run_loop;
-    DBusThreadManager::Get()->GetSessionManagerClient()->StoreDevicePolicy(
-        device_policy_test_helper_.device_policy()->GetBlob(),
-        base::Bind(&KioskEnterpriseTest::StorePolicyCallback,
-                   run_loop.QuitClosure()));
-    run_loop.Run();
-
-    DeviceSettingsService::Get()->Load();
-  }
-
-  policy::DevicePolicyCrosTestHelper device_policy_test_helper_;
+    settings_helper_.SetCurrentUserIsOwner(true);
+    std::vector<policy::DeviceLocalAccount> accounts;
+    accounts.push_back(
+        policy::DeviceLocalAccount(policy::DeviceLocalAccount::TYPE_KIOSK_APP,
+                                   account_id, app_id, update_url));
+    policy::SetDeviceLocalAccounts(owner_settings_service_.get(), accounts);
+    settings_helper_.SetString(kAccountsPrefDeviceLocalAccountAutoLoginId,
+                               account_id);
+    settings_helper_.SetString(kServiceAccountIdentity,
+                               kTestEnterpriseServiceAccountId);
+    settings_helper_.SetCurrentUserIsOwner(false);
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(KioskEnterpriseTest);