Cleanup: Make most of FileSystemOperationContext setters private

    
    - To make it clear which value might be changed during the
      execution (currently remaining quota bytes (allowed_bytes_growth)
      is the only value that could be updated
    - To make it clear when one operation context can be shared
      by multiple file operations
    
    BUG=none
    TEST=existing tests


Review URL: https://chromiumcodereview.appspot.com/12045014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@178298 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 43 insertions, 33 deletions

diff --git a/webkit/fileapi/file_system_operation_context.cc b/webkit/fileapi/file_system_operation_context.cc
index 27103b5..9597954 100644
--- a/webkit/fileapi/file_system_operation_context.cc
+++ b/webkit/fileapi/file_system_operation_context.cc
@@ -5,6 +5,7 @@
 #include "webkit/fileapi/file_system_operation_context.h"
 
 #include "base/sequenced_task_runner.h"
+#include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_task_runners.h"
 
 namespace fileapi {
diff --git a/webkit/fileapi/file_system_operation_context.h b/webkit/fileapi/file_system_operation_context.h
index 490a534..2cd2c6b 100644
--- a/webkit/fileapi/file_system_operation_context.h
+++ b/webkit/fileapi/file_system_operation_context.h
@@ -5,52 +5,43 @@
 #ifndef WEBKIT_FILEAPI_FILE_SYSTEM_OPERATION_CONTEXT_H_
 #define WEBKIT_FILEAPI_FILE_SYSTEM_OPERATION_CONTEXT_H_
 
-#include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
-#include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/media/mtp_device_file_system_config.h"
 #include "webkit/fileapi/task_runner_bound_observer_list.h"
 #include "webkit/storage/webkit_storage_export.h"
 
-#if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-#include "webkit/fileapi/media/mtp_device_delegate.h"
-#endif
-
 namespace base {
 class SequencedTaskRunner;
 }
 
 namespace fileapi {
 
+class FileSystemContext;
 class MediaPathFilter;
 
 // A context class which is carried around by FileSystemOperation and
-// its delegated tasks. It is ok to reuse the same instance for multiple
-// file system operations as far as they're supposed to share one operation
-// context.
-//
-// This class should only have simple copyable fields and basic
-// setter/getters for them.
+// its delegated tasks. It is valid to reuse one context instance across
+// multiple operations as far as those operations are supposed to share
+// the same context (e.g. use the same task runner, share the quota etc).
+// Note that the remaining quota bytes (allowed_bytes_growth) may be
+// updated during the execution of write operations.
 class WEBKIT_STORAGE_EXPORT_PRIVATE FileSystemOperationContext {
  public:
   explicit FileSystemOperationContext(FileSystemContext* context);
   ~FileSystemOperationContext();
 
   FileSystemContext* file_system_context() const {
-    return file_system_context_.get();
+    return file_system_context_;
   }
 
+  // Updates the current remaining quota.
   void set_allowed_bytes_growth(const int64& allowed_bytes_growth) {
     allowed_bytes_growth_ = allowed_bytes_growth;
   }
+
+  // Returns the current remaining quota.
   int64 allowed_bytes_growth() const { return allowed_bytes_growth_; }
 
 #if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
-  // Initializes |mtp_device_delegate_url_| on the IO thread.
-  void set_mtp_device_delegate_url(const std::string& delegate_url) {
-    mtp_device_delegate_url_ = delegate_url;
-  }
-
   // Reads |mtp_device_delegate_url_| on |task_runner_|.
   const std::string& mtp_device_delegate_url() const {
     return mtp_device_delegate_url_;
@@ -62,6 +53,25 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE FileSystemOperationContext {
     return task_runner_.get();
   }
 
+  MediaPathFilter* media_path_filter() { return media_path_filter_; }
+  ChangeObserverList* change_observers() { return &change_observers_; }
+  AccessObserverList* access_observers() { return &access_observers_; }
+  UpdateObserverList* update_observers() { return &update_observers_; }
+
+ private:
+  // Only MountPointProviders can access these setters.
+  friend class CrosMountPointProvider;
+  friend class IsolatedMountPointProvider;
+  friend class SandboxMountPointProvider;
+  friend class TestMountPointProvider;
+
+  // Tests also need access to some setters.
+  friend class FileSystemQuotaClientTest;
+  friend class LocalFileSystemOperationTest;
+  friend class LocalFileSystemOperationWriteTest;
+  friend class LocalFileSystemTestOriginHelper;
+  friend class ObfuscatedFileUtilTest;
+
   // Overrides TaskRunner which the operation is performed on.
   // file_system_context_->task_runners()->file_task_runner() is used otherwise.
   void set_task_runner(base::SequencedTaskRunner* task_runner);
@@ -69,28 +79,23 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE FileSystemOperationContext {
   void set_media_path_filter(MediaPathFilter* media_path_filter) {
     media_path_filter_ = media_path_filter;
   }
-
-  MediaPathFilter* media_path_filter() {
-    return media_path_filter_;
-  }
-
   void set_change_observers(const ChangeObserverList& list) {
     change_observers_ = list;
   }
-  ChangeObserverList* change_observers() { return &change_observers_; }
-
   void set_access_observers(const AccessObserverList& list) {
     access_observers_ = list;
   }
-  AccessObserverList* access_observers() { return &access_observers_; }
-
   void set_update_observers(const UpdateObserverList& list) {
     update_observers_ = list;
   }
-  UpdateObserverList* update_observers() { return &update_observers_; }
+#if defined(SUPPORT_MTP_DEVICE_FILESYSTEM)
+  // Initializes |mtp_device_delegate_url_| on the IO thread.
+  void set_mtp_device_delegate_url(const std::string& delegate_url) {
+    mtp_device_delegate_url_ = delegate_url;
+  }
+#endif
 
- private:
-  scoped_refptr<FileSystemContext> file_system_context_;
+  FileSystemContext* file_system_context_;
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   int64 allowed_bytes_growth_;
diff --git a/webkit/fileapi/local_file_system_operation.cc b/webkit/fileapi/local_file_system_operation.cc
index ab07cf2..1aa7071 100644
--- a/webkit/fileapi/local_file_system_operation.cc
+++ b/webkit/fileapi/local_file_system_operation.cc
@@ -453,7 +453,8 @@ void LocalFileSystemOperation::CopyInForeignFile(
 LocalFileSystemOperation::LocalFileSystemOperation(
     FileSystemContext* file_system_context,
     scoped_ptr<FileSystemOperationContext> operation_context)
-    : operation_context_(operation_context.Pass()),
+    : file_system_context_(file_system_context),
+      operation_context_(operation_context.Pass()),
       src_util_(NULL),
       dest_util_(NULL),
       is_cross_operation_(false),
diff --git a/webkit/fileapi/local_file_system_operation.h b/webkit/fileapi/local_file_system_operation.h
index d6ba7c6..e80e91f 100644
--- a/webkit/fileapi/local_file_system_operation.h
+++ b/webkit/fileapi/local_file_system_operation.h
@@ -115,7 +115,7 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
       scoped_ptr<FileSystemOperationContext> operation_context);
 
   FileSystemContext* file_system_context() const {
-    return operation_context_->file_system_context();
+    return file_system_context_;
   }
 
   FileSystemOperationContext* operation_context() const {
@@ -230,6 +230,8 @@ class WEBKIT_STORAGE_EXPORT LocalFileSystemOperation
   // Returns false if there's another inflight pending operation.
   bool SetPendingOperationType(OperationType type);
 
+  scoped_refptr<FileSystemContext> file_system_context_;
+
   scoped_ptr<FileSystemOperationContext> operation_context_;
   FileSystemFileUtil* src_util_;  // Not owned.
   FileSystemFileUtil* dest_util_;  // Not owned.
diff --git a/webkit/fileapi/media/device_media_file_util.cc b/webkit/fileapi/media/device_media_file_util.cc
index 3de3012..14304ed 100644
--- a/webkit/fileapi/media/device_media_file_util.cc
+++ b/webkit/fileapi/media/device_media_file_util.cc
@@ -11,6 +11,7 @@
 #include "webkit/fileapi/isolated_context.h"
 #include "webkit/fileapi/media/filtering_file_enumerator.h"
 #include "webkit/fileapi/media/media_path_filter.h"
+#include "webkit/fileapi/media/mtp_device_delegate.h"
 #include "webkit/fileapi/media/mtp_device_map_service.h"
 
 using base::PlatformFile;