[fsp] Separate logic for saving/restoring state to a separate class.

Previously, the Service class was responsible for saving and restoring state,
as well as managing file systems. Testing it became problematic.

This patch separates it into two classes Service and Registry, which can
be both tested separately.

TBR=pam@chromium.org
TEST=unit_tests: *FileSystemProvider*Registry*
BUG=248427

Review URL: https://codereview.chromium.org/661393002

Cr-Commit-Position: refs/heads/master@{#300422}

chrome/browser/chromeos/file_system_provider/provided_file_system.cc

@@ -476,7 +476,7 @@ bool ProvidedFileSystem::Notify(
   FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
                     observers_,
                     OnObservedEntryChanged(file_system_info_,
-                                           observed_path,
+                                           it->second,
                                            change_type,
                                            child_changes_ref,
                                            auto_updater->CreateCallback()));
@@ -549,10 +549,9 @@ void ProvidedFileSystem::OnNotifyCompleted(
 
   it->second.last_tag = tag;
 
-  FOR_EACH_OBSERVER(
-      ProvidedFileSystemObserver,
-      observers_,
-      OnObservedEntryTagUpdated(file_system_info_, observed_path, tag));
+  FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
+                    observers_,
+                    OnObservedEntryTagUpdated(file_system_info_, it->second));
 
   // If the observed entry is deleted, then unobserve it.
   if (change_type == ProvidedFileSystemObserver::DELETED)

chrome/browser/chromeos/file_system_provider/provided_file_system_info.h

@@ -20,7 +20,6 @@ struct MountOptions {
   MountOptions(const std::string& file_system_id,
                const std::string& display_name);
 
-  std::string extension_id;
   std::string file_system_id;
   std::string display_name;
   bool writable;

chrome/browser/chromeos/file_system_provider/provided_file_system_observer.h

@@ -46,7 +46,7 @@ class ProvidedFileSystemObserver {
   // |callback|.
   virtual void OnObservedEntryChanged(
       const ProvidedFileSystemInfo& file_system_info,
-      const base::FilePath& observed_path,
+      const ObservedEntry& observed_entry,
       ChangeType change_type,
       const ChildChanges& child_changes,
       const base::Closure& callback) = 0;
@@ -54,8 +54,7 @@ class ProvidedFileSystemObserver {
   // Called after the tag value is updated for the observed entry.
   virtual void OnObservedEntryTagUpdated(
       const ProvidedFileSystemInfo& file_system_info,
-      const base::FilePath& observed_path,
-      const std::string& tag) = 0;
+      const ObservedEntry& observed_entry) = 0;
 
   // Called after the list of observed entries is changed.
   virtual void OnObservedEntryListChanged(

chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc

@@ -124,7 +124,7 @@ class Observer : public ProvidedFileSystemObserver {
   // ProvidedFileSystemInterfaceObserver overrides.
   virtual void OnObservedEntryChanged(
       const ProvidedFileSystemInfo& file_system_info,
-      const base::FilePath& observed_path,
+      const ObservedEntry& observed_entry,
       ProvidedFileSystemObserver::ChangeType change_type,
       const ProvidedFileSystemObserver::ChildChanges& child_changes,
       const base::Closure& callback) override {
@@ -135,8 +135,7 @@ class Observer : public ProvidedFileSystemObserver {
 
   virtual void OnObservedEntryTagUpdated(
       const ProvidedFileSystemInfo& file_system_info,
-      const base::FilePath& observed_path,
-      const std::string& tag) override {
+      const ObservedEntry& observed_entry) override {
     EXPECT_EQ(kFileSystemId, file_system_info.file_system_id());
     ++tag_updated_counter_;
   }

chrome/browser/chromeos/file_system_provider/registry.h

+// Copyright 2014 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_FILE_SYSTEM_PROVIDER_REGISTRY_H_
+#define CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_REGISTRY_H_
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/chromeos/file_system_provider/observed_entry.h"
+#include "chrome/browser/chromeos/file_system_provider/registry_interface.h"
+
+class Profile;
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}  // namespace user_prefs
+
+namespace chromeos {
+namespace file_system_provider {
+
+// Key names for preferences.
+extern const char kPrefKeyFileSystemId[];
+extern const char kPrefKeyDisplayName[];
+extern const char kPrefKeyWritable[];
+extern const char kPrefKeySupportsNotifyTag[];
+extern const char kPrefKeyObservedEntries[];
+extern const char kPrefKeyObservedEntryEntryPath[];
+extern const char kPrefKeyObservedEntryRecursive[];
+extern const char kPrefKeyObservedEntryLastTag[];
+
+class ProvidedFileSystemInfo;
+
+// Registers preferences to remember registered file systems between reboots.
+void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
+// Remembers and restores file systems in a persistent storage.
+class Registry : public RegistryInterface {
+ public:
+  explicit Registry(Profile* profile);
+  virtual ~Registry();
+
+  // RegistryInterface overrides.
+  virtual void RememberFileSystem(
+      const ProvidedFileSystemInfo& file_system_info,
+      const ObservedEntries& observed_entries) override;
+  virtual void ForgetFileSystem(const std::string& extension_id,
+                                const std::string& file_system_id) override;
+  virtual scoped_ptr<RestoredFileSystems> RestoreFileSystems(
+      const std::string& extension_id) override;
+  virtual void UpdateObservedEntryTag(
+      const ProvidedFileSystemInfo& file_system_info,
+      const ObservedEntry& observed_entry) override;
+
+ private:
+  Profile* profile_;  // Not owned.
+  DISALLOW_COPY_AND_ASSIGN(Registry);
+};
+
+}  // namespace file_system_provider
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_REGISTRY_H_

chrome/browser/chromeos/file_system_provider/registry_interface.cc

+// Copyright 2014 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/browser/chromeos/file_system_provider/registry_interface.h"
+
+namespace chromeos {
+namespace file_system_provider {
+
+RegistryInterface::~RegistryInterface() {
+}
+
+RegistryInterface::RestoredFileSystem::RestoredFileSystem() {
+}
+
+RegistryInterface::RestoredFileSystem::~RestoredFileSystem() {
+}
+
+}  // namespace file_system_provider
+}  // namespace chromeos

chrome/browser/chromeos/file_system_provider/registry_interface.h

+// Copyright 2014 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_FILE_SYSTEM_PROVIDER_REGISTRY_INTERFACE_H_
+#define CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_REGISTRY_INTERFACE_H_
+
+#include <string>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/chromeos/file_system_provider/observed_entry.h"
+#include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
+
+namespace chromeos {
+namespace file_system_provider {
+
+// Remembers and restores file systems in a persistent storage.
+class RegistryInterface {
+ public:
+  struct RestoredFileSystem;
+
+  // List of file systems together with their observed entries to be remounted.
+  typedef std::vector<RestoredFileSystem> RestoredFileSystems;
+
+  // Information about a file system to be restored.
+  struct RestoredFileSystem {
+    RestoredFileSystem();
+    ~RestoredFileSystem();
+
+    std::string extension_id;
+    MountOptions options;
+    ObservedEntries observed_entries;
+  };
+
+  virtual ~RegistryInterface();
+
+  // Remembers the file system in preferences, in order to remount after a
+  // reboot.
+  virtual void RememberFileSystem(
+      const ProvidedFileSystemInfo& file_system_info,
+      const ObservedEntries& observed_entries) = 0;
+
+  // Removes the file system from preferences, so it is not remounmted anymore
+  // after a reboot.
+  virtual void ForgetFileSystem(const std::string& extension_id,
+                                const std::string& file_system_id) = 0;
+
+  // Restores from preferences file systems mounted previously by the
+  // |extension_id| providing extension. The returned list should be used to
+  // remount them.
+  virtual scoped_ptr<RestoredFileSystems> RestoreFileSystems(
+      const std::string& extension_id) = 0;
+
+  // Updates a tag for the specified observed entry.
+  virtual void UpdateObservedEntryTag(
+      const ProvidedFileSystemInfo& file_system_info,
+      const ObservedEntry& observed_entry) = 0;
+};
+
+}  // namespace file_system_provider
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_REGISTRY_INTERFACE_H_

chrome/browser/chromeos/file_system_provider/service.h

@@ -39,19 +39,10 @@ class PrefRegistrySyncable;
 namespace chromeos {
 namespace file_system_provider {
 
-// Key names for preferences.
-extern const char kPrefKeyFileSystemId[];
-extern const char kPrefKeyDisplayName[];
-extern const char kPrefKeyWritable[];
-extern const char kPrefKeySupportsNotifyTag[];
-extern const char kPrefKeyObservedEntries[];
-extern const char kPrefKeyObservedEntryEntryPath[];
-extern const char kPrefKeyObservedEntryRecursive[];
-extern const char kPrefKeyObservedEntryLastTag[];
-
 class ProvidedFileSystemFactoryInterface;
 class ProvidedFileSystemInfo;
 class ProvidedFileSystemInterface;
+class RegistryInterface;
 class ServiceFactory;
 struct MountOptions;
 
@@ -82,6 +73,9 @@ class Service : public KeyedService,
   void SetFileSystemFactoryForTesting(
       const FileSystemFactoryCallback& factory_callback);
 
+  // Sets a custom Registry implementation. Used by unit tests.
+  void SetRegistryForTesting(scoped_ptr<RegistryInterface> registry);
+
   // Mounts a file system provided by an extension with the |extension_id|. If
   // |writable| is set to true, then the file system is mounted in a R/W mode.
   // Otherwise, only read-only operations are supported. If change notification
@@ -137,14 +131,13 @@ class Service : public KeyedService,
   // ProvidedFileSystemInterface::Observer overrides.
   virtual void OnObservedEntryChanged(
       const ProvidedFileSystemInfo& file_system_info,
-      const base::FilePath& observed_path,
+      const ObservedEntry& observed_entry,
       ProvidedFileSystemObserver::ChangeType change_type,
       const ProvidedFileSystemObserver::ChildChanges& child_changes,
       const base::Closure& callback) override;
   virtual void OnObservedEntryTagUpdated(
       const ProvidedFileSystemInfo& file_system_info,
-      const base::FilePath& observed_path,
-      const std::string& tag) override;
+      const ObservedEntry& observed_entry) override;
   virtual void OnObservedEntryListChanged(
       const ProvidedFileSystemInfo& file_system_info,
       const ObservedEntries& observed_entries) override;
@@ -185,9 +178,10 @@ class Service : public KeyedService,
   ObserverList<Observer> observers_;
   ProvidedFileSystemMap file_system_map_;  // Owns pointers.
   MountPointNameToKeyMap mount_point_name_to_key_map_;
+  scoped_ptr<RegistryInterface> registry_;
   base::ThreadChecker thread_checker_;
-  base::WeakPtrFactory<Service> weak_ptr_factory_;
 
+  base::WeakPtrFactory<Service> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(Service);
 };
 

chrome/browser/prefs/browser_prefs.cc

@@ -155,7 +155,7 @@
 #include "chrome/browser/chromeos/customization_document.h"
 #include "chrome/browser/chromeos/display/display_preferences.h"
 #include "chrome/browser/chromeos/extensions/echo_private_api.h"
-#include "chrome/browser/chromeos/file_system_provider/service.h"
+#include "chrome/browser/chromeos/file_system_provider/registry.h"
 #include "chrome/browser/chromeos/first_run/first_run.h"
 #include "chrome/browser/chromeos/login/default_pinned_apps_field_trial.h"
 #include "chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.h"

chrome/chrome_browser_chromeos.gypi

@@ -327,6 +327,10 @@
         'browser/chromeos/file_system_provider/provided_file_system_interface.h',
         'browser/chromeos/file_system_provider/provided_file_system_observer.cc',
         'browser/chromeos/file_system_provider/provided_file_system_observer.h',
+        'browser/chromeos/file_system_provider/registry.cc',
+        'browser/chromeos/file_system_provider/registry.h',
+        'browser/chromeos/file_system_provider/registry_interface.cc',
+        'browser/chromeos/file_system_provider/registry_interface.h',
         'browser/chromeos/file_system_provider/request_manager.cc',
         'browser/chromeos/file_system_provider/request_manager.h',
         'browser/chromeos/file_system_provider/request_value.cc',

chrome/chrome_tests_unit.gypi

@@ -177,6 +177,7 @@
       'browser/chromeos/file_system_provider/operations/unobserve_entry_unittest.cc',
       'browser/chromeos/file_system_provider/operations/write_file_unittest.cc',
       'browser/chromeos/file_system_provider/provided_file_system_unittest.cc',
+      'browser/chromeos/file_system_provider/registry_unittest.cc',
       'browser/chromeos/file_system_provider/request_manager_unittest.cc',
       'browser/chromeos/file_system_provider/service_unittest.cc',
       'browser/chromeos/fileapi/external_file_url_request_job_unittest.cc',