gdata: Support mouting archive file in GData cache

chrome/browser/chromeos/gdata/gdata_file_system.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.cc b/chrome/browser/chromeos/gdata/gdata_file_system.cc
index 4b55a6532f71d33b9b4e118ad39a47bc6b370396..e483fa81aad81e7780684782b192bf3d4f074d35 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.cc
@@ -53,6 +53,7 @@ const FilePath::CharType kGDataRootDirectory[] = FILE_PATH_LITERAL("gdata");
 const char kFeedField[] = "feed";
 const char kWildCard[] = "*";
 const char kLocallyModifiedFileExtension[] = "local";
+const char kMountedArchiveFileExtension[] = "mounted";
 
 const FilePath::CharType kGDataCacheVersionDir[] = FILE_PATH_LITERAL("v1");
 const FilePath::CharType kGDataCacheMetaDir[] = FILE_PATH_LITERAL("meta");
@@ -1813,6 +1814,11 @@ bool GDataFileSystem::GetFileInfoFromPath(
   return true;
 }
 
+bool GDataFileSystem::IsUnderGDataCacheDirectory(const FilePath& path) const {
+  return gdata_cache_path_ == path ||
+         gdata_cache_path_.IsParent(path);

[Ben Chan, 2012/04/17 20:57:53]

nit: can this fit in one line?

also, please add unit test.

+}
+
 FilePath GDataFileSystem::GetGDataCacheTmpDirectory() const {
   return cache_paths_[GDataRootDirectory::CACHE_TYPE_TMP];
 }
@@ -1961,6 +1967,96 @@ void GDataFileSystem::SetPinState(const FilePath& file_path, bool to_pin,
     Unpin(resource_id, md5, cache_callback);
 }
 
+void GDataFileSystem::SetMountedState(const FilePath& file_path,
+                                      const std::string& file_name,

[tbarzic, 2012/04/17 21:33:13]

I don't see that you really use file_name and mount_type..
Why don't you bind it to the callback before you call the method.

+                                      const std::string& mount_type,
+                                      const SetMountedStateCallback& callback) {
+  std::string resource_id = file_path.BaseName().RemoveExtension().value();

[tbarzic, 2012/04/17 21:33:13]

I think you have to acquire |lock_| before you access cache.
Same in ClearMountedState

+  GDataRootDirectory::CacheEntry* entry = root_->GetCacheEntry(resource_id,
+                                                               std::string());

[Ben Chan, 2012/04/17 20:57:53]

Should we pass md5 to GetCacheEntry here?

+  if (!entry || entry->IsMounted()) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE,
+          base::Bind(callback, base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
+              FilePath(), std::string(), std::string()));
+    }
+    return;
+  }

[Ben Chan, 2012/04/17 20:57:53]

nit: line break

+  base::PlatformFileError error;

[Ben Chan, 2012/04/17 20:57:53]

this can be defined in the scope below.

+  {
+    base::AutoLock lock(lock_);

[tbarzic, 2012/04/17 21:33:13]

You should move this to the method scope.

+    // Returns original path of the file in cache
+    FilePath src_path = GetCacheFilePath(resource_id, entry->md5,
+                            entry->IsPinned() ?
+                            GDataRootDirectory::CACHE_TYPE_PERSISTENT :
+                            GDataRootDirectory::CACHE_TYPE_TMP,
+                            CACHED_FILE_FROM_SERVER);
+    // Returns destination path of the file if it were to be mounted
+    FilePath dst_path = GetCacheFilePath(resource_id, entry->md5,
+                            GDataRootDirectory::CACHE_TYPE_PERSISTENT,
+                            CACHED_FILE_MOUNTED_ARCHIVE);
+    // Move cache blob to the mounted path
+    error = ModifyCacheState(src_path, dst_path,

[Ben Chan, 2012/04/17 20:57:53]

base::PlatformFileError error = ...

+                             GDataFileSystem::FILE_OPERATION_MOVE,
+                             FilePath(), false);
+    if (error == base::PLATFORM_FILE_OK) {
+      // Now that cache operation is complete, update cache map
+      entry->cache_state = GDataFile::SetCacheMounted(entry->cache_state);
+      root_->UpdateCacheMap(resource_id, entry->md5,
+                            GDataRootDirectory::CACHE_TYPE_PERSISTENT,
+                            entry->cache_state);
+    }
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE,
+          base::Bind(callback, error, dst_path, file_name, mount_type));
+    }
+  }
+}
+
+void GDataFileSystem::ClearMountedState(const FilePath& file_path,
+                                        const FileOperationCallback& callback) {

[tbarzic, 2012/04/17 21:33:13]

This looks very similar to SetMountState..

Why don't you try to merge these two in one method SetMountedState(file_path,
is_mounted, callback)

[hshi, 2012/04/17 22:36:45]

Yes I had initially attempted to merge the two, but unfortunately the
SetMountedStateCallback requires passing back the new path to the cached blob
(persistent/<resource_id>.mounted) whereas ClearMountedState does not. However I
can simplify the SetMountedState as Ben suggested and bind the file_name and
mount_type before the call.

[tbarzic, 2012/04/17 23:01:10]

when I think about it, it may have sense to pass dst_path to ClearMountedState
callback too (it can be ignored in file_browser_pricate_api for now)

+  std::string resource_id = file_path.BaseName().RemoveExtension().value();
+  GDataRootDirectory::CacheEntry* entry = root_->GetCacheEntry(resource_id,
+                                                               std::string());
+  if (!entry || !entry->IsMounted()) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE,
+          base::Bind(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND));
+    }
+    return;
+  }

[Ben Chan, 2012/04/17 20:57:53]

nit: line break

+  base::PlatformFileError error;

[Ben Chan, 2012/04/17 20:57:53]

this can be defined within the scope below

+  {
+    base::AutoLock lock(lock_);
+    // Returns mounted path of the file in cache
+    FilePath src_path = GetCacheFilePath(resource_id, entry->md5,
+                            GDataRootDirectory::CACHE_TYPE_PERSISTENT,

[Ben Chan, 2012/04/17 20:57:53]

nit: thisseems like a weird indentation

+                            CACHED_FILE_MOUNTED_ARCHIVE);
+    // Returns destination path of the file if it were to be unmounted
+    FilePath dst_path = GetCacheFilePath(resource_id, entry->md5,
+                            entry->IsPinned() ?

[Ben Chan, 2012/04/17 20:57:53]

ditto

+                            GDataRootDirectory::CACHE_TYPE_PERSISTENT :
+                            GDataRootDirectory::CACHE_TYPE_TMP,
+                            CACHED_FILE_FROM_SERVER);
+    // Move cache blob to the destination path
+    error = ModifyCacheState(src_path, dst_path,

[Ben Chan, 2012/04/17 20:57:53]

base::PlatformFileError error = ...

+                             GDataFileSystem::FILE_OPERATION_MOVE,
+                             FilePath(), false);
+    if (error == base::PLATFORM_FILE_OK) {
+      // Now that cache operation is complete, update cache map
+      entry->cache_state = GDataFile::ClearCacheMounted(entry->cache_state);
+      root_->UpdateCacheMap(resource_id, entry->md5,
+                            entry->IsPinned() ?
+                            GDataRootDirectory::CACHE_TYPE_PERSISTENT :
+                            GDataRootDirectory::CACHE_TYPE_TMP,
+                            entry->cache_state);
+    }
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, error));
+    }
+  }
+}
+
 void GDataFileSystem::OnSetPinStateCompleted(
     const FileOperationCallback& callback,
     base::PlatformFileError error,
@@ -3067,6 +3163,10 @@ FilePath GDataFileSystem::GetCacheFilePath(
     DCHECK(sub_dir_type == GDataRootDirectory::CACHE_TYPE_PERSISTENT);
     base_name += FilePath::kExtensionSeparator;
     base_name += kLocallyModifiedFileExtension;
+  } else if (file_origin == CACHED_FILE_MOUNTED_ARCHIVE) {
+    DCHECK(sub_dir_type == GDataRootDirectory::CACHE_TYPE_PERSISTENT);
+    base_name += FilePath::kExtensionSeparator;
+    base_name += kMountedArchiveFileExtension;
   } else if (!md5.empty()) {
     base_name += FilePath::kExtensionSeparator;
     base_name += GDataFileBase::EscapeUtf8FileName(md5);
@@ -3313,6 +3413,7 @@ void GDataFileSystem::GetFromCacheOnIOThreadPool(
         resource_id,
         md5,
         entry->sub_dir_type,
+        entry->IsMounted() ? CACHED_FILE_MOUNTED_ARCHIVE :

[tbarzic, 2012/04/17 21:33:13]

This looks a bit ugly (at least to me:) )

         entry->IsDirty() ? CACHED_FILE_LOCALLY_MODIFIED :
                            CACHED_FILE_FROM_SERVER);
     *error = base::PLATFORM_FILE_OK;