Sanitize PrefStore interface.

This reworks the PrefStore interface, specifically:

- Split up the interface into PrefStore, which only provides reading functionality, and the derived PersistentPrefStore for the actual user pref store
- Remove the hurt-me-plenty prefs() function from PrefStore, instead provide Get/Set/Remove operations
- Remove special handling of default and user pref store from PrefValueStore and put it into PrefService
- Pref change notification handling now almost exclusively handled through PrefValueStore
- Adjust all consumers of these interfaces (but keep ConfigurationPolicyPrefStore untouched, that's up next on the list)

BUG=64232
TEST=existing unit tests

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

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

6 files changed, 146 insertions, 41 deletions

diff --git a/chrome/service/cloud_print/cloud_print_proxy.cc b/chrome/service/cloud_print/cloud_print_proxy.cc
index 4141b2d..d2fb73c 100644
--- a/chrome/service/cloud_print/cloud_print_proxy.cc
+++ b/chrome/service/cloud_print/cloud_print_proxy.cc
@@ -10,10 +10,10 @@
 #include "base/values.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
-#include "chrome/common/json_pref_store.h"
 #include "chrome/service/cloud_print/cloud_print_consts.h"
 #include "chrome/service/cloud_print/print_system.h"
 #include "chrome/service/service_process.h"
+#include "chrome/service/service_process_prefs.h"
 
 // This method is invoked on the IO thread to launch the browser process to
 // display a desktop notification that the Cloud Print token is invalid and
@@ -48,7 +48,8 @@ CloudPrintProxy::~CloudPrintProxy() {
   Shutdown();
 }
 
-void CloudPrintProxy::Initialize(JsonPrefStore* service_prefs, Client* client) {
+void CloudPrintProxy::Initialize(ServiceProcessPrefs* service_prefs,
+                                 Client* client) {
   DCHECK(CalledOnValidThread());
   service_prefs_ = service_prefs;
   client_ = client;
@@ -60,22 +61,22 @@ void CloudPrintProxy::EnableForUser(const std::string& lsid) {
     return;
 
   std::string proxy_id;
-  service_prefs_->prefs()->GetString(prefs::kCloudPrintProxyId, &proxy_id);
+  service_prefs_->GetString(prefs::kCloudPrintProxyId, &proxy_id);
   if (proxy_id.empty()) {
     proxy_id = cloud_print::PrintSystem::GenerateProxyId();
-    service_prefs_->prefs()->SetString(prefs::kCloudPrintProxyId, proxy_id);
+    service_prefs_->SetString(prefs::kCloudPrintProxyId, proxy_id);
     service_prefs_->WritePrefs();
   }
 
   // Getting print system specific settings from the preferences.
   DictionaryValue* print_system_settings = NULL;
-  service_prefs_->prefs()->GetDictionary(prefs::kCloudPrintPrintSystemSettings,
-                                         &print_system_settings);
+  service_prefs_->GetDictionary(prefs::kCloudPrintPrintSystemSettings,
+                                &print_system_settings);
 
   // Check if there is an override for the cloud print server URL.
   std::string cloud_print_server_url_str;
-  service_prefs_->prefs()->GetString(prefs::kCloudPrintServiceURL,
-                                     &cloud_print_server_url_str);
+  service_prefs_->GetString(prefs::kCloudPrintServiceURL,
+                            &cloud_print_server_url_str);
   if (cloud_print_server_url_str.empty()) {
     cloud_print_server_url_str = kDefaultCloudPrintServerUrl;
   }
@@ -90,15 +91,15 @@ void CloudPrintProxy::EnableForUser(const std::string& lsid) {
     backend_->InitializeWithLsid(lsid, proxy_id);
   } else {
     std::string cloud_print_token;
-    service_prefs_->prefs()->GetString(prefs::kCloudPrintAuthToken,
-                                       &cloud_print_token);
+    service_prefs_->GetString(prefs::kCloudPrintAuthToken,
+                              &cloud_print_token);
     DCHECK(!cloud_print_token.empty());
     std::string cloud_print_xmpp_token;
-    service_prefs_->prefs()->GetString(prefs::kCloudPrintXMPPAuthToken,
-                                       &cloud_print_xmpp_token);
+    service_prefs_->GetString(prefs::kCloudPrintXMPPAuthToken,
+                              &cloud_print_xmpp_token);
     DCHECK(!cloud_print_xmpp_token.empty());
-    service_prefs_->prefs()->GetString(prefs::kCloudPrintEmail,
-                                       &cloud_print_email_);
+    service_prefs_->GetString(prefs::kCloudPrintEmail,
+                              &cloud_print_email_);
     DCHECK(!cloud_print_email_.empty());
     backend_->InitializeWithToken(cloud_print_token, cloud_print_xmpp_token,
                                   cloud_print_email_, proxy_id);
@@ -125,13 +126,6 @@ bool CloudPrintProxy::IsEnabled(std::string* email) const {
   return enabled;
 }
 
-void CloudPrintProxy::Shutdown() {
-  DCHECK(CalledOnValidThread());
-  if (backend_.get())
-    backend_->Shutdown();
-  backend_.reset();
-}
-
 // Notification methods from the backend. Called on UI thread.
 void CloudPrintProxy::OnPrinterListAvailable(
     const printing::PrinterList& printer_list) {
@@ -147,11 +141,12 @@ void CloudPrintProxy::OnAuthenticated(
     const std::string& email) {
   DCHECK(CalledOnValidThread());
   cloud_print_email_ = email;
-  service_prefs_->prefs()->SetString(prefs::kCloudPrintAuthToken,
-                                     cloud_print_token);
-  service_prefs_->prefs()->SetString(prefs::kCloudPrintXMPPAuthToken,
-                                     cloud_print_xmpp_token);
-  service_prefs_->prefs()->SetString(prefs::kCloudPrintEmail, email);
+  service_prefs_->SetString(prefs::kCloudPrintAuthToken,
+                            cloud_print_token);
+  service_prefs_->SetString(prefs::kCloudPrintXMPPAuthToken,
+                            cloud_print_xmpp_token);
+  service_prefs_->SetString(prefs::kCloudPrintEmail,
+                            email);
   service_prefs_->WritePrefs();
 }
 
@@ -164,3 +159,10 @@ void CloudPrintProxy::OnAuthenticationFailed() {
   g_service_process->io_thread()->message_loop_proxy()->PostTask(
       FROM_HERE, NewRunnableFunction(&ShowTokenExpiredNotificationInBrowser));
 }
+
+void CloudPrintProxy::Shutdown() {
+  DCHECK(CalledOnValidThread());
+  if (backend_.get())
+    backend_->Shutdown();
+  backend_.reset();
+}
diff --git a/chrome/service/cloud_print/cloud_print_proxy.h b/chrome/service/cloud_print/cloud_print_proxy.h
index 36399ca..a5832f2 100644
--- a/chrome/service/cloud_print/cloud_print_proxy.h
+++ b/chrome/service/cloud_print/cloud_print_proxy.h
@@ -13,7 +13,7 @@
 #include "base/scoped_ptr.h"
 #include "chrome/service/cloud_print/cloud_print_proxy_backend.h"
 
-class JsonPrefStore;
+class ServiceProcessPrefs;
 
 // CloudPrintProxy is the layer between the service process UI thread
 // and the cloud print proxy backend.
@@ -31,7 +31,7 @@ class CloudPrintProxy : public CloudPrintProxyFrontend,
 
   // Initializes the object. This should be called every time an object of this
   // class is constructed.
-  void Initialize(JsonPrefStore* service_prefs, Client* client);
+  void Initialize(ServiceProcessPrefs* service_prefs, Client* client);
 
   // Enables/disables cloud printing for the user
   void EnableForUser(const std::string& lsid);
@@ -60,7 +60,7 @@ class CloudPrintProxy : public CloudPrintProxyFrontend,
   scoped_ptr<CloudPrintProxyBackend> backend_;
   // This class does not own this. It is guaranteed to remain valid for the
   // lifetime of this class.
-  JsonPrefStore* service_prefs_;
+  ServiceProcessPrefs* service_prefs_;
   // This class does not own this. If non-NULL, It is guaranteed to remain
   // valid for the lifetime of this class.
   Client* client_;
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc
index e826c1f..aa91ca8 100644
--- a/chrome/service/service_process.cc
+++ b/chrome/service/service_process.cc
@@ -16,11 +16,11 @@
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/json_pref_store.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/service_process_util.h"
 #include "chrome/service/cloud_print/cloud_print_proxy.h"
 #include "chrome/service/service_ipc_server.h"
+#include "chrome/service/service_process_prefs.h"
 #include "net/base/network_change_notifier.h"
 
 #if defined(ENABLE_REMOTING)
@@ -95,11 +95,10 @@ bool ServiceProcess::Initialize(MessageLoop* message_loop,
   FilePath user_data_dir;
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   FilePath pref_path = user_data_dir.Append(chrome::kServiceStateFileName);
-  service_prefs_.reset(new JsonPrefStore(pref_path,
-                                         file_thread_->message_loop_proxy()));
+  service_prefs_.reset(
+      new ServiceProcessPrefs(pref_path, file_thread_->message_loop_proxy()));
   service_prefs_->ReadPrefs();
 
-  DictionaryValue* values = service_prefs_->prefs();
   bool remoting_host_enabled = false;
 
   // For development, we allow forcing the enabling of the host daemon via a
@@ -107,7 +106,8 @@ bool ServiceProcess::Initialize(MessageLoop* message_loop,
   //
   // TODO(ajwong): When we've gotten the preference setting workflow more
   // stable, we should remove the command-line flag force-enable.
-  values->GetBoolean(prefs::kRemotingHostEnabled, &remoting_host_enabled);
+  service_prefs_->GetBoolean(prefs::kRemotingHostEnabled,
+                             &remoting_host_enabled);
   remoting_host_enabled |= command_line.HasSwitch(switches::kEnableRemoting);
 
 #if defined(ENABLE_REMOTING)
@@ -122,8 +122,8 @@ bool ServiceProcess::Initialize(MessageLoop* message_loop,
       command_line.HasSwitch(switches::kEnableCloudPrintProxy);
   if (!cloud_print_proxy_enabled) {
     // Then check if the cloud print proxy was previously enabled.
-    values->GetBoolean(prefs::kCloudPrintProxyEnabled,
-                       &cloud_print_proxy_enabled);
+    service_prefs_->GetBoolean(prefs::kCloudPrintProxyEnabled,
+                               &cloud_print_proxy_enabled);
   }
 
   if (cloud_print_proxy_enabled) {
@@ -192,14 +192,14 @@ CloudPrintProxy* ServiceProcess::GetCloudPrintProxy() {
 
 void ServiceProcess::OnCloudPrintProxyEnabled() {
   // Save the preference that we have enabled the cloud print proxy.
-  service_prefs_->prefs()->SetBoolean(prefs::kCloudPrintProxyEnabled, true);
+  service_prefs_->SetBoolean(prefs::kCloudPrintProxyEnabled, true);
   service_prefs_->WritePrefs();
   OnServiceEnabled();
 }
 
 void ServiceProcess::OnCloudPrintProxyDisabled() {
   // Save the preference that we have disabled the cloud print proxy.
-  service_prefs_->prefs()->SetBoolean(prefs::kCloudPrintProxyEnabled, false);
+  service_prefs_->SetBoolean(prefs::kCloudPrintProxyEnabled, false);
   service_prefs_->WritePrefs();
   OnServiceDisabled();
 }
@@ -311,7 +311,7 @@ void ServiceProcess::OnRemotingHostAdded() {
   talk_token_ = "";
 
   // Save the preference that we have enabled the remoting host.
-  service_prefs_->prefs()->SetBoolean(prefs::kRemotingHostEnabled, true);
+  service_prefs_->SetBoolean(prefs::kRemotingHostEnabled, true);
 
   // Force writing prefs to the disk.
   service_prefs_->WritePrefs();
diff --git a/chrome/service/service_process.h b/chrome/service/service_process.h
index 91d51a0..230cf77 100644
--- a/chrome/service/service_process.h
+++ b/chrome/service/service_process.h
@@ -16,7 +16,7 @@
 #include "chrome/service/cloud_print/cloud_print_proxy.h"
 #include "chrome/service/remoting/remoting_directory_service.h"
 
-class JsonPrefStore;
+class ServiceProcessPrefs;
 class ServiceIPCServer;
 
 namespace net {
@@ -152,7 +152,7 @@ class ServiceProcess : public RemotingDirectoryService::Client,
   scoped_ptr<base::Thread> io_thread_;
   scoped_ptr<base::Thread> file_thread_;
   scoped_ptr<CloudPrintProxy> cloud_print_proxy_;
-  scoped_ptr<JsonPrefStore> service_prefs_;
+  scoped_ptr<ServiceProcessPrefs> service_prefs_;
   scoped_ptr<ServiceIPCServer> ipc_server_;
 
 #if defined(ENABLE_REMOTING)
diff --git a/chrome/service/service_process_prefs.cc b/chrome/service/service_process_prefs.cc
new file mode 100644
index 0000000..182b92f
--- /dev/null
+++ b/chrome/service/service_process_prefs.cc
@@ -0,0 +1,54 @@
+// Copyright (c) 2010 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/service/service_process_prefs.h"
+
+#include "base/values.h"
+
+ServiceProcessPrefs::ServiceProcessPrefs(
+    const FilePath& pref_filename,
+    base::MessageLoopProxy* file_message_loop_proxy)
+    : prefs_(pref_filename, file_message_loop_proxy) {
+}
+
+void ServiceProcessPrefs::ReadPrefs() {
+  prefs_.ReadPrefs();
+}
+
+void ServiceProcessPrefs::WritePrefs() {
+  prefs_.WritePrefs();
+}
+
+void ServiceProcessPrefs::GetString(const std::string& key,
+                                    std::string* result) {
+  Value* value;
+  if (prefs_.GetValue(key, &value) == PersistentPrefStore::READ_OK)
+    value->GetAsString(result);
+}
+
+void ServiceProcessPrefs::SetString(const std::string& key,
+                                    const std::string& value) {
+  prefs_.SetValue(key, Value::CreateStringValue(value));
+}
+
+void ServiceProcessPrefs::GetBoolean(const std::string& key, bool* result) {
+  Value* value;
+  if (prefs_.GetValue(key, &value) == PersistentPrefStore::READ_OK)
+    value->GetAsBoolean(result);
+}
+
+void ServiceProcessPrefs::SetBoolean(const std::string& key, bool value) {
+  prefs_.SetValue(key, Value::CreateBooleanValue(value));
+}
+
+void ServiceProcessPrefs::GetDictionary(const std::string& key,
+                                        DictionaryValue** result) {
+  Value* value;
+  if (prefs_.GetValue(key, &value) != PersistentPrefStore::READ_OK ||
+      !value->IsType(Value::TYPE_DICTIONARY)) {
+    return;
+  }
+
+  *result = static_cast<DictionaryValue*>(value);
+}
diff --git a/chrome/service/service_process_prefs.h b/chrome/service/service_process_prefs.h
new file mode 100644
index 0000000..ce86b03
--- /dev/null
+++ b/chrome/service/service_process_prefs.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2010 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_SERVICE_SERVICE_PROCESS_PREFS_H_
+#define CHROME_SERVICE_SERVICE_PROCESS_PREFS_H_
+#pragma once
+
+#include <string>
+
+#include "chrome/common/json_pref_store.h"
+
+// Manages persistent preferences for the service process. This is basically a
+// thin wrapper around JsonPrefStore for more comfortable use.
+class ServiceProcessPrefs {
+ public:
+  // |file_message_loop_proxy| is the MessageLoopProxy for a thread on which
+  // file I/O can be done.
+  ServiceProcessPrefs(const FilePath& pref_filename,
+                      base::MessageLoopProxy* file_message_loop_proxy);
+
+  // Read preferences from the backing file.
+  void ReadPrefs();
+
+  // Write the data to the backing file.
+  void WritePrefs();
+
+  // Get a string preference for |key| and store it in |result|.
+  void GetString(const std::string& key, std::string* result);
+
+  // Set a string |value| for |key|.
+  void SetString(const std::string& key, const std::string& value);
+
+  // Get a boolean preference for |key| and store it in |result|.
+  void GetBoolean(const std::string& key, bool* result);
+
+  // Set a boolean |value| for |key|.
+  void SetBoolean(const std::string& key, bool value);
+
+  // Get a dictionary preference for |key| and store it in |result|.
+  void GetDictionary(const std::string& key, DictionaryValue** result);
+
+ private:
+  JsonPrefStore prefs_;
+
+  DISALLOW_COPY_AND_ASSIGN(ServiceProcessPrefs);
+};
+
+#endif  // CHROME_SERVICE_SERVICE_PROCESS_PREFS_H_