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

chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc

Index: chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
diff --git a/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc b/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
index 48a81cb418a7dca2df0e0da89efb161a3cb84440..37228b0f27d3741f87fc9a8ad0b7ea4821c35ead 100644
--- a/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
+++ b/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
@@ -175,6 +175,8 @@ bool CheckManagementModeTransition(policy::ManagementMode current_mode,
   return false;
 }
 
+static bool g_test_owner_settings = false;
+
 }  // namespace
 
 OwnerSettingsServiceChromeOS::ManagementSettings::ManagementSettings() {
@@ -637,6 +639,60 @@ void OwnerSettingsServiceChromeOS::UpdateDeviceSettings(
     } else {
       NOTREACHED();
     }
+  } else if (g_test_owner_settings && path == kExtensionCacheSize) {

[Mattias Nissler (ping if slow), 2015/03/27 12:42:06]

yuck - why is this needed?

[Ivan Podogov, 2015/03/27 12:51:14]

You may see below a list of settings that are not really meant to be set by
user, hence those are read-only. But we need a way to specify them in tests.
Until now, it was by using CrosSettings::Set(), which is not the way to go.
Another approach would be using the (stub) DeviceSettingsProvider::Set(), which
might also become deprecated pretty quickly... This seemed like a fair
compromise.

[Mattias Nissler (ping if slow), 2015/03/27 12:58:52]

Which are these tests? Can't we inject the necessary settings by manipulating
the underlying device settings (assuming these tests use a production
CrosSettings instance now)?

[Ivan Podogov, 2015/03/27 13:10:50]

In order of the settings listed here (their names basically speak for
themselves):
chrome/browser/chromeos/policy/extension_cache_unittest.cc (this one was
actually added by me, so as far as I understand we may remove
"g_test_owner_settings &&" from this one safely)
chrome/browser/chromeos/attestation/attestation_policy_observer_unittest.cc
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api_unittest.cc
chrome/browser/chromeos/policy/heartbeat_scheduler_unittest.cc
chrome/browser/chromeos/settings/shutdown_policy_handler_unittest.cc
chrome/browser/chromeos/policy/status_uploader_unittest.cc

[Mattias Nissler (ping if slow), 2015/03/27 13:45:58]

Looked at this - I think the correct approach is to either rely on
StubCrosSettingsProvider and use a Set method (which is fine) or if the test
uses DeviceSettingsTestHelper, update the device settings under the hood via the
test helper and then call ReloadDeviceSettings().

+    em::ExtensionCacheSizeProto* extension_cache_size =
+        settings.mutable_extension_cache_size();
+    int cache_size;
+    if (value.GetAsInteger(&cache_size)) {
+      extension_cache_size->set_extension_cache_size(cache_size);
+    } else {
+      NOTREACHED();
+    }
+  } else if (g_test_owner_settings && path == kDeviceAttestationEnabled) {
+    em::AttestationSettingsProto* attestation_settings =
+        settings.mutable_attestation_settings();
+    bool setting_enabled;
+    if (value.GetAsBoolean(&setting_enabled)) {
+      attestation_settings->set_attestation_enabled(setting_enabled);
+    } else {
+      NOTREACHED();
+    }
+  } else if (g_test_owner_settings && path == kHeartbeatEnabled) {
+    em::DeviceHeartbeatSettingsProto* heartbeat_settings =
+        settings.mutable_device_heartbeat_settings();
+    bool setting_enabled;
+    if (value.GetAsBoolean(&setting_enabled)) {
+      heartbeat_settings->set_heartbeat_enabled(setting_enabled);
+    } else {
+      NOTREACHED();
+    }
+  } else if (g_test_owner_settings && path == kHeartbeatFrequency) {
+    em::DeviceHeartbeatSettingsProto* heartbeat_settings =
+        settings.mutable_device_heartbeat_settings();
+    int heartbeat_frequency;
+    if (value.GetAsInteger(&heartbeat_frequency)) {
+      heartbeat_settings->set_heartbeat_frequency(heartbeat_frequency);
+    } else {
+      NOTREACHED();
+    }
+  } else if (g_test_owner_settings && path == kRebootOnShutdown) {
+    em::RebootOnShutdownProto* reboot_on_shutdown =
+        settings.mutable_reboot_on_shutdown();
+    bool setting_enabled;
+    if (value.GetAsBoolean(&setting_enabled)) {
+      reboot_on_shutdown->set_reboot_on_shutdown(setting_enabled);
+    } else {
+      NOTREACHED();
+    }
+  } else if (g_test_owner_settings && path == kReportUploadFrequency) {
+    em::DeviceReportingProto* reporting_policy =
+        settings.mutable_device_reporting();
+    int upload_frequency;
+    if (value.GetAsInteger(&upload_frequency)) {
+      reporting_policy->set_device_status_frequency(upload_frequency);
+    } else {
+      NOTREACHED();
+    }
   } else {
     // The remaining settings don't support Set(), since they are not
     // intended to be customizable by the user:
@@ -778,4 +834,12 @@ void OwnerSettingsServiceChromeOS::ReportStatusAndContinueStoring(
   StorePendingChanges();
 }
 
+ScopedTestOwnerSettings::ScopedTestOwnerSettings() {
+  g_test_owner_settings = true;
+}
+
+ScopedTestOwnerSettings::~ScopedTestOwnerSettings() {
+  g_test_owner_settings = false;
+}
+
 }  // namespace chromeos