Merge policy provider objects into singleton where possible

BUG=50623
TEST=ConfigurationPolicyPrefStoreTest.*

Review URL: http://codereview.chromium.org/3072009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56016 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 78 insertions, 43 deletions

diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc
index 744ee70..f606708 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/path_service.h"
+#include "base/singleton.h"
 #include "base/string16.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ -24,6 +25,67 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 
+namespace {
+
+// Manages the lifecycle of the shared platform-specific policy providers
+// for managed and recommended policy. Instantiated as a Singleton.
+class ConfigurationPolicyProviderKeeper {
+ public:
+  ConfigurationPolicyProviderKeeper()
+      : managed_provider_(CreateManagedProvider()),
+        recommended_provider_(CreateRecommendedProvider()) {
+  }
+  virtual ~ConfigurationPolicyProviderKeeper() {}
+
+  ConfigurationPolicyProvider* managed_provider() const {
+    return managed_provider_.get();
+  }
+
+  ConfigurationPolicyProvider* recommended_provider() const {
+    return recommended_provider_.get();
+  }
+
+ private:
+  scoped_ptr<ConfigurationPolicyProvider> managed_provider_;
+  scoped_ptr<ConfigurationPolicyProvider> recommended_provider_;
+
+  static ConfigurationPolicyProvider* CreateManagedProvider() {
+#if defined(OS_WIN)
+    return new ConfigurationPolicyProviderWin();
+#elif defined(OS_MACOSX)
+    return new ConfigurationPolicyProviderMac();
+#elif defined(OS_POSIX)
+    FilePath config_dir_path;
+    if (PathService::Get(chrome::DIR_POLICY_FILES, &config_dir_path)) {
+      return new ConfigDirPolicyProvider(
+          config_dir_path.Append(FILE_PATH_LITERAL("managed")));
+    } else {
+      return new DummyConfigurationPolicyProvider();
+    }
+#else
+    return new DummyConfigurationPolicyProvider();
+#endif
+  }
+
+  static ConfigurationPolicyProvider* CreateRecommendedProvider() {
+#if defined(OS_POSIX) && !defined(OS_MACOSX)
+    FilePath config_dir_path;
+    if (PathService::Get(chrome::DIR_POLICY_FILES, &config_dir_path)) {
+      return new ConfigDirPolicyProvider(
+          config_dir_path.Append(FILE_PATH_LITERAL("recommended")));
+    } else {
+      return new DummyConfigurationPolicyProvider();
+    }
+#else
+    return new DummyConfigurationPolicyProvider();
+#endif
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProviderKeeper);
+};
+
+}
+
 const ConfigurationPolicyPrefStore::PolicyToPreferenceMapEntry
     ConfigurationPolicyPrefStore::simple_policy_map_[] = {
   { Value::TYPE_STRING, kPolicyHomePage,  prefs::kHomePage },
@@ -112,50 +174,24 @@ PrefStore::PrefReadError ConfigurationPolicyPrefStore::ReadPrefs() {
   proxy_configuration_specified_ = false;
   command_line_proxy_settings_cleared_ = false;
 
-  return (provider_.get() == NULL || provider_->Provide(this)) ?
+  return (provider_ == NULL || provider_->Provide(this)) ?
       PrefStore::PREF_READ_ERROR_NONE : PrefStore::PREF_READ_ERROR_OTHER;
 }
 
 // static
 ConfigurationPolicyPrefStore*
 ConfigurationPolicyPrefStore::CreateManagedPolicyPrefStore() {
-  ConfigurationPolicyProvider* provider;
-#if defined(OS_WIN)
-  provider = new ConfigurationPolicyProviderWin();
-#elif defined(OS_MACOSX)
-  provider = new ConfigurationPolicyProviderMac();
-#elif defined(OS_POSIX)
-  FilePath config_dir_path;
-  if (PathService::Get(chrome::DIR_POLICY_FILES, &config_dir_path))
-    provider = new ConfigDirPolicyProvider(
-        config_dir_path.Append(FILE_PATH_LITERAL("managed")));
-  else
-    provider = new DummyConfigurationPolicyProvider();
-#else
-  provider = new DummyConfigurationPolicyProvider();
-#endif
-
   return new ConfigurationPolicyPrefStore(CommandLine::ForCurrentProcess(),
-                                          provider);
+      Singleton<ConfigurationPolicyProviderKeeper>::get()->managed_provider());
 }
 
 // static
 ConfigurationPolicyPrefStore*
 ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore() {
-  ConfigurationPolicyProvider* provider;
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-  FilePath config_dir_path;
-  if (PathService::Get(chrome::DIR_POLICY_FILES, &config_dir_path))
-    provider = new ConfigDirPolicyProvider(
-        config_dir_path.Append(FILE_PATH_LITERAL("recommended")));
-  else
-    provider = new DummyConfigurationPolicyProvider();
-#else
-  provider = new DummyConfigurationPolicyProvider();
-#endif
-
+  ConfigurationPolicyProviderKeeper* manager =
+      Singleton<ConfigurationPolicyProviderKeeper>::get();
   return new ConfigurationPolicyPrefStore(CommandLine::ForCurrentProcess(),
-                                          provider);
+      manager->recommended_provider());
 }
 
 bool ConfigurationPolicyPrefStore::ApplyProxyPolicy(PolicyType policy,
diff --git a/chrome/browser/policy/configuration_policy_pref_store.h b/chrome/browser/policy/configuration_policy_pref_store.h
index a214ea2..252614b 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.h
+++ b/chrome/browser/policy/configuration_policy_pref_store.h
@@ -23,8 +23,8 @@ class CommandLine;
 class ConfigurationPolicyPrefStore : public PrefStore,
                                      public ConfigurationPolicyStore {
  public:
-  // The ConfigurationPolicyPrefStore takes the ownership of the passed
-  // |provider|.
+  // The ConfigurationPolicyPrefStore does not take ownership of the
+  // passed-in |provider|.
   ConfigurationPolicyPrefStore(const CommandLine* command_line,
                                ConfigurationPolicyProvider* provider);
   virtual ~ConfigurationPolicyPrefStore() { }
@@ -56,7 +56,7 @@ class ConfigurationPolicyPrefStore : public PrefStore,
   static const PolicyToPreferenceMapEntry proxy_policy_map_[];
 
   const CommandLine* command_line_;
-  scoped_ptr<ConfigurationPolicyProvider> provider_;
+  ConfigurationPolicyProvider* provider_;
   scoped_ptr<DictionaryValue> prefs_;
 
   // Set to false until the first proxy-relevant policy is applied. Allows
diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
index ecb8fc4..3f74266 100644
--- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
@@ -246,8 +246,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyProxyConfigManualOverride) {
   provider->AddPolicy(ConfigurationPolicyStore::kPolicyProxyBypassList,
       Value::CreateStringValue("http://chromium.org/override"));
 
-  ConfigurationPolicyPrefStore store(&command_line,
-                                     provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   EXPECT_EQ(store.ReadPrefs(), PrefStore::PREF_READ_ERROR_NONE);
 
   // Ensure that all traces of the command-line specified proxy
@@ -278,7 +277,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyProxyConfigNoProxy) {
       Value::CreateIntegerValue(
           ConfigurationPolicyStore::kPolicyNoProxyServerMode));
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   EXPECT_EQ(store.ReadPrefs(), PrefStore::PREF_READ_ERROR_NONE);
 
   std::string string_result;
@@ -307,7 +306,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest,
   provider->AddPolicy(ConfigurationPolicyStore::kPolicyProxyBypassList,
       Value::CreateStringValue("http://chromium.org/override"));
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   EXPECT_EQ(store.ReadPrefs(), PrefStore::PREF_READ_ERROR_NONE);
 
   std::string string_result;
@@ -335,7 +334,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyProxyConfigAutoDetect) {
       Value::CreateIntegerValue(
           ConfigurationPolicyStore::kPolicyAutoDetectProxyMode));
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   EXPECT_EQ(store.ReadPrefs(), PrefStore::PREF_READ_ERROR_NONE);
 
   // Ensure that all traces of the command-line specified proxy
@@ -366,7 +365,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyProxyConfiguseSystem) {
       Value::CreateIntegerValue(
           ConfigurationPolicyStore::kPolicyUseSystemProxyMode));
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   EXPECT_EQ(store.ReadPrefs(), PrefStore::PREF_READ_ERROR_NONE);
 
   std::string string_result;
@@ -394,7 +393,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest,
   provider->AddPolicy(ConfigurationPolicyStore::kPolicyProxyBypassList,
       Value::CreateStringValue("http://chromium.org/override"));
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   EXPECT_EQ(store.ReadPrefs(), PrefStore::PREF_READ_ERROR_NONE);
 
   std::string string_result;
@@ -417,7 +416,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest,
   scoped_ptr<MockConfigurationPolicyProvider> provider(
       new MockConfigurationPolicyProvider());
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   ApplyStringPolicyValue(&store,
       ConfigurationPolicyStore::kPolicyDisabledPlugins,
       "plugin1");
@@ -445,7 +444,7 @@ TEST_F(ConfigurationPolicyPrefStoreTest,
   scoped_ptr<MockConfigurationPolicyProvider> provider(
      new MockConfigurationPolicyProvider());
 
-  ConfigurationPolicyPrefStore store(&command_line, provider.release());
+  ConfigurationPolicyPrefStore store(&command_line, provider.get());
   ApplyStringPolicyValue(&store,
                          ConfigurationPolicyStore::kPolicyDisabledPlugins,
                          "plugin1,plugin2\\,");