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

chrome/browser/chromeos/settings/device_settings_provider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/settings/device_settings_provider.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/logging.h"
@@ skipping 37 matching lines @@
     kAccountsPrefDeviceLocalAccountPromptForNetworkWhenOffline,
     kAccountsPrefEphemeralUsersEnabled,
     kAccountsPrefShowUserNamesOnSignIn,
     kAccountsPrefSupervisedUsersEnabled,
     kAccountsPrefTransferSAMLCookies,
     kAccountsPrefUsers,
     kAllowRedeemChromeOsRegistrationOffers,
     kAllowedConnectionTypesForUpdate,
     kAttestationForContentProtectionEnabled,
     kDeviceAttestationEnabled,
+    kDeviceDisabled,
+    kDeviceDisabledMessage,
     kDeviceOwner,
+    kExtensionCacheSize,
     kHeartbeatEnabled,
     kHeartbeatFrequency,
     kPolicyMissingMitigationMode,
+    kRebootOnShutdown,
     kReleaseChannel,
     kReleaseChannelDelegated,
     kReportDeviceActivityTimes,
     kReportDeviceBootMode,
     kReportDeviceHardwareStatus,
     kReportDeviceLocation,
     kReportDeviceNetworkInterfaces,
     kReportDeviceSessionStatus,
     kReportDeviceUsers,
     kReportDeviceVersionInfo,
     kReportUploadFrequency,
     kServiceAccountIdentity,
     kSignedDataRoamingEnabled,
     kStartUpFlags,
     kStatsReportingPref,
     kSystemTimezonePolicy,
     kSystemUse24HourClock,
     kUpdateDisabled,
     kVariationsRestrictParameter,
-    kDeviceDisabled,
-    kDeviceDisabledMessage,
-    kRebootOnShutdown,
-    kExtensionCacheSize,
 };
 
 bool HasOldMetricsFile() {
   // TODO(pastarmovj): Remove this once migration is not needed anymore.
   // If the value is not set we should try to migrate legacy consent file.
   // Loading consent file state causes us to do blocking IO on UI thread.
   // Temporarily allow it until we fix http://crbug.com/62626
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   return GoogleUpdateSettings::GetCollectStatsConsent();
 }
@@ skipping 349 matching lines @@
 
 }  // namespace
 
 DeviceSettingsProvider::DeviceSettingsProvider(
     const NotifyObserversCallback& notify_cb,
     DeviceSettingsService* device_settings_service)
     : CrosSettingsProvider(notify_cb),
       device_settings_service_(device_settings_service),
       trusted_status_(TEMPORARILY_UNTRUSTED),
       ownership_status_(device_settings_service_->GetOwnershipStatus()),
+      trusted_for_test_(false),
       store_callback_factory_(this) {
   device_settings_service_->AddObserver(this);
   if (!UpdateFromService()) {
     // Make sure we have at least the cache data immediately.
     RetrieveCachedData();
   }
 }
 
 DeviceSettingsProvider::~DeviceSettingsProvider() {
   if (device_settings_service_->GetOwnerSettingsService())
@@ skipping 247 matching lines @@
       return value;
   } else {
     NOTREACHED() << "Trying to get non cros setting.";
   }
 
   return NULL;
 }
 
 DeviceSettingsProvider::TrustedStatus
     DeviceSettingsProvider::PrepareTrustedValues(const base::Closure& cb) {
+  if (trusted_for_test_)

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

Uh, why is this needed?

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

Because the first time the owner settings observers are called, they get
untrusted values, and those may then go on waiting for the settings to become
trusted some time in the future. And so the tests become broken, as right now
stub settings provider that's used in tests just always returns TRUSTED.

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

There are a number of tests that prepare device settings in their Setup() to get
them into trusted state. Can you point me at test tests that are broken?

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

Off the top of my head those would be HeartbeatSchedulerTest and
ShutdownPolicyHandlerTest, maybe some others.

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

I think it's fine to actually retain StubSettingsProvider, see other comment in
heartbeat_scheduler_unittest.cc

+    return TRUSTED;
   TrustedStatus status = RequestTrustedEntity();
   if (status == TEMPORARILY_UNTRUSTED && !cb.is_null())
     callbacks_.push_back(cb);
   return status;
 }
 
 bool DeviceSettingsProvider::HandlesSetting(const std::string& path) const {
   return IsDeviceSetting(path);
 }
 
@@ skipping 73 matching lines @@
 void DeviceSettingsProvider::AttemptMigration() {
   if (device_settings_service_->HasPrivateOwnerKey()) {
     PrefValueMap::const_iterator i;
     for (i = migration_values_.begin(); i != migration_values_.end(); ++i)
       DoSet(i->first, *i->second);
     migration_values_.Clear();
   }
 }
 
 }  // namespace chromeos