Add a Restore() method to ValueStore and make StorageAPI use it

chrome/browser/extensions/api/storage/settings_backend.h

Index: chrome/browser/extensions/api/storage/settings_backend.h
diff --git a/chrome/browser/extensions/api/storage/settings_backend.h b/chrome/browser/extensions/api/storage/settings_backend.h
index 0882b68ed3538996b8767075c802c5813026b9c3..39d82c6376cd31704c9476996ea4fbb5733f84c6 100644
--- a/chrome/browser/extensions/api/storage/settings_backend.h
+++ b/chrome/browser/extensions/api/storage/settings_backend.h
@@ -5,112 +5,56 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_STORAGE_SETTINGS_BACKEND_H_
 #define CHROME_BROWSER_EXTENSIONS_API_STORAGE_SETTINGS_BACKEND_H_
 
-#include <map>
-#include <set>
 #include <string>
 
-#include "base/compiler_specific.h"
 #include "base/files/file_path.h"
-#include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "chrome/browser/extensions/api/storage/settings_observer.h"
-#include "chrome/browser/extensions/api/storage/settings_storage_factory.h"
 #include "chrome/browser/extensions/api/storage/settings_storage_quota_enforcer.h"
-#include "sync/api/syncable_service.h"
 
 namespace syncer {
-class SyncErrorFactory;
+class SyncableService;
 }
 
-namespace extensions {
+class ValueStore;
 
-class SettingsSyncProcessor;
-class SyncableSettingsStorage;
+namespace extensions {
+class SettingsStorageFactory;
 
-// Manages ValueStore objects for extensions, including routing
-// changes from sync to them.
-// Lives entirely on the FILE thread.
-class SettingsBackend : public syncer::SyncableService {
+class SettingsBackend {
  public:
-  // |storage_factory| is use to create leveldb storage areas.
-  // |base_path| is the base of the extension settings directory, so the
-  // databases will be at base_path/extension_id.
-  // |observers| is the list of observers to settings changes.
-  SettingsBackend(
-      const scoped_refptr<SettingsStorageFactory>& storage_factory,
-      const base::FilePath& base_path,
-      syncer::ModelType sync_type,
-      const syncer::SyncableService::StartSyncFlare& flare,
-      const SettingsStorageQuotaEnforcer::Limits& quota,
-      const scoped_refptr<SettingsObserverList>& observers);
-
+  SettingsBackend(const scoped_refptr<SettingsStorageFactory>& storage_factory,
+                  const base::FilePath& base_path,
+                  const SettingsStorageQuotaEnforcer::Limits& quota);
   virtual ~SettingsBackend();
 
   // Gets a weak reference to the storage area for |extension_id|.
   // Must be run on the FILE thread.
-  ValueStore* GetStorage(const std::string& extension_id) const;
+  virtual ValueStore* GetStorage(const std::string& extension_id) = 0;
 
   // Deletes all setting data for an extension. Call on the FILE thread.
-  void DeleteStorage(const std::string& extension_id);
-
-  // syncer::SyncableService implementation.
-  virtual syncer::SyncDataList GetAllSyncData(
-      syncer::ModelType type) const OVERRIDE;
-  virtual syncer::SyncMergeResult MergeDataAndStartSyncing(
-      syncer::ModelType type,
-      const syncer::SyncDataList& initial_sync_data,
-      scoped_ptr<syncer::SyncChangeProcessor> sync_processor,
-      scoped_ptr<syncer::SyncErrorFactory> sync_error_factory) OVERRIDE;
-  virtual syncer::SyncError ProcessSyncChanges(
-      const tracked_objects::Location& from_here,
-      const syncer::SyncChangeList& change_list) OVERRIDE;
-  virtual void StopSyncing(syncer::ModelType type) OVERRIDE;
+  virtual void DeleteStorage(const std::string& extension_id) = 0;
 
- private:
-  // Gets a weak reference to the storage area for a given extension,
-  // initializing sync with some initial data if sync enabled.
-  SyncableSettingsStorage* GetOrCreateStorageWithSyncData(
-      const std::string& extension_id,
-      const base::DictionaryValue& sync_data) const;
-
-  // Gets all extension IDs known to extension settings.  This may not be all
-  // installed extensions.
-  std::set<std::string> GetKnownExtensionIDs() const;
+  // A slight hack so we can get a SyncableService from a SettingsBackend if
+  // it's actually a SyncStorageBackend. If called on a LocalStorageBackend,
+  // this asserts and returns null.
+  virtual syncer::SyncableService* GetAsSyncableService();
 
-  // Creates a new SettingsSyncProcessor for an extension.
-  scoped_ptr<SettingsSyncProcessor> CreateSettingsSyncProcessor(
-      const std::string& extension_id) const;
+  SettingsStorageFactory* storage_factory() const {
+    return storage_factory_.get();
+  }
+  const base::FilePath& base_path() const { return base_path_; }
+  const SettingsStorageQuotaEnforcer::Limits& quota() const { return quota_; }

[not at google - send to devlin, 2014/02/14 19:35:56]

It looks like these accessors can be protected, but, protected things imply
complex inheritance structure which is rarely a good thing.

That said, one of the purpose of these "storage factory" classes is to be able
to pull the implementation details of creating storage areas out of the
backends.

see comment in sync_storage_backend.cc about this.

[Devlin, 2014/02/18 23:55:22]

Done.

 
-  // The Factory to use for creating leveldb storage areas.
+ private:
+  // The Factory to use for creating new ValueStores.
   const scoped_refptr<SettingsStorageFactory> storage_factory_;
 
-  // The base file path to create any leveldb databases at.
+  // The base file path to use when creating new ValueStores.
   const base::FilePath base_path_;
 
   // Quota limits (see SettingsStorageQuotaEnforcer).
   const SettingsStorageQuotaEnforcer::Limits quota_;
 
-  // The list of observers to settings changes.
-  const scoped_refptr<SettingsObserverList> observers_;
-
-  // A cache of ValueStore objects that have already been created.
-  // Ensure that there is only ever one created per extension.
-  typedef std::map<std::string, linked_ptr<SyncableSettingsStorage> >
-      StorageObjMap;
-  mutable StorageObjMap storage_objs_;
-
-  // Current sync model type. Either EXTENSION_SETTINGS or APP_SETTINGS.
-  syncer::ModelType sync_type_;
-
-  // Current sync processor, if any.
-  scoped_ptr<syncer::SyncChangeProcessor> sync_processor_;
-
-  // Current sync error handler if any.
-  scoped_ptr<syncer::SyncErrorFactory> sync_error_factory_;
-
-  syncer::SyncableService::StartSyncFlare flare_;
-
   DISALLOW_COPY_AND_ASSIGN(SettingsBackend);
 };