Cleanup: Removing FileSystemPathManager

webkit/fileapi/file_system_context.h

 // Copyright (c) 2011 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 WEBKIT_FILEAPI_FILE_SYSTEM_CONTEXT_H_
 #define WEBKIT_FILEAPI_FILE_SYSTEM_CONTEXT_H_
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "webkit/fileapi/file_system_types.h"
 #include "webkit/quota/special_storage_policy.h"
 
 class FilePath;
 class GURL;
 
 namespace base {
 class MessageLoopProxy;
 }
 
 namespace quota {
 class QuotaManagerProxy;
 }
 
 namespace fileapi {
 
+class ExternalFileSystemMountPointProvider;
 class FileSystemContext;
+class FileSystemFileUtil;
+class FileSystemMountPointProvider;
+class FileSystemOptions;
 class FileSystemPathManager;
 class FileSystemQuotaUtil;
 class SandboxMountPointProvider;
 
 struct DefaultContextDeleter;
 
 // This class keeps and provides a file system context for FileSystem API.
 // An instance of this class is created and owned by profile.
 class FileSystemContext
     : public base::RefCountedThreadSafe<FileSystemContext,
                                         DefaultContextDeleter> {
  public:
+  // The ownership of |options| is transferred from the caller.

[ericu, 2011/12/20 18:12:59]

It's a very small class.  Perhaps we should just pass it by value?

[kinuko, 2011/12/21 13:00:50]

The point is I used virtual methods for allowing browser code to implement them.
I could also turn this class into a simple struct but I wanted to avoid using
raw boolean values.

[ericu, 2011/12/21 22:22:48]

I meant that the class, with its virtual methods, could be passed by value to
avoid the mental overhead of lifetime management of a dynamically-allocated
object.  But if you really prefer it this way, I don't feel very strongly about
it.

[kinuko, 2011/12/22 05:28:39]

Sorry I might be missing something but I cannot imagine what type we should
receive for the passed value.  (The BrowserFileSystemOption is defined in
content but FileSystemContext doesn't know the subclass(es) so that it cannot
tell how much storage it needs for the value.)

[kinuko, 2011/12/27 17:33:27]

Ok I stopped using virtual methods and converted them to use simple pass-by
values.

   FileSystemContext(
-      scoped_refptr<base::MessageLoopProxy> file_message_loop,
-      scoped_refptr<base::MessageLoopProxy> io_message_loop,
-      scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy,
+      base::MessageLoopProxy* file_message_loop,
+      base::MessageLoopProxy* io_message_loop,
+      quota::SpecialStoragePolicy* special_storage_policy,

[ericu, 2011/12/20 18:12:59]

Why are these no longer scoped_refptrs?  Should they not have been all along?

[kinuko, 2011/12/21 13:00:50]

They are all cast'ed to scoped_refptr in the initializers so they should be just
fine.  (There's no strong reason to turn them into raw pointers for now though.
Subtle benefits from this change are the code looks simpler and we can more
easily pass NULL pointers)

[ericu, 2011/12/21 22:22:48]

Making the code look simpler than it is may cause confusion, and these values
won't often be NULL.  When they are, it's probably worth the explicit
scoped_refptr<base::MessageLoopProxy>() to make it clear.  I'd prefer not to
make this change.

[kinuko, 2011/12/22 05:28:39]

Another subtle benefit is we can save refcount churn at arg passing (though it's
not the only way to do so).  Recently I saw similar changes elsewhere so I
thought this change'd be preferable.  For example SpecialStoragePolicy is almost
always passed as a raw pointer)  I can revert this (personally I have no strong
opinion on this) but I feel changing this to pointer might make the code more
consistent with other parts.

[kinuko, 2011/12/27 17:33:27]

I reverted these changes for now.  We can change them later if we really want
to.

       quota::QuotaManagerProxy* quota_manager_proxy,
       const FilePath& profile_path,
-      bool is_incognito,
-      bool allow_file_access_from_files,
-      FileSystemPathManager* path_manager);
+      FileSystemOptions* options);
   ~FileSystemContext();
 
   // This method can be called on any thread.
   bool DeleteDataForOriginOnFileThread(const GURL& origin_url);
   bool DeleteDataForOriginAndTypeOnFileThread(const GURL& origin_url,
                                               FileSystemType type);
 
-  FileSystemPathManager* path_manager() const { return path_manager_.get(); }
   quota::QuotaManagerProxy* quota_manager_proxy() const {
     return quota_manager_proxy_.get();
   }
 
   // Returns a quota util for a given filesystem type.  This may
   // return NULL if the type does not support the usage tracking or
   // it is not a quota-managed storage.
   FileSystemQuotaUtil* GetQuotaUtil(FileSystemType type) const;
 
+  // Returns the appropriate FileUtil instance for the given |type|.
+  // This may return NULL if it is given an invalid or unsupported filesystem
+  // type.
+  FileSystemFileUtil* GetFileUtil(FileSystemType type) const;
+
+  // Returns the mount point provider instance for the given |type|.
+  // This may return NULL if it is given an invalid or unsupported filesystem
+  // type.
+  FileSystemMountPointProvider* GetMountPointProvider(
+      FileSystemType type) const;
+
+  // Returns a FileSystemMountPointProvider instance for sandboxed filesystem
+  // types (e.g. TEMPORARY or PERSISTENT).  This is equivalent to calling
+  // GetMountPointProvider(kFileSystemType{Temporary, Persistent}).
+  SandboxMountPointProvider* sandbox_provider() const;
+
+  // Returns a FileSystemMountPointProvider instance for external filesystem
+  // type, which is used only by chromeos for now.  This is equivalent to
+  // calling GetMountPointProvider(kFileSystemTypeExternal).
+  ExternalFileSystemMountPointProvider* external_provider() const;
+
  private:
   friend struct DefaultContextDeleter;
   void DeleteOnCorrectThread() const;
-  SandboxMountPointProvider* sandbox_provider() const;
 
   scoped_refptr<base::MessageLoopProxy> file_message_loop_;
   scoped_refptr<base::MessageLoopProxy> io_message_loop_;
 
   scoped_refptr<quota::QuotaManagerProxy> quota_manager_proxy_;
 
-  scoped_ptr<FileSystemPathManager> path_manager_;
+  // Mount point providers.
+  scoped_ptr<SandboxMountPointProvider> sandbox_provider_;
+  scoped_ptr<ExternalFileSystemMountPointProvider> external_provider_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(FileSystemContext);
 };
 
 struct DefaultContextDeleter {
   static void Destruct(const FileSystemContext* context) {
     context->DeleteOnCorrectThread();
   }
 };
 
 }  // namespace fileapi
 
 #endif  // WEBKIT_FILEAPI_FILE_SYSTEM_CONTEXT_H_