Wired GDataFileSystem::GetFile() method with internal cache.

chrome/browser/chromeos/gdata/gdata_file_system.h

 // Copyright (c) 2012 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 CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILE_SYSTEM_H_
 #define CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILE_SYSTEM_H_
 
 #include <sys/stat.h>
 
 #include <map>
@@ skipping 250 matching lines @@
   // Gets the cache state of file corresponding to |resource_id| and |md5| if it
   // exists on disk.
   // Initializes cache if it has not been initialized.
   // Upon completion, |callback| is invoked on the thread where this method was
   // called with the cache state if file exists in cache or CACHE_STATE_NONE
   // otherwise.
   void GetCacheState(const std::string& resource_id,
                      const std::string& md5,
                      const GetCacheStateCallback& callback);
 
-  // Finds file object by |file_path| and returns its |file_info|.
-  // Returns true if file was found.
+  // Finds file object by |file_path| and returns its |file_info| and optionally
+  // its |resource_id|, |md5| and |content_url| properties. Returns true if file
+  // was found.
   bool GetFileInfoFromPath(const FilePath& gdata_file_path,
-                           base::PlatformFileInfo* file_info);
+                           base::PlatformFileInfo* file_info,
+                           std::string* resource_id,
+                           std::string* md5,
+                           GURL* content_url);

[satorux1, 2012/03/20 04:51:17]

Having four output parameters looks unclean. Cannot we define a struct that
includes all of them, like

struct GDataFileInfo {
  file_info;
  resource_id;
  md5;
  content_url;
}

this is more like subset of GDataFileBase.

[zel, 2012/03/20 05:24:25]

Then I have to copy way more than I really want every time I look up files. This
way I can avoid extra memory copies from gdata_file_system_proxy.cc layer and
still reuse the same function.

[satorux1, 2012/03/20 05:35:14]

I see. Then we could define two separate functions 

GetFileInfoFromPath(file_path, file_info); -- the original
GetFileAndCacheInfoFromPath(file_path, file_info, x, y, z); -- the new one

However, the original function doesn't seem to be used from many places so,
having only the complex version would be ok.

 
   // Returns the tmp sub-directory under gdata cache directory, i.e.
   // <user_profile_dir>/GCache/v1/tmp
   FilePath GetGDataCacheTmpDirectory() {
     return cache_paths_[CACHE_TYPE_TMP];
   }
 
   // Fetches the user's Account Metadata to find out current quota information
   // and returns it to the callback.
   void GetAvailableSpace(const GetAvailableSpaceCallback& callback);
@@ skipping 29 matching lines @@
                           const FileOperationCallback& callback);
     ~CreateDirectoryParams();
 
     const FilePath created_directory_path;
     const FilePath target_directory_path;
     const bool is_exclusive;
     const bool is_recursive;
     FileOperationCallback callback;
   };
 
+  // Defines set of parameters passed to intermediate callbacks during
+  // execution of GetFile() method.
+  struct GetFileFromCacheParams {
+    GetFileFromCacheParams(const FilePath& virtual_file_path,
+        const FilePath& local_tmp_path,
+        const GURL& content_url,
+        const std::string& resource_id,
+        const std::string& md5,
+        scoped_refptr<base::MessageLoopProxy> proxy,
+        const GetFileCallback& callback);
+    ~GetFileFromCacheParams();
+
+    FilePath virtual_file_path;
+    FilePath local_tmp_path;
+    GURL content_url;
+    std::string resource_id;
+    std::string md5;
+    scoped_refptr<base::MessageLoopProxy> proxy;
+    const GetFileCallback callback;
+  };
+
   // Callback similar to FileOperationCallback but with a given
   // |file_path|.
   typedef base::Callback<void(base::PlatformFileError error,
                               const FilePath& file_path)>
       FilePathUpdateCallback;
 
   GDataFileSystem(Profile* profile,
                   DocumentsServiceInterface* documents_service);
   virtual ~GDataFileSystem();
 
@@ skipping 115 matching lines @@
       const GURL& document_url);
 
   // Callback for handling directory create requests.
   void OnCreateDirectoryCompleted(
       const CreateDirectoryParams& params,
       GDataErrorCode status,
       scoped_ptr<base::Value> created_entry);
 
   // Callback for handling file downloading requests.
   void OnFileDownloaded(
-    const GetFileCallback& callback,
+    const GetFileFromCacheParams& params,
     GDataErrorCode status,
     const GURL& content_url,
-    const FilePath& temp_file);
+    const FilePath& downloaded_file_path);
+
+  // Callback for handling internal StoreToCache() calls after downloading
+  // file content.
+  void OnDownloadStoredToCache(base::PlatformFileError error,
+                               const std::string& resource_id,
+                               const std::string& md5);
 
   // Callback for handling file upload initialization requests.
   void OnUploadLocationReceived(
       const InitiateUploadCallback& callback,
       scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
       GDataErrorCode code,
       const GURL& upload_location);
 
   // Callback for handling file upload resume requests.
   void OnResumeUpload(const ResumeUploadCallback& callback,
@@ skipping 96 matching lines @@
   // The functionalities of GData blob cache include:
   // - stores downloaded gdata files on disk, indexed by the resource_id and md5
   //   of the gdata file.
   // - provides absolute path for files to be cached or cached.
   // - updates the cached file on disk after user has edited it locally
   // - handles eviction when disk runs out of space
   // - uploads dirty files to gdata server.
   // - etc.
 
   // Returns absolute path of the file if it were cached or to be cached.
   FilePath GetCacheFilePath(const std::string& resource_id,

[satorux1, 2012/03/20 04:51:17]

is_local is not documented. what is this for?

[zel, 2012/03/20 05:24:25]

switched to self-describing enum instead.

                             const std::string& md5,
                             CacheSubdir subdir_id,
                             bool is_local);
 
   // Stores |source_path| corresponding to |resource_id| and |md5| to cache.
   // Initializes cache if it has not been initialized.
   // Upon completion, |callback| is invoked on the thread where this method was
   // called.
-  // TODO(kuan): When URLFetcher can save response to a specified file (as
-  // opposed to only temporary file currently), remove |source_path| parameter.
   void StoreToCache(const std::string& resource_id,
                     const std::string& md5,
                     const FilePath& source_path,
                     const CacheOperationCallback& callback);
 
   // Checks if file corresponding to |resource_id| and |md5| exist on disk and
   // can be accessed i.e. not corrupted by previous file operations that didn't
   // complete for whatever reasons.
   // Initializes cache if it has not been initialized.
   // Upon completion, |callback| is invoked on the thread where this method was
@@ skipping 85 matching lines @@
                       mode_t mode_bits,
                       const CacheOperationCallback& callback);
 
   // Helper function for OnFilePinned() and OnFileUnpinned().
   void OnCacheStatusModified(base::PlatformFileError error,
                              const std::string& resource_id,
                              const std::string& md5,
                              mode_t mode_bits,
                              const CacheOperationCallback& callback);
 
+  // Helper function for internally handling responses from GetFromCache()
+  // calls during processing of GetFile() request.
+  void OnGetFileFromCache(const GetFileFromCacheParams& params,
+                          base::PlatformFileError error,
+                          const std::string& resource_id,
+                          const std::string& md5,
+                          const FilePath& gdata_file_path,
+                          const FilePath& cache_file_path);
+
   // Callback for GetCacheState that gets cache state of file corresponding to
   // |resource_id| and |md5|.
   void OnGetCacheState(const std::string& resource_id,
                        const std::string& md5,
                        const GetCacheStateCallback& callback);
 
   // Cache internal helper functions.
 
   void GetFromCacheInternal(const std::string& resource_id,
                             const std::string& md5,
                             const FilePath& gdata_file_path,
                             const GetFromCacheCallback& callback);
 
   // Unsafe (unlocked) version of InitializeCacheIfnecessary method.
   void UnsafeInitializeCacheIfNecessary();
 
   // Helper function used to perform file search on the calling thread of
   // FindFileByPath() request.
   void FindFileByPathOnCallingThread(const FilePath& search_file_path,
                                      const FindFileCallback& callback);
 
-    scoped_ptr<GDataRootDirectory> root_;
+  scoped_ptr<GDataRootDirectory> root_;
 
   base::Lock lock_;
 
   // The profile hosts the GDataFileSystem.
   Profile* profile_;
 
   // The document service for the GDataFileSystem.
   scoped_ptr<DocumentsServiceInterface> documents_service_;
 
   // File content uploader.
@@ skipping 41 matching lines @@
   virtual ~GDataFileSystemFactory();
 
   // ProfileKeyedServiceFactory:
   virtual ProfileKeyedService* BuildServiceInstanceFor(
       Profile* profile) const OVERRIDE;
 };
 
 }  // namespace gdata
 
 #endif  // CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILE_SYSTEM_H_