gdata: Support opening zip file on Google Docs.

chrome/browser/chromeos/extensions/file_browser_private_api.cc

Index: chrome/browser/chromeos/extensions/file_browser_private_api.cc
diff --git a/chrome/browser/chromeos/extensions/file_browser_private_api.cc b/chrome/browser/chromeos/extensions/file_browser_private_api.cc
index 907488eb12f95368737211373f03538452c4e558..4aca2cc38c3727bd225050f36bc726c656a8a0de 100644
--- a/chrome/browser/chromeos/extensions/file_browser_private_api.cc
+++ b/chrome/browser/chromeos/extensions/file_browser_private_api.cc
@@ -1015,14 +1015,42 @@ void AddMountFunction::GetLocalPathsResponseOnUIThread(
   }
 
 #if defined(OS_CHROMEOS)
-  FilePath source_file = files[0].path;
-  DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
-  // MountPath() takes a std::string.
-  disk_mount_manager->MountPath(source_file.AsUTF8Unsafe(),
-      DiskMountManager::MountTypeFromString(mount_type_str));
+  const FilePath& source_path = files[0].path;
+  const FilePath::StringType& display_name = files[0].display_name;
+  // Check if the source path is under GData cache directory.
+  gdata::GDataSystemService* system_service =
+      gdata::GDataSystemServiceFactory::GetForProfile(profile_);
+  gdata::GDataFileSystem* file_system =
+      system_service ? system_service->file_system() : NULL;
+  if (file_system && file_system->IsUnderGDataCacheDirectory(source_path)) {
+    file_system->SetMountedState(
+        source_path,
+        true,
+        base::Bind(&AddMountFunction::OnMountedStateSet, this, mount_type_str,
+                   display_name));
+  } else {
+    OnMountedStateSet(mount_type_str, FilePath::StringType(),

[satorux1, 2012/04/24 20:46:53]

can you add some comment why we are passing  FilePath::StringType()?

[hshi, 2012/04/24 21:18:22]

I think I should actually pass the display_name here. The OnMountedStateSet
expects to get the filename (base name) for this argument.

[satorux1, 2012/04/24 22:54:15]

Nice. i caught something. :)

+                      base::PLATFORM_FILE_OK, source_path);
+  }
+#else
+  SendResponse(true);
 #endif  // defined(OS_CHROMEOS)
+}
 
+void AddMountFunction::OnMountedStateSet(const std::string& mount_type,
+                                         const FilePath::StringType& file_name,
+                                         base::PlatformFileError error,
+                                         const FilePath& file_path) {
+#if defined(OS_CHROMEOS)
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
+  // MountPath() takes a std::string.
+  disk_mount_manager->MountPath(file_path.AsUTF8Unsafe(),
+                                FilePath(file_name).Extension(),
+                                DiskMountManager::MountTypeFromString(
+                                    mount_type));
   SendResponse(true);
+#endif  // defined(OS_CHROMEOS)
 }
 
 RemoveMountFunction::RemoveMountFunction() {
@@ -1058,10 +1086,48 @@ void RemoveMountFunction::GetLocalPathsResponseOnUIThread(
     return;
   }
 #if defined(OS_CHROMEOS)
-  DiskMountManager::GetInstance()->UnmountPath(files[0].path.value());
-#endif
+  DiskMountManager* disk_mount_manager = DiskMountManager::GetInstance();
+  if (!disk_mount_manager) {
+    SendResponse(false);
+    return;
+  }
 
+  // Look up source path from mount path.
+  const std::string& mount_path = files[0].path.value();
+  FilePath source_path;
+  const DiskMountManager::MountPointMap& mount_points =
+      disk_mount_manager->mount_points();
+  DiskMountManager::MountPointMap::const_iterator it =
+      mount_points.find(mount_path);
+  if (it != mount_points.end()) {

[satorux1, 2012/04/24 20:46:53]

nit: remove {} per the chromium style.

+    source_path = FilePath(it->second.source_path);
+  }

[hshi, 2012/04/24 19:49:33]

Please review this patch set #5. My previous patch set #4 causes the browser
tests (ExtensionFileBroswerPrivateApiTest) to fail because it alters the
RemoveMount behavior.

Here we should not bail out prematurally if the source_path associated with the
mount_path does not exist. The test still expects a call to
disk_mount_manager->UnmountPath as long as the mount_path itself is non-empty.

[hshi, 2012/04/24 21:18:22]

Toni told me that it is necessary after all to bail out if the corresponding
source_path does not exist. He suggested me to modify the unit test instead.
(Previously the unit test was incorrectly unmounting a non-existing mount path).

On 2012/04/24 19:49:33, hshi wrote:

+  // Unmount archive.
+  disk_mount_manager->UnmountPath(mount_path);

[satorux1, 2012/04/24 20:46:53]

Please move this to right after mount_path is computed:

const std::string& mount_path = files[0].path.value();

In other words, we don't need to compute|source_path| to unmount this.

[hshi, 2012/04/24 21:18:22]

I've added comments to explain this.

On 2012/04/24 20:46:53, satorux1 wrote:

+  // Check if the source path is under GData cache directory.
+  gdata::GDataSystemService* system_service =
+      gdata::GDataSystemServiceFactory::GetForProfile(profile_);
+  gdata::GDataFileSystem* file_system =
+      system_service ? system_service->file_system() : NULL;
+  if (file_system && file_system->IsUnderGDataCacheDirectory(source_path)) {
+    file_system->SetMountedState(
+        source_path,
+        false,
+        base::Bind(&RemoveMountFunction::OnMountedStateSet, this));
+  } else {
+    OnMountedStateSet(base::PLATFORM_FILE_OK, source_path);
+  }
+#else
   SendResponse(true);
+#endif  // defined(OS_CHROMEOS)
+}
+
+void RemoveMountFunction::OnMountedStateSet(base::PlatformFileError error,
+                                            const FilePath& file_path) {
+#if defined(OS_CHROMEOS)
+  // Ignore the file_path argument for now.

[satorux1, 2012/04/24 20:46:53]

remove this comment and change the parameter to:

const FilePath& /* file_path */) {

+  SendResponse(true);
+#endif  // defined(OS_CHROMEOS)
 }
 
 GetMountPointsFunction::GetMountPointsFunction() {