Make CrosSettings and DeviceSettingsService non Lazy instances

chrome/browser/chromeos/settings/device_settings_test_helper.h

Index: chrome/browser/chromeos/settings/device_settings_test_helper.h
diff --git a/chrome/browser/chromeos/settings/device_settings_test_helper.h b/chrome/browser/chromeos/settings/device_settings_test_helper.h
index f19e271b681fc73b89728fee866706001690e75f..e3b03b4b00346d7a04cc4b8c6e06af38b7c9098c 100644
--- a/chrome/browser/chromeos/settings/device_settings_test_helper.h
+++ b/chrome/browser/chromeos/settings/device_settings_test_helper.h
@@ -15,6 +15,7 @@
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #include "chrome/browser/chromeos/policy/device_policy_builder.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/chromeos/settings/device_settings_test_helper.h"
 #include "chrome/browser/chromeos/settings/mock_owner_key_util.h"
@@ -125,6 +126,7 @@ class DeviceSettingsTestHelper : public SessionManagerClient {
 
   PolicyState device_policy_;
   std::map<std::string, PolicyState> device_local_account_policy_;
+  scoped_ptr<ScopedTestCrosSettings> test_cros_settings_;

[Mattias Nissler (ping if slow), 2013/04/16 16:09:51]

This file shouldn't have to depend on CrosSettings really.

[stevenjb, 2013/04/16 16:49:43]

Enough tests that rely on this also seem to rely on CrosSettings, so this was a
convenient place to put this, rather than each individual test.
That said, if you really don't like this here, I can remove it and track down
each dependency.

[stevenjb, 2013/04/16 17:07:41]

I'll go ahead and move this, I agree it is awkward here.

[Mattias Nissler (ping if slow), 2013/04/16 18:04:13]

Thanks!

 
   DISALLOW_COPY_AND_ASSIGN(DeviceSettingsTestHelper);
 };
@@ -167,7 +169,9 @@ class DeviceSettingsTestBase : public testing::Test {
 
   DeviceSettingsTestHelper device_settings_test_helper_;
   scoped_refptr<MockOwnerKeyUtil> owner_key_util_;
-  DeviceSettingsService device_settings_service_;
+
+  // Convenience pointer to DeviceSettingsService singleton.
+  DeviceSettingsService* device_settings_service_;

[Mattias Nissler (ping if slow), 2013/04/16 16:09:51]

The whole point of this class was to enable testing against a non-singleton
DeviceSettingsService instance. I'm not a fan of adding references to globals
all over the code.

[stevenjb, 2013/04/16 16:49:43]

The problem is, because it *is* a global singleton, we invariably end up
accessing it in test code whether we expect to or not, which means we have two
instances, causing all sorts of potential trouble. I prefer to control the
global singleton for the test.

[Mattias Nissler (ping if slow), 2013/04/16 18:04:13]

Per offline discussion, the plan is to not initialize the global pointer in
tests so access to hidden globals is flagged to people adding new tests.

 
  private:
   DISALLOW_COPY_AND_ASSIGN(DeviceSettingsTestBase);