Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(719)

Unified Diff: chrome/browser/chromeos/file_system_provider/service.cc

Issue 210803003: [fsp] Decouple file_service_provider::Service. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Addressed comments. Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/chromeos/file_system_provider/service.cc
diff --git a/chrome/browser/chromeos/file_system_provider/service.cc b/chrome/browser/chromeos/file_system_provider/service.cc
index f8e7136560008f9998b016bdb4d92da180155043..5348b34712a5e165057dc6871014dd0cdd9fb4eb 100644
--- a/chrome/browser/chromeos/file_system_provider/service.cc
+++ b/chrome/browser/chromeos/file_system_provider/service.cc
@@ -5,16 +5,14 @@
#include "chrome/browser/chromeos/file_system_provider/service.h"
#include "base/files/file_path.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/values.h"
+#include "base/stl_util.h"
+#include "chrome/browser/chromeos/file_system_provider/mount_path_util.h"
#include "chrome/browser/chromeos/file_system_provider/observer.h"
#include "chrome/browser/chromeos/file_system_provider/provided_file_system.h"
+#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/service_factory.h"
-#include "chrome/browser/chromeos/login/user.h"
-#include "chrome/browser/chromeos/login/user_manager.h"
-#include "chrome/common/extensions/api/file_system_provider.h"
#include "content/public/browser/browser_thread.h"
-#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_system.h"
#include "webkit/browser/fileapi/external_mount_points.h"
@@ -22,46 +20,32 @@ namespace chromeos {
namespace file_system_provider {
namespace {
-// Root mount path for all of the provided file systems.
-const base::FilePath::CharType kProvidedMountPointRoot[] =
- FILE_PATH_LITERAL("/provided");
-
// Maximum number of file systems to be mounted in the same time, per profile.
const size_t kMaxFileSystems = 16;
-// Constructs a safe mount point path for the provided file system represented
-// by |file_system_handle|. The handle is a numeric part of the file system id.
-base::FilePath GetMountPointPath(Profile* profile,
- std::string extension_id,
- int file_system_id) {
- chromeos::User* const user =
- chromeos::UserManager::IsInitialized()
- ? chromeos::UserManager::Get()->GetUserByProfile(
- profile->GetOriginalProfile())
- : NULL;
- const std::string user_suffix = user ? "-" + user->username_hash() : "";
- return base::FilePath(kProvidedMountPointRoot).AppendASCII(
- extension_id + "-" + base::IntToString(file_system_id) + user_suffix);
-}
-
-// Creates values to be passed to request events. These values can be extended
-// by additional fields.
-scoped_ptr<base::ListValue> CreateRequestValues(int file_system_id,
- int request_id) {
- scoped_ptr<base::ListValue> values(new base::ListValue());
- values->AppendInteger(file_system_id);
- values->AppendInteger(request_id);
- return values.Pass();
+// Default factory for provided file systems. The |event_router| nor the
+// |request_manager| arguments must not be NULL.
+ProvidedFileSystemInterface* CreateProvidedFileSystem(
+ extensions::EventRouter* event_router,
+ RequestManager* request_manager,
+ const ProvidedFileSystemInfo& file_system_info) {
+ DCHECK(event_router);
+ DCHECK(request_manager);
+ return new ProvidedFileSystem(
+ event_router, request_manager, file_system_info);
}
} // namespace
Service::Service(Profile* profile)
- : profile_(profile), next_id_(1), weak_ptr_factory_(this) {
+ : profile_(profile),
+ file_system_factory_(base::Bind(CreateProvidedFileSystem)),
+ next_id_(1),
+ weak_ptr_factory_(this) {
AddObserver(&request_manager_);
}
-Service::~Service() {}
+Service::~Service() { STLDeleteValues(&file_system_map_); }
// static
Service* Service::Get(content::BrowserContext* context) {
@@ -78,21 +62,27 @@ void Service::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
+void Service::SetFileSystemFactoryForTests(
+ const FileSystemFactoryCallback& factory_callback) {
+ DCHECK(!factory_callback.is_null());
+ file_system_factory_ = factory_callback;
+}
+
int Service::MountFileSystem(const std::string& extension_id,
const std::string& file_system_name) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
// Restrict number of file systems to prevent system abusing.
- if (file_systems_.size() + 1 > kMaxFileSystems) {
+ if (file_system_map_.size() + 1 > kMaxFileSystems) {
FOR_EACH_OBSERVER(
Observer,
observers_,
- OnProvidedFileSystemMount(ProvidedFileSystem(),
+ OnProvidedFileSystemMount(ProvidedFileSystemInfo(),
base::File::FILE_ERROR_TOO_MANY_OPENED));
return 0;
}
- // The file system id is unique per service, so per profile.
+ // The provided file system id is unique per service, so per profile.
int file_system_id = next_id_;
fileapi::ExternalMountPoints* const mount_points =
@@ -102,7 +92,7 @@ int Service::MountFileSystem(const std::string& extension_id,
// 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_point_path =
- GetMountPointPath(profile_, extension_id, file_system_id);
+ util::GetMountPointPath(profile_, extension_id, file_system_id);
const std::string mount_point_name =
mount_point_path.BaseName().AsUTF8Unsafe();
@@ -113,7 +103,7 @@ int Service::MountFileSystem(const std::string& extension_id,
FOR_EACH_OBSERVER(
Observer,
observers_,
- OnProvidedFileSystemMount(ProvidedFileSystem(),
+ OnProvidedFileSystemMount(ProvidedFileSystemInfo(),
base::File::FILE_ERROR_INVALID_OPERATION));
return 0;
}
@@ -124,14 +114,22 @@ int Service::MountFileSystem(const std::string& extension_id,
// file_system_id = 41
// mount_point_name = file_system_id = b33f1337-41-5aa5
// mount_point_path = /provided/b33f1337-41-5aa5
- ProvidedFileSystem file_system(
+ ProvidedFileSystemInfo file_system_info(
extension_id, file_system_id, file_system_name, mount_point_path);
- file_systems_[file_system_id] = file_system;
+
+ // The event router may be NULL for unit tests.
+ extensions::EventRouter* event_router =
+ extensions::ExtensionSystem::Get(profile_)->event_router();
+
+ ProvidedFileSystemInterface* file_system = file_system_factory_.Run(
+ event_router, &request_manager_, file_system_info);
+ DCHECK(file_system);
+ file_system_map_[file_system_id] = file_system;
FOR_EACH_OBSERVER(
Observer,
observers_,
- OnProvidedFileSystemMount(file_system, base::File::FILE_OK));
+ OnProvidedFileSystemMount(file_system_info, base::File::FILE_OK));
next_id_++;
return file_system_id;
@@ -141,14 +139,17 @@ bool Service::UnmountFileSystem(const std::string& extension_id,
int file_system_id) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
- FileSystemMap::iterator file_system_it = file_systems_.find(file_system_id);
- if (file_system_it == file_systems_.end() ||
- file_system_it->second.extension_id() != extension_id) {
- const ProvidedFileSystem empty_file_system;
- FOR_EACH_OBSERVER(Observer,
- observers_,
- OnProvidedFileSystemUnmount(
- empty_file_system, base::File::FILE_ERROR_NOT_FOUND));
+ ProvidedFileSystemMap::iterator file_system_it =
+ file_system_map_.find(file_system_id);
+ if (file_system_it == file_system_map_.end() ||
+ file_system_it->second->GetFileSystemInfo().extension_id() !=
+ extension_id) {
+ const ProvidedFileSystemInfo empty_file_system_info;
+ FOR_EACH_OBSERVER(
+ Observer,
+ observers_,
+ OnProvidedFileSystemUnmount(empty_file_system_info,
+ base::File::FILE_ERROR_NOT_FOUND));
return false;
}
@@ -156,13 +157,16 @@ bool Service::UnmountFileSystem(const std::string& extension_id,
fileapi::ExternalMountPoints::GetSystemInstance();
DCHECK(mount_points);
+ const ProvidedFileSystemInfo& file_system_info =
+ file_system_it->second->GetFileSystemInfo();
+
const std::string mount_point_name =
- file_system_it->second.mount_path().BaseName().value();
+ file_system_info.mount_path().BaseName().value();
if (!mount_points->RevokeFileSystem(mount_point_name)) {
FOR_EACH_OBSERVER(
Observer,
observers_,
- OnProvidedFileSystemUnmount(file_system_it->second,
+ OnProvidedFileSystemUnmount(file_system_info,
base::File::FILE_ERROR_INVALID_OPERATION));
return false;
}
@@ -170,96 +174,68 @@ bool Service::UnmountFileSystem(const std::string& extension_id,
FOR_EACH_OBSERVER(
Observer,
observers_,
- OnProvidedFileSystemUnmount(file_system_it->second, base::File::FILE_OK));
+ OnProvidedFileSystemUnmount(file_system_info, base::File::FILE_OK));
- file_systems_.erase(file_system_it);
+ delete file_system_it->second;
+ file_system_map_.erase(file_system_it);
return true;
}
-std::vector<ProvidedFileSystem> Service::GetMountedFileSystems() {
- DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
- std::vector<ProvidedFileSystem> result;
- for (FileSystemMap::const_iterator it = file_systems_.begin();
- it != file_systems_.end();
- ++it) {
- result.push_back(it->second);
- }
- return result;
-}
-
-bool Service::FulfillRequest(const std::string& extension_id,
- int file_system_id,
- int request_id,
- scoped_ptr<base::DictionaryValue> result,
- bool has_next) {
+bool Service::RequestUnmount(int file_system_id) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
- FileSystemMap::iterator file_system_it = file_systems_.find(file_system_id);
- if (file_system_it == file_systems_.end() ||
- file_system_it->second.extension_id() != extension_id) {
+ ProvidedFileSystemMap::iterator file_system_it =
+ file_system_map_.find(file_system_id);
+ if (file_system_it == file_system_map_.end())
return false;
- }
- return request_manager_.FulfillRequest(
- file_system_it->second, request_id, result.Pass(), has_next);
+ return file_system_it->second->RequestUnmount(
+ base::Bind(&Service::OnRequestUnmountStatus,
+ weak_ptr_factory_.GetWeakPtr(),
+ file_system_it->second->GetFileSystemInfo()));
}
-bool Service::RejectRequest(const std::string& extension_id,
- int file_system_id,
- int request_id,
- base::File::Error error) {
+std::vector<ProvidedFileSystemInfo> Service::GetProvidedFileSystemInfoList() {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
- FileSystemMap::iterator file_system_it = file_systems_.find(file_system_id);
- if (file_system_it == file_systems_.end() ||
- file_system_it->second.extension_id() != extension_id) {
- return false;
+ std::vector<ProvidedFileSystemInfo> result;
+ for (ProvidedFileSystemMap::const_iterator it = file_system_map_.begin();
+ it != file_system_map_.end();
+ ++it) {
+ result.push_back(it->second->GetFileSystemInfo());
}
-
- return request_manager_.RejectRequest(
- file_system_it->second, request_id, error);
+ return result;
}
-bool Service::RequestUnmount(int file_system_id) {
+ProvidedFileSystemInterface* Service::GetProvidedFileSystem(
+ const std::string& extension_id,
+ int file_system_id) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
- FileSystemMap::iterator file_system_it = file_systems_.find(file_system_id);
- if (file_system_it == file_systems_.end())
- return false;
-
- int request_id =
- request_manager_.CreateRequest(file_system_it->second,
- SuccessCallback(),
- base::Bind(&Service::OnRequestUnmountError,
- weak_ptr_factory_.GetWeakPtr(),
- file_system_it->second));
-
- if (!request_id)
- return false;
-
- scoped_ptr<base::ListValue> values(
- CreateRequestValues(file_system_id, request_id));
-
- extensions::EventRouter* event_router =
- extensions::ExtensionSystem::Get(profile_)->event_router();
- DCHECK(event_router);
-
- event_router->DispatchEventToExtension(
- file_system_it->second.extension_id(),
- make_scoped_ptr(new extensions::Event(
- extensions::api::file_system_provider::OnUnmountRequested::kEventName,
- values.Pass())));
+ ProvidedFileSystemMap::iterator file_system_it =
+ file_system_map_.find(file_system_id);
+ if (file_system_it == file_system_map_.end() ||
+ file_system_it->second->GetFileSystemInfo().extension_id() !=
+ extension_id) {
+ return NULL;
+ }
- return true;
+ return file_system_it->second;
}
void Service::Shutdown() { RemoveObserver(&request_manager_); }
-void Service::OnRequestUnmountError(const ProvidedFileSystem& file_system,
- base::File::Error error) {
- FOR_EACH_OBSERVER(
- Observer, observers_, OnProvidedFileSystemUnmount(file_system, error));
+void Service::OnRequestUnmountStatus(
+ const ProvidedFileSystemInfo& file_system_info,
+ base::File::Error error) {
+ // Notify observers about failure in unmounting, since mount() will not be
+ // called by the provided file system. In case of success mount() will be
+ // invoked, and observers notified, so there is no need to call them now.
+ if (error != base::File::FILE_OK) {
+ FOR_EACH_OBSERVER(Observer,
+ observers_,
+ OnProvidedFileSystemUnmount(file_system_info, error));
+ }
}
} // namespace file_system_provider

Powered by Google App Engine
This is Rietveld 408576698