[fsp] Create a RequestManager per a ProvidedFileSystem instance.

chrome/browser/chromeos/file_system_provider/service.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/service.h"
 
 #include "base/files/file_path.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 "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_system.h"
 #include "webkit/browser/fileapi/external_mount_points.h"
 
 namespace chromeos {
 namespace file_system_provider {
 namespace {
 
 // Maximum number of file systems to be mounted in the same time, per profile.
 const size_t kMaxFileSystems = 16;
 
-// Default factory for provided file systems. The |event_router| nor the
-// |request_manager| arguments must not be NULL.
+// Default factory for provided file systems. The |event_router| 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);
+  return new ProvidedFileSystem(event_router, file_system_info);
 }
 
 }  // namespace
 
 Service::Service(Profile* profile)
     : profile_(profile),
       file_system_factory_(base::Bind(CreateProvidedFileSystem)),
       next_id_(1),
-      weak_ptr_factory_(this) {
-  AddObserver(&request_manager_);
-}
+      weak_ptr_factory_(this) {}
 
-Service::~Service() { STLDeleteValues(&file_system_map_); }
+Service::~Service() {}

[hashimoto, 2014/04/16 04:14:13]

Does this mean file_system_map_ will leak if Shutdown() was not called
correctly?
How about deleting values here too or having DCHECK(file_system_map_.empty())?

[mtomasz, 2014/04/16 05:08:10]

That's a good point. In tests, it would leak. I think we can safely move it to
the destructor. It calls ~RequestManager, which aborts requests. Aborting only
calls the error callback, which should be safe. Done.

 
 // static
 Service* Service::Get(content::BrowserContext* context) {
   return ServiceFactory::Get(context);
 }
 
 void Service::AddObserver(Observer* observer) {
   DCHECK(observer);
   observers_.AddObserver(observer);
 }
@@ skipping 55 matching lines @@
   //   file_system_id = 41
   //   mount_point_name = file_system_id = b33f1337-41-5aa5
   //   mount_point_path = /provided/b33f1337-41-5aa5
   ProvidedFileSystemInfo file_system_info(
       extension_id, file_system_id, file_system_name, mount_point_path);
 
   // 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);
+  ProvidedFileSystemInterface* file_system =
+      file_system_factory_.Run(event_router, file_system_info);
   DCHECK(file_system);
   file_system_map_[file_system_id] = file_system;
 
   FOR_EACH_OBSERVER(
       Observer,
       observers_,
       OnProvidedFileSystemMount(file_system_info, base::File::FILE_OK));
 
   next_id_++;
   return file_system_id;
@@ skipping 80 matching lines @@
       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 file_system_it->second;
 }
 
-void Service::Shutdown() { RemoveObserver(&request_manager_); }
+void Service::Shutdown() { STLDeleteValues(&file_system_map_); }
 
 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
 }  // namespace chromeos