Create ExtensionProvider per each extension.

To match semantics of ExtensionProvider.

Test: All current FSP tests pass.
Bug: 790836
Change-Id: I6948086c234319031a4ae95664d9216c2fb5f55c
Reviewed-on: https://chromium-review.googlesource.com/802836
Commit-Queue: Tomasz Mikolajewski <mtomasz@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521608}

chrome/browser/chromeos/file_manager/volume_manager_unittest.cc

@@ -6,7 +6,9 @@
 
 #include <stddef.h>
 
+#include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/macros.h"
@@ -16,7 +18,6 @@
 #include "chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager_observer.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_extension_provider.h"
-#include "chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
@@ -182,9 +183,6 @@ class VolumeManagerTest : public testing::Test {
               file_system_provider_service_.get(),
               base::Bind(&ProfileEnvironment::GetFakeMtpStorageInfo,
                          base::Unretained(this)))) {
-      file_system_provider_service_->SetExtensionProviderForTesting(
-          std::make_unique<
-              chromeos::file_system_provider::FakeExtensionProvider>());
     }
 
     Profile* profile() const { return profile_.get(); }

chrome/browser/chromeos/file_system_provider/extension_provider.cc

@@ -11,7 +11,6 @@
 
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system.h"
-#include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/chromeos/file_system_provider/throttled_file_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "extensions/browser/extension_registry.h"
@@ -23,12 +22,10 @@ namespace {
 
 // Returns boolean indicating success. result->capabilities contains the
 // capabilites of the extension.
-bool GetProvidingExtensionInfo(const std::string& extension_id,
+bool GetProvidingExtensionInfo(const extensions::ExtensionId& extension_id,
                                ProvidingExtensionInfo* result,
-                               Profile* profile) {
+                               extensions::ExtensionRegistry* registry) {
   DCHECK(result);
-  extensions::ExtensionRegistry* const registry =
-      extensions::ExtensionRegistry::Get(profile);
   DCHECK(registry);
 
   const extensions::Extension* const extension = registry->GetExtensionById(
@@ -50,6 +47,22 @@ bool GetProvidingExtensionInfo(const std::string& extension_id,
 
 }  // namespace
 
+ProvidingExtensionInfo::ProvidingExtensionInfo() = default;
+
+ProvidingExtensionInfo::~ProvidingExtensionInfo() = default;
+
+// static
+std::unique_ptr<ProviderInterface> ExtensionProvider::Create(
+    extensions::ExtensionRegistry* registry,
+    const extensions::ExtensionId& extension_id) {
+  ProvidingExtensionInfo info;
+  if (!GetProvidingExtensionInfo(extension_id, &info, registry))
+    return nullptr;
+
+  return std::unique_ptr<ProviderInterface>(
+      new ExtensionProvider(extension_id, info));
+}
+
 std::unique_ptr<ProvidedFileSystemInterface>
 ExtensionProvider::CreateProvidedFileSystem(
     Profile* profile,
@@ -59,24 +72,23 @@ ExtensionProvider::CreateProvidedFileSystem(
       std::make_unique<ProvidedFileSystem>(profile, file_system_info));
 }
 
-bool ExtensionProvider::GetCapabilities(Profile* profile,
-                                        const ProviderId& provider_id,
-                                        Capabilities& result) {
-  ProvidingExtensionInfo providing_extension_info;
-
-  // TODO(baileyberro): Change this so error is not swallowed once
-  // bug is resolved (crrev.com/c/767629).
-  bool success = GetProvidingExtensionInfo(provider_id.GetExtensionId(),
-                                           &providing_extension_info, profile);
-
-  result = Capabilities(providing_extension_info.capabilities.configurable(),
-                        providing_extension_info.capabilities.watchable(),
-                        providing_extension_info.capabilities.multiple_mounts(),
-                        providing_extension_info.capabilities.source());
-  return success;
+const Capabilities& ExtensionProvider::GetCapabilities() const {
+  return capabilities_;
+}
+
+const ProviderId& ExtensionProvider::GetId() const {
+  return provider_id_;
 }
 
-ExtensionProvider::ExtensionProvider() {}
+ExtensionProvider::ExtensionProvider(
+    const extensions::ExtensionId& extension_id,
+    const ProvidingExtensionInfo& info)
+    : provider_id_(ProviderId::CreateFromExtensionId(extension_id)) {
+  capabilities_.configurable = info.capabilities.configurable();
+  capabilities_.watchable = info.capabilities.watchable();
+  capabilities_.multiple_mounts = info.capabilities.multiple_mounts();
+  capabilities_.source = info.capabilities.source();
+}
 
 }  // namespace file_system_provider
 }  // namespace chromeos

chrome/browser/chromeos/file_system_provider/extension_provider.h

@@ -6,30 +6,55 @@
 #define CHROME_BROWSER_CHROMEOS_FILE_SYSTEM_PROVIDER_EXTENSION_PROVIDER_H_
 
 #include <memory>
+#include <string>
 
+#include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h"
 #include "chrome/browser/chromeos/file_system_provider/provider_interface.h"
+#include "extensions/common/extension_id.h"
 
 class Profile;
 
+namespace extensions {
+class ExtensionRegistry;
+}  // namespace extensions
+
 namespace chromeos {
 namespace file_system_provider {
 
-class ProvidedFileSystemInfo;
-class ProviderId;
+// Holds information for a providing extension.
+struct ProvidingExtensionInfo {
+  ProvidingExtensionInfo();
+  ~ProvidingExtensionInfo();
+
+  extensions::ExtensionId extension_id;
+  std::string name;
+  extensions::FileSystemProviderCapabilities capabilities;
+};
 
 class ExtensionProvider : public ProviderInterface {
  public:
-  ExtensionProvider();
   ~ExtensionProvider() override {}
 
+  // Returns a provider instance for the specified extension. If the extension
+  // cannot be a providing extension, returns nullptr.
+  static std::unique_ptr<ProviderInterface> Create(
+      extensions::ExtensionRegistry* registry,
+      const extensions::ExtensionId& extension_id);
+
   // ProviderInterface overrides.
   std::unique_ptr<ProvidedFileSystemInterface> CreateProvidedFileSystem(
       Profile* profile,
       const ProvidedFileSystemInfo& file_system_info) override;
-  bool GetCapabilities(Profile* profile,
-                       const ProviderId& provider_id,
-                       Capabilities& result) override;
+  const Capabilities& GetCapabilities() const override;
+  const ProviderId& GetId() const override;
+
+ private:
+  ExtensionProvider(const extensions::ExtensionId& extension_id,
+                    const ProvidingExtensionInfo& info);
+
+  ProviderId provider_id_;
+  Capabilities capabilities_;
 };
 
 }  // namespace file_system_provider

chrome/browser/chromeos/file_system_provider/fake_extension_provider.cc

@@ -12,13 +12,21 @@
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
-#include "chrome/browser/profiles/profile.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/permissions/permissions_data.h"
 
 namespace chromeos {
 namespace file_system_provider {
 
+// static
+std::unique_ptr<ProviderInterface> FakeExtensionProvider::Create(
+    const extensions::ExtensionId& extension_id) {
+  Capabilities default_capabilities(false, false, false,
+                                    extensions::SOURCE_NETWORK);
+  return std::unique_ptr<ProviderInterface>(
+      new FakeExtensionProvider(extension_id, default_capabilities));
+}
+
 // Factory callback, to be used in Service::SetFileSystemFactory(). The
 // |event_router| argument can be NULL.
 std::unique_ptr<ProvidedFileSystemInterface>
@@ -29,7 +37,19 @@ FakeExtensionProvider::CreateProvidedFileSystem(
   return std::make_unique<FakeProvidedFileSystem>(file_system_info);
 }
 
-FakeExtensionProvider::FakeExtensionProvider() {}
+const Capabilities& FakeExtensionProvider::GetCapabilities() const {
+  return capabilities_;
+}
+
+const ProviderId& FakeExtensionProvider::GetId() const {
+  return provider_id_;
+}
+
+FakeExtensionProvider::FakeExtensionProvider(
+    const extensions::ExtensionId& extension_id,
+    const Capabilities& capabilities)
+    : provider_id_(ProviderId::CreateFromExtensionId(extension_id)),
+      capabilities_(capabilities) {}
 
 }  // namespace file_system_provider
 }  // namespace chromeos

chrome/browser/chromeos/file_system_provider/fake_extension_provider.h

@@ -10,23 +10,35 @@
 #include "chrome/browser/chromeos/file_system_provider/extension_provider.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h"
 #include "chrome/browser/chromeos/file_system_provider/provider_interface.h"
+#include "extensions/common/extension_id.h"
 
 class Profile;
 
 namespace chromeos {
 namespace file_system_provider {
 
-class ProvidedFileSystemInfo;
-
-class FakeExtensionProvider : public ExtensionProvider {
+class FakeExtensionProvider : public ProviderInterface {
  public:
-  FakeExtensionProvider();
   ~FakeExtensionProvider() override {}
 
-  // ExtensionProvider override.
+  // Returns a fake provider instance for the specified extension. The extension
+  // doesn't have to exist.
+  static std::unique_ptr<ProviderInterface> Create(
+      const extensions::ExtensionId& extension_id);
+
+  // ProviderInterface overrides.
   std::unique_ptr<ProvidedFileSystemInterface> CreateProvidedFileSystem(
       Profile* profile,
       const ProvidedFileSystemInfo& file_system_info) override;
+  const Capabilities& GetCapabilities() const override;
+  const ProviderId& GetId() const override;
+
+ protected:
+  FakeExtensionProvider(const extensions::ExtensionId& extension_id,
+                        const Capabilities& capabilities);
+
+  ProviderId provider_id_;
+  Capabilities capabilities_;
 };
 
 }  // namespace file_system_provider

chrome/browser/chromeos/file_system_provider/fileapi/file_stream_reader_unittest.cc

@@ -92,8 +92,7 @@ class FileSystemProviderFileStreamReader : public testing::Test {
     profile_ = profile_manager_->CreateTestingProfile("testing-profile");
 
     Service* service = Service::Get(profile_);  // Owned by its factory.
-    service->SetExtensionProviderForTesting(
-        std::make_unique<FakeExtensionProvider>());
+    service->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
     const base::File::Error result = service->MountFileSystem(
         kProviderId, MountOptions(kFileSystemId, "Testing File System"));

chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer_unittest.cc

@@ -74,8 +74,7 @@ class FileSystemProviderFileStreamWriter : public testing::Test {
     profile_ = profile_manager_->CreateTestingProfile("testing-profile");
 
     Service* service = Service::Get(profile_);  // Owned by its factory.
-    service->SetExtensionProviderForTesting(
-        std::make_unique<FakeExtensionProvider>());
+    service->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
     const base::File::Error result = service->MountFileSystem(
         kProviderId, MountOptions(kFileSystemId, "Testing File System"));

chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc

@@ -130,8 +130,7 @@ class FileSystemProviderProviderAsyncFileUtilTest : public testing::Test {
         content::CreateFileSystemContextForTesting(NULL, data_dir_.GetPath());
 
     Service* service = Service::Get(profile_);  // Owned by its factory.
-    service->SetExtensionProviderForTesting(
-        std::make_unique<FakeExtensionProvider>());
+    service->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
     const base::File::Error result = service->MountFileSystem(
         kProviderId, MountOptions(kFileSystemId, "Testing File System"));

chrome/browser/chromeos/file_system_provider/mount_path_util_unittest.cc

@@ -77,8 +77,8 @@ class FileSystemProviderMountPathUtilTest : public testing::Test {
     user_manager_->AddUser(
         AccountId::FromUserEmail(profile_->GetProfileUserName()));
     file_system_provider_service_ = Service::Get(profile_);
-    file_system_provider_service_->SetExtensionProviderForTesting(
-        std::make_unique<FakeExtensionProvider>());
+    file_system_provider_service_->RegisterProvider(
+        FakeExtensionProvider::Create(kExtensionId));
   }
 
   content::TestBrowserThreadBundle thread_bundle_;

chrome/browser/chromeos/file_system_provider/provided_file_system_info.cc

@@ -61,6 +61,11 @@ bool ProviderId::operator==(const ProviderId& other) const {
   return type_ == other.GetType() && internal_id_ == other.GetIdUnsafe();
 }
 
+bool ProviderId::operator<(const ProviderId& other) const {
+  return std::tie(type_, internal_id_) <
+         std::tie(other.type_, other.internal_id_);
+}
+
 MountOptions::MountOptions()
     : writable(false),
       supports_notify_tag(false),

chrome/browser/chromeos/file_system_provider/provided_file_system_info.h

@@ -46,6 +46,7 @@ class ProviderId {
   ProviderType GetType() const;
 
   bool operator==(const ProviderId& other) const;
+  bool operator<(const ProviderId& other) const;
 
  private:
   ProviderId(const std::string& internal_id, ProviderType provider_type);

chrome/browser/chromeos/file_system_provider/provider_interface.h

@@ -7,6 +7,8 @@
 #include <memory>
 #include <string>
 
+#include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.h"
 
 class Profile;
@@ -15,7 +17,6 @@ namespace chromeos {
 namespace file_system_provider {
 
 class ProvidedFileSystemInterface;
-class ProvidedFileSystemInfo;
 class ProviderId;
 
 struct Capabilities {
@@ -27,7 +28,12 @@ struct Capabilities {
         watchable(watchable),
         multiple_mounts(multiple_mounts),
         source(source) {}
-  Capabilities() = default;
+  Capabilities()
+      : configurable(false),
+        watchable(false),
+        multiple_mounts(false),
+        source(extensions::SOURCE_NETWORK) {}
+
   bool configurable;
   bool watchable;
   bool multiple_mounts;
@@ -43,10 +49,11 @@ class ProviderInterface {
       Profile* profile,
       const ProvidedFileSystemInfo& file_system_info) = 0;
 
-  // Returns the capabilites of file system with |provider_id|.
-  virtual bool GetCapabilities(Profile* profile,
-                               const ProviderId& provider_id,
-                               Capabilities& result) = 0;
+  // Returns the capabilites of the provider.
+  virtual const Capabilities& GetCapabilities() const = 0;
+
+  // Returns id of this provider.
+  virtual const ProviderId& GetId() const = 0;
 };
 
 }  // namespace file_system_provider

chrome/browser/chromeos/file_system_provider/service.cc

@@ -38,19 +38,11 @@ const size_t kMaxFileSystems = 16;
 
 }  // namespace
 
-ProvidingExtensionInfo::ProvidingExtensionInfo() {
-}
-
-ProvidingExtensionInfo::~ProvidingExtensionInfo() {
-}
-
 Service::Service(Profile* profile,
                  extensions::ExtensionRegistry* extension_registry)
     : profile_(profile),
       extension_registry_(extension_registry),
       registry_(new Registry(profile)),
-      extension_provider_(
-          std::make_unique<ExtensionProvider>(ExtensionProvider())),
       weak_ptr_factory_(this) {
   extension_registry_->AddObserver(this);
 }
@@ -94,12 +86,6 @@ void Service::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void Service::SetExtensionProviderForTesting(
-    std::unique_ptr<ProviderInterface> provider) {
-  DCHECK(provider);
-  extension_provider_ = std::move(provider);
-}
-
 void Service::SetRegistryForTesting(
     std::unique_ptr<RegistryInterface> registry) {
   DCHECK(registry);
@@ -117,17 +103,23 @@ base::File::Error Service::MountFileSystemInternal(
     MountContext context) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  ProviderInterface* const provider = GetProvider(provider_id);
+  if (!provider) {
+    for (auto& observer : observers_) {
+      observer.OnProvidedFileSystemMount(
+          ProvidedFileSystemInfo(), context,
+          base::File::FILE_ERROR_INVALID_OPERATION);
+    }
+    return base::File::FILE_ERROR_INVALID_OPERATION;
+  }
+
   // The mount point path and name are unique per system, since they are system
   // wide. This is necessary for copying between profiles.
   const base::FilePath& mount_path =
       util::GetMountPath(profile_, provider_id, options.file_system_id);
   const std::string mount_point_name = mount_path.BaseName().AsUTF8Unsafe();
 
-  ProvidingExtensionInfo provider_info;
-  // TODO(mtomasz): Set up a testing extension in unit tests.
-  ProviderInterface* provider = GetProvider(provider_id);
-  Capabilities capabilities;
-  provider->GetCapabilities(profile_, provider_id, capabilities);
+  Capabilities capabilities = provider->GetCapabilities();
   // Store the file system descriptor. Use the mount point name as the file
   // system provider file system id.
   // Examples:
@@ -357,7 +349,17 @@ bool Service::GetProvidingExtensionInfo(const std::string& extension_id,
 void Service::OnExtensionUnloaded(content::BrowserContext* browser_context,
                                   const extensions::Extension* extension,
                                   extensions::UnloadedExtensionReason reason) {
-  // Unmount all of the provided file systems associated with this extension.
+  ProviderId provider_id = ProviderId::CreateFromExtensionId(extension->id());
+  UnregisterProvider(
+      provider_id,
+      reason == extensions::UnloadedExtensionReason::PROFILE_SHUTDOWN
+          ? UNMOUNT_REASON_SHUTDOWN
+          : UNMOUNT_REASON_USER);
+}
+
+void Service::UnmountFileSystems(const ProviderId& provider_id,
+                                 UnmountReason reason) {
+  // Unmount all of the provided file systems associated with this provider.
   auto it = file_system_map_.begin();
   while (it != file_system_map_.end()) {
     const ProvidedFileSystemInfo& file_system_info =
@@ -365,20 +367,31 @@ void Service::OnExtensionUnloaded(content::BrowserContext* browser_context,
     // Advance the iterator beforehand, otherwise it will become invalidated
     // by the UnmountFileSystem() call.
     ++it;
-    if (file_system_info.provider_id().GetExtensionId() == extension->id()) {
-      const base::File::Error unmount_result = UnmountFileSystem(
-          file_system_info.provider_id(), file_system_info.file_system_id(),
-          reason == extensions::UnloadedExtensionReason::PROFILE_SHUTDOWN
-              ? UNMOUNT_REASON_SHUTDOWN
-              : UNMOUNT_REASON_USER);
+    if (file_system_info.provider_id() == provider_id) {
+      const base::File::Error unmount_result =
+          UnmountFileSystem(file_system_info.provider_id(),
+                            file_system_info.file_system_id(), reason);
       DCHECK_EQ(base::File::FILE_OK, unmount_result);
     }
   }
 }
 
+void Service::UnregisterProvider(const ProviderId& provider_id,
+                                 UnmountReason reason) {
+  UnmountFileSystems(provider_id, reason);
+  provider_map_.erase(provider_id);
+}
+
 void Service::OnExtensionLoaded(content::BrowserContext* browser_context,
                                 const extensions::Extension* extension) {
-  ProviderId provider_id = ProviderId::CreateFromExtensionId(extension->id());
+  // If the extension is a provider, then register it.
+  std::unique_ptr<ProviderInterface> provider =
+      ExtensionProvider::Create(extension_registry_, extension->id());
+  if (provider)
+    RegisterProvider(std::move(provider));
+}
+
+void Service::RestoreFileSystems(const ProviderId& provider_id) {
   std::unique_ptr<RegistryInterface::RestoredFileSystems>
       restored_file_systems = registry_->RestoreFileSystems(provider_id);
 
@@ -457,21 +470,17 @@ void Service::OnWatcherListChanged(
   registry_->RememberFileSystem(file_system_info, watchers);
 }
 
-void Service::RegisterNativeProvider(
-    const ProviderId& provider_id,
-    std::unique_ptr<ProviderInterface> provider) {
-  DCHECK_EQ(ProviderId::NATIVE, provider_id.GetType());
-  native_provider_map_[provider_id.GetNativeId()] = std::move(provider);
+void Service::RegisterProvider(std::unique_ptr<ProviderInterface> provider) {
+  ProviderId provider_id = provider->GetId();
+  provider_map_[provider_id] = std::move(provider);
+  RestoreFileSystems(provider_id);
 }
 
 ProviderInterface* Service::GetProvider(const ProviderId& provider_id) {
   DCHECK_NE(ProviderId::INVALID, provider_id.GetType());
-
-  if (provider_id.GetType() == ProviderId::EXTENSION)
-    return extension_provider_.get();
-
-  auto it = native_provider_map_.find(provider_id.GetNativeId());
-  DCHECK(it != native_provider_map_.end());
+  auto it = provider_map_.find(provider_id);
+  if (it == provider_map_.end())
+    return nullptr;
 
   return it->second.get();
 }

chrome/browser/chromeos/file_system_provider/service.h

@@ -34,6 +34,7 @@
 #include "content/public/browser/browser_context.h"
 #include "extensions/browser/extension_registry_observer.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_id.h"
 #include "storage/browser/fileapi/watcher_manager.h"
 
 namespace extensions {
@@ -55,27 +56,12 @@ struct MountOptions;
 // Registers preferences to remember registered file systems between reboots.
 void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
-// Holds information for a providing extension.
-struct ProvidingExtensionInfo {
-  ProvidingExtensionInfo();
-  ~ProvidingExtensionInfo();
-
-  std::string extension_id;
-  std::string name;
-  extensions::FileSystemProviderCapabilities capabilities;
-};
-
 // Manages and registers the file system provider service. Maintains provided
 // file systems.
 class Service : public KeyedService,
                 public extensions::ExtensionRegistryObserver,
                 public ProvidedFileSystemObserver {
  public:
-  typedef base::Callback<std::unique_ptr<ProvidedFileSystemInterface>(
-      Profile* profile,
-      const ProvidedFileSystemInfo& file_system_info)>
-      FileSystemFactoryCallback;
-
   // Reason for unmounting. In case of UNMOUNT_REASON_SHUTDOWN, the file system
   // will be remounted automatically after a reboot. In case of
   // UNMOUNT_REASON_USER it will be permanently unmounted.
@@ -90,11 +76,6 @@ class Service : public KeyedService,
   // KeyedService:
   void Shutdown() override;
 
-  // Sets a custom ProvidedFileSystemInterface factory. Used by unit tests,
-  // where an event router is not available.
-  void SetExtensionProviderForTesting(
-      std::unique_ptr<ProviderInterface> provider);
-
   // Sets a custom Registry implementation. Used by unit tests.
   void SetRegistryForTesting(std::unique_ptr<RegistryInterface> registry);
 
@@ -171,9 +152,13 @@ class Service : public KeyedService,
   void OnWatcherListChanged(const ProvidedFileSystemInfo& file_system_info,
                             const Watchers& watchers) override;
 
-  // Registers a FileSystemFactory for the passed |provider_id|.
-  void RegisterNativeProvider(const ProviderId& provider_id,
-                              std::unique_ptr<ProviderInterface> provider);
+  // Registers a provider. Restores all remembered mounts.
+  void RegisterProvider(std::unique_ptr<ProviderInterface> provider);
+
+  // Unregisters a provider. Unmounts all currently mounted file systems.
+  // If the reason is UNMOUNT_REASON_USER then they will not be automatically
+  // restored on the next registration.
+  void UnregisterProvider(const ProviderId& provider_id, UnmountReason reason);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(FileSystemProviderServiceTest, RememberFileSystem);
@@ -207,7 +192,13 @@ class Service : public KeyedService,
   // |provider_id| provided file system.
   void RestoreFileSystems(const ProviderId& provider_id);
 
-  // Returns a file system provider for the passed |provider_id|.
+  // Unmounts all currently mounted file systems for this provider. If
+  // reason is UNMOUNT_REASON_USER then the file systems will not be remembered
+  // for automagical remount in the future.
+  void UnmountFileSystems(const ProviderId& provider_id, UnmountReason reason);
+
+  // Returns a file system provider for the passed |provider_id|. If not found
+  // then returns nullptr.
   ProviderInterface* GetProvider(const ProviderId& provider_id);
 
   Profile* profile_;
@@ -218,9 +209,7 @@ class Service : public KeyedService,
   std::map<std::string, FileSystemKey> mount_point_name_to_key_map_;
   std::unique_ptr<RegistryInterface> registry_;
   base::ThreadChecker thread_checker_;
-  std::unordered_map<std::string, std::unique_ptr<ProviderInterface>>
-      native_provider_map_;
-  std::unique_ptr<ProviderInterface> extension_provider_;
+  std::map<ProviderId, std::unique_ptr<ProviderInterface>> provider_map_;
 
   base::WeakPtrFactory<Service> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(Service);

chrome/browser/chromeos/file_system_provider/service_unittest.cc

@@ -42,8 +42,8 @@ namespace chromeos {
 namespace file_system_provider {
 namespace {
 
-const ProviderId kProviderId =
-    ProviderId::CreateFromExtensionId("mbflcebpggnecokmikipoihdbecnjfoj");
+const extensions::ExtensionId kExtensionId = "mbflcebpggnecokmikipoihdbecnjfoj";
+const ProviderId kProviderId = ProviderId::CreateFromExtensionId(kExtensionId);
 const char kDisplayName[] = "Camera Pictures";
 const ProviderId kCustomProviderId =
     ProviderId::CreateFromNativeId("custom provider id");
@@ -54,54 +54,35 @@ const ProviderId kCustomProviderId =
 const char kFileSystemId[] = "camera/pictures/id .!@#$%^&*()_+";
 
 // Creates a fake extension with the specified |extension_id|.
+// TODO(mtomasz): Use the extension builder.
 scoped_refptr<extensions::Extension> CreateFakeExtension(
     const std::string& extension_id) {
   base::DictionaryValue manifest;
   std::string error;
   manifest.SetKey(extensions::manifest_keys::kVersion, base::Value("1.0.0.0"));
   manifest.SetKey(extensions::manifest_keys::kName, base::Value("unused"));
-  return extensions::Extension::Create(base::FilePath(),
-                                       extensions::Manifest::UNPACKED,
-                                       manifest,
-                                       extensions::Extension::NO_FLAGS,
-                                       extension_id,
-                                       &error);
+
+  std::unique_ptr<base::ListValue> permissions_list(new base::ListValue());
+  permissions_list->AppendString("fileSystemProvider");
+  manifest.Set(extensions::manifest_keys::kPermissions,
+               std::move(permissions_list));
+
+  std::unique_ptr<base::DictionaryValue> capabilities(
+      new base::DictionaryValue);
+  capabilities->SetString("source", "network");
+  manifest.Set(extensions::manifest_keys::kFileSystemProviderCapabilities,
+               std::move(capabilities));
+
+  scoped_refptr<extensions::Extension> extension =
+      extensions::Extension::Create(
+          base::FilePath(), extensions::Manifest::UNPACKED, manifest,
+          extensions::Extension::NO_FLAGS, extension_id, &error);
+  EXPECT_TRUE(extension) << error;
+  return extension;
 }
 
 }  // namespace
 
-class FakeDefaultExtensionProvider : public FakeExtensionProvider {
- public:
-  std::unique_ptr<ProvidedFileSystemInterface> CreateProvidedFileSystem(
-      Profile* profile,
-      const ProvidedFileSystemInfo& file_system_info) override {
-    called_default_factory = true;
-    return std::make_unique<FakeProvidedFileSystem>(file_system_info);
-  }
-
-  static bool called_default_factory;
-};
-bool FakeDefaultExtensionProvider::called_default_factory = false;
-
-class FakeNativeProvider : public ProviderInterface {
- public:
-  // ProviderInterface overrides
-  std::unique_ptr<ProvidedFileSystemInterface> CreateProvidedFileSystem(
-      Profile* profile,
-      const ProvidedFileSystemInfo& file_system_info) override {
-    called_custom_factory = true;
-    return std::make_unique<FakeProvidedFileSystem>(file_system_info);
-  }
-  bool GetCapabilities(Profile* profile,
-                       const ProviderId& provider_id,
-                       Capabilities& result) override {
-    result = Capabilities(false, false, false, extensions::SOURCE_NETWORK);
-    return true;
-  }
-  static bool called_custom_factory;
-};
-bool FakeNativeProvider::called_custom_factory = false;
-
 class FileSystemProviderServiceTest : public testing::Test {
  protected:
   FileSystemProviderServiceTest() : profile_(NULL) {}
@@ -119,14 +100,8 @@ class FileSystemProviderServiceTest : public testing::Test {
     user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
         base::WrapUnique(user_manager_));
     extension_registry_.reset(new extensions::ExtensionRegistry(profile_));
-
     service_.reset(new Service(profile_, extension_registry_.get()));
 
-    service_->SetExtensionProviderForTesting(
-        std::make_unique<FakeExtensionProvider>());
-
-    extension_ = CreateFakeExtension(kProviderId.GetExtensionId());
-
     registry_ = new FakeRegistry;
     // Passes ownership to the service instance.
     service_->SetRegistryForTesting(base::WrapUnique(registry_));
@@ -147,38 +122,14 @@ class FileSystemProviderServiceTest : public testing::Test {
   std::unique_ptr<user_manager::ScopedUserManager> user_manager_enabler_;
   std::unique_ptr<extensions::ExtensionRegistry> extension_registry_;
   std::unique_ptr<Service> service_;
-  scoped_refptr<extensions::Extension> extension_;
   FakeRegistry* registry_;  // Owned by Service.
   Watcher fake_watcher_;
-  bool called_default_factory_;
-  bool called_custom_factory_;
 };
 
-TEST_F(FileSystemProviderServiceTest, RegisterFileSystemProvider) {
-  service_->RegisterNativeProvider(kCustomProviderId,
-                                   std::make_unique<FakeNativeProvider>());
-  service_->SetExtensionProviderForTesting(
-      std::make_unique<FakeDefaultExtensionProvider>());
-
-  FakeNativeProvider::called_custom_factory = false;
-  FakeDefaultExtensionProvider::called_default_factory = false;
-
-  EXPECT_EQ(base::File::FILE_OK,
-            service_->MountFileSystem(
-                kProviderId, MountOptions(kFileSystemId, kDisplayName)));
-
-  EXPECT_TRUE(FakeDefaultExtensionProvider::called_default_factory);
-
-  EXPECT_EQ(base::File::FILE_OK,
-            service_->MountFileSystem(
-                kCustomProviderId, MountOptions(kFileSystemId, kDisplayName)));
-
-  EXPECT_TRUE(FakeNativeProvider::called_custom_factory);
-}
-
 TEST_F(FileSystemProviderServiceTest, MountFileSystem) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   EXPECT_EQ(base::File::FILE_OK,
             service_->MountFileSystem(
@@ -210,6 +161,7 @@ TEST_F(FileSystemProviderServiceTest,
        MountFileSystem_WritableAndSupportsNotifyTag) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   MountOptions options(kFileSystemId, kDisplayName);
   options.writable = true;
@@ -231,6 +183,7 @@ TEST_F(FileSystemProviderServiceTest,
 TEST_F(FileSystemProviderServiceTest, MountFileSystem_UniqueIds) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   EXPECT_EQ(base::File::FILE_OK,
             service_->MountFileSystem(
@@ -253,6 +206,7 @@ TEST_F(FileSystemProviderServiceTest, MountFileSystem_UniqueIds) {
 TEST_F(FileSystemProviderServiceTest, MountFileSystem_StressTest) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   const size_t kMaxFileSystems = 16;
   for (size_t i = 0; i < kMaxFileSystems; ++i) {
@@ -284,6 +238,8 @@ TEST_F(FileSystemProviderServiceTest, UnmountFileSystem) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
 
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
+
   EXPECT_EQ(base::File::FILE_OK,
             service_->MountFileSystem(
                 kProviderId, MountOptions(kFileSystemId, kDisplayName)));
@@ -310,13 +266,18 @@ TEST_F(FileSystemProviderServiceTest, UnmountFileSystem_OnExtensionUnload) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
 
+  scoped_refptr<extensions::Extension> extension =
+      CreateFakeExtension(kExtensionId);
+  extension_registry_->AddEnabled(extension);
+  service_->OnExtensionLoaded(profile_, extension.get());
+
   EXPECT_EQ(base::File::FILE_OK,
             service_->MountFileSystem(
                 kProviderId, MountOptions(kFileSystemId, kDisplayName)));
   ASSERT_EQ(1u, observer.mounts.size());
 
   // Directly call the observer's method.
-  service_->OnExtensionUnloaded(profile_, extension_.get(),
+  service_->OnExtensionUnloaded(profile_, extension.get(),
                                 extensions::UnloadedExtensionReason::DISABLE);
 
   ASSERT_EQ(1u, observer.unmounts.size());
@@ -336,6 +297,7 @@ TEST_F(FileSystemProviderServiceTest, UnmountFileSystem_OnExtensionUnload) {
 TEST_F(FileSystemProviderServiceTest, UnmountFileSystem_WrongExtensionId) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   const ProviderId kWrongProviderId =
       ProviderId::CreateFromExtensionId("helloworldhelloworldhelloworldhe");
@@ -377,8 +339,12 @@ TEST_F(FileSystemProviderServiceTest, RestoreFileSystem_OnExtensionLoad) {
 
   EXPECT_EQ(0u, observer.mounts.size());
 
+  scoped_refptr<extensions::Extension> extension =
+      CreateFakeExtension(kExtensionId);
+  extension_registry_->AddEnabled(extension);
+
   // Directly call the observer's method.
-  service_->OnExtensionLoaded(profile_, extension_.get());
+  service_->OnExtensionLoaded(profile_, extension.get());
 
   ASSERT_EQ(1u, observer.mounts.size());
   EXPECT_EQ(base::File::FILE_OK, observer.mounts[0].error());
@@ -419,6 +385,7 @@ TEST_F(FileSystemProviderServiceTest, RestoreFileSystem_OnExtensionLoad) {
 TEST_F(FileSystemProviderServiceTest, DoNotRememberNonPersistent) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   EXPECT_FALSE(registry_->file_system_info());
   EXPECT_FALSE(registry_->watchers());
@@ -437,6 +404,7 @@ TEST_F(FileSystemProviderServiceTest, DoNotRememberNonPersistent) {
 TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnMount) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   EXPECT_FALSE(registry_->file_system_info());
   EXPECT_FALSE(registry_->watchers());
@@ -462,6 +430,7 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnMount) {
 TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountOnShutdown) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   {
     EXPECT_FALSE(registry_->file_system_info());
@@ -491,6 +460,7 @@ TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountOnShutdown) {
 TEST_F(FileSystemProviderServiceTest, RememberFileSystem_OnUnmountByUser) {
   LoggingObserver observer;
   service_->AddObserver(&observer);
+  service_->RegisterProvider(FakeExtensionProvider::Create(kExtensionId));
 
   {
     EXPECT_FALSE(registry_->file_system_info());

chrome/browser/chromeos/smb_client/smb_service.cc

@@ -12,19 +12,18 @@ using chromeos::file_system_provider::Service;
 namespace chromeos {
 namespace smb_client {
 
-file_system_provider::ProviderId kSmbProviderId =
-    ProviderId::CreateFromNativeId("smb");
-
-SmbService::SmbService(Profile* profile) : profile_(profile) {
-  GetProviderService()->RegisterNativeProvider(
-      kSmbProviderId, std::make_unique<SmbService>(profile));
+SmbService::SmbService(Profile* profile)
+    : profile_(profile),
+      provider_id_(ProviderId::CreateFromNativeId("smb")),
+      capabilities_(false, false, false, extensions::SOURCE_NETWORK) {
+  GetProviderService()->RegisterProvider(std::make_unique<SmbService>(profile));
 }
 
 SmbService::~SmbService() {}
 
 base::File::Error SmbService::Mount(
     const file_system_provider::MountOptions& options) {
-  return GetProviderService()->MountFileSystem(kSmbProviderId, options);
+  return GetProviderService()->MountFileSystem(provider_id_, options);
 }
 
 Service* SmbService::GetProviderService() const {
@@ -39,11 +38,12 @@ SmbService::CreateProvidedFileSystem(
   return std::make_unique<SmbFileSystem>(file_system_info);
 }
 
-bool SmbService::GetCapabilities(Profile* profile,
-                                 const ProviderId& provider_id,
-                                 Capabilities& result) {
-  result = Capabilities(false, false, false, extensions::SOURCE_NETWORK);
-  return true;
+const Capabilities& SmbService::GetCapabilities() const {
+  return capabilities_;
+}
+
+const ProviderId& SmbService::GetId() const {
+  return provider_id_;
 }
 
 }  // namespace smb_client

chrome/browser/chromeos/smb_client/smb_service.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_SERVICE_H_
 #define CHROME_BROWSER_CHROMEOS_SMB_CLIENT_SMB_SERVICE_H_
 
+#include <memory>
+
 #include "base/files/file.h"
 #include "base/macros.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
@@ -33,18 +35,19 @@ class SmbService : public KeyedService, public ProviderInterface {
   // file_system_provider::Service::MountFileSystem().
   base::File::Error Mount(const file_system_provider::MountOptions& options);
 
-  // ProviderInterface overrides
+  // ProviderInterface overrides.
   std::unique_ptr<ProvidedFileSystemInterface> CreateProvidedFileSystem(
       Profile* profile,
       const ProvidedFileSystemInfo& file_system_info) override;
-  bool GetCapabilities(Profile* profile,
-                       const ProviderId& provider_id,
-                       Capabilities& result) override;
+  const Capabilities& GetCapabilities() const override;
+  const ProviderId& GetId() const override;
 
  private:
   Service* GetProviderService() const;
 
   Profile* profile_;
+  ProviderId provider_id_;
+  Capabilities capabilities_;
 
   DISALLOW_COPY_AND_ASSIGN(SmbService);
 };

chrome/browser/chromeos/smb_client/smb_service_unittest.cc

@@ -5,10 +5,11 @@
 #include "chrome/browser/chromeos/smb_client/smb_service.h"
 
 #include <stddef.h>
+
 #include <memory>
+#include <utility>
 
 #include "base/memory/ptr_util.h"
-#include "chrome/browser/chromeos/file_system_provider/fake_extension_provider.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_provided_file_system.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_registry.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
@@ -47,8 +48,6 @@ class SmbServiceTest : public testing::Test {
         std::make_unique<extensions::ExtensionRegistry>(profile_);
     fsp_service_ = std::make_unique<file_system_provider::Service>(
         profile_, extension_registry_.get());
-    fsp_service_->SetExtensionProviderForTesting(
-        std::make_unique<file_system_provider::FakeExtensionProvider>());
 
     fsp_service_->SetRegistryForTesting(
         std::make_unique<file_system_provider::FakeRegistry>());