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

chrome/browser/chromeos/settings/stub_cros_settings_provider.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_SETTINGS_STUB_CROS_SETTINGS_PROVIDER_H_
 #define CHROME_BROWSER_CHROMEOS_SETTINGS_STUB_CROS_SETTINGS_PROVIDER_H_
 
 #include <string>
 
 #include "base/prefs/pref_value_map.h"
 #include "chromeos/settings/cros_settings_provider.h"
 
 namespace chromeos {
 
 // CrosSettingsProvider implementation that stores settings in memory unsigned.
 class StubCrosSettingsProvider : public CrosSettingsProvider {
  public:
   explicit StubCrosSettingsProvider(const NotifyObserversCallback& notify_cb);
   StubCrosSettingsProvider();
   ~StubCrosSettingsProvider() override;
 
   // CrosSettingsProvider implementation.
   const base::Value* Get(const std::string& path) const override;
   TrustedStatus PrepareTrustedValues(const base::Closure& callback) override;
   bool HandlesSetting(const std::string& path) const override;
 
+  void SetTrustedStatus(TrustedStatus status);
+  void SetCurrentUserIsOwner(bool owner);
+
  private:
   // CrosSettingsProvider implementation:
   void DoSet(const std::string& path, const base::Value& value) override;
 
   // Initializes settings to their defaults.
   void SetDefaults();
 
   // In-memory settings storage.
   PrefValueMap values_;
 
+  // By default, every value from Set is written. However, in some cases (e.g.

[bartfab (slow), 2015/04/10 08:38:13]

I find this comment very difficult to parse. Rewording it would help. However, I
also think that the functionality you describe here is fishy: You are abusing
Set() as a non-owner to not set but send out a notification only. If you just
want to send a notification, you should add a testing method that triggers the
notification directly.

[Ivan Podogov, 2015/04/10 09:50:53]

This behavior is actually something that some tests depend on, so I left it that
way.

[bartfab (slow), 2015/04/14 14:10:09]

This still looks very hacky. Anyone reading such a test will have no way to know
why the value passed is not actually being applied. Maybe you could at least add
a check that verifies the value being passed in matches the current value? That
way, if someone tries to modify such a test to use Set() for changing values,
they will get a CHECK() and will be pointed at this comment?

[Ivan Podogov, 2015/04/14 14:40:37]

This is actually a mimic of the current DeviceSettingsProvider::DoSet behavior
(right in the beginnning of it). The hacky thing is that KioskTest actually
depends on this behavior (and tries to set different values, so the CHECK
wouldn't help). That test should probably be rewritten from scratch anyway, but
for now that is not the task that I'm trying to accomplish...

As a compromise, I've added a warning log message (again, like in
DeviceSettingsProvider::DoSet) for such cases, so that the tests would still
work and the test author would have at least some information.

+  // enterprise-managed) it is required to retain current settings, but notify
+  // about the attempted change. That's what happens when the current user is
+  // not the owner.
+  bool current_user_is_owner_;

[bartfab (slow), 2015/04/10 08:38:14]

Nit: Initialize this.

[Ivan Podogov, 2015/04/10 09:50:53]

Done.

+
+  // Default value is TRUSTED.
+  TrustedStatus trusted_status_;

[bartfab (slow), 2015/04/10 08:38:14]

Nit: Initialize this.

[Ivan Podogov, 2015/04/10 09:50:53]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(StubCrosSettingsProvider);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_SETTINGS_STUB_CROS_SETTINGS_PROVIDER_H_