gdata: Apply correct mount label when mounting archives in GData

chrome/browser/chromeos/extensions/file_browser_event_router.cc

 // 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.
 
 #include "chrome/browser/chromeos/extensions/file_browser_event_router.h"
 
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
@@ skipping 350 matching lines @@
   ListValue args;
   DictionaryValue* mount_info_value = new DictionaryValue();
   args.Append(mount_info_value);
   mount_info_value->SetString("eventType",
       event == DiskMountManager::MOUNTING ? "mount" : "unmount");
   mount_info_value->SetString("status", MountErrorToString(error_code));
   mount_info_value->SetString(
       "mountType",
       DiskMountManager::MountTypeToString(mount_info.mount_type));
 
-  if (mount_info.mount_type == chromeos::MOUNT_TYPE_ARCHIVE ||
-      mount_info.mount_type == chromeos::MOUNT_TYPE_GDATA) {
+  // Determine if the source path is an archive on gdata.
+  bool is_archive_on_gdata = false;
+  FilePath source_path(mount_info.source_path);
+  if (mount_info.mount_type == chromeos::MOUNT_TYPE_ARCHIVE) {
+    gdata::GDataSystemService* system_service =
+        gdata::GDataSystemServiceFactory::GetForProfile(profile_);
+    gdata::GDataFileSystem* file_system =

[tbarzic, 2012/04/27 05:44:21]

This should probably be separate function.

On the other hand, this may not be needed..se my other comment here:

[hshi, 2012/04/27 18:48:56]

Removed as suggested.

+        system_service ? system_service->file_system() : NULL;
+    is_archive_on_gdata =
+        file_system && file_system->IsUnderGDataCacheDirectory(source_path);
+  }
+
+  if (!is_archive_on_gdata &&
+      (mount_info.mount_type == chromeos::MOUNT_TYPE_ARCHIVE ||
+       mount_info.mount_type == chromeos::MOUNT_TYPE_GDATA)) {
     GURL source_url;
     if (file_manager_util::ConvertFileToFileSystemUrl(profile_,

[tbarzic, 2012/04/27 05:44:21]

We do this conversion for archives file_manager can map mount requests it sends
with onMountCompleted event (file_manager only knows original source url).
So, if we send archive's source path in a callback from
fileBrowserPrivate.addMount, we can use the source path to map mount requests
and mount events (and set archives' sourceUrl to mount_info.source_path here).

We could probably do the same for the gdata mount point and get rid of this code
all together

Also, if we go this way, it would make sense to rename "sourceUrl" to
"sourcePath"

[hshi, 2012/04/27 18:48:56]

Done.

-            FilePath(mount_info.source_path),
+            source_path,
             file_manager_util::GetFileBrowserExtensionUrl().GetOrigin(),
             &source_url)) {
       mount_info_value->SetString("sourceUrl", source_url.spec());
     } else {
       // If mounting of gdata moutn point failed, we may not be able to convert
       // source path to source url, so let just send empty string.
       DCHECK(mount_info.mount_type == chromeos::MOUNT_TYPE_GDATA &&
              error_code != chromeos::MOUNT_ERROR_NONE);
       mount_info_value->SetString("sourceUrl", "");
     }
@@ skipping 37 matching lines @@
     const DiskMountManager::Disk* disk) {
   VLOG(1) << "Disk added: " << disk->device_path();
   if (disk->device_path().empty()) {
     VLOG(1) << "Empty system path for " << disk->device_path();
     return;
   }
 
   // If disk is not mounted yet and it has media, give it a try.
   if (disk->mount_path().empty() && disk->has_media()) {
     // Initiate disk mount operation. MountPath auto-detects the filesystem
-    // format if the second argument is empty.
+    // format if the second argument is empty. The third argument (mount label)
+    // is not used in a disk mount operation.
     DiskMountManager::GetInstance()->MountPath(
-        disk->device_path(), std::string(), chromeos::MOUNT_TYPE_DEVICE);
+        disk->device_path(), std::string(), std::string(),
+        chromeos::MOUNT_TYPE_DEVICE);
   } else {
     // Either the disk was mounted or it has no media. In both cases we don't
     // want the Scanning notification to persist.
     notifications_->HideNotification(FileBrowserNotifications::DEVICE,
                                      disk->system_path_prefix());
   }
   DispatchDiskEvent(disk, true);
 }
 
 void FileBrowserEventRouter::OnDiskRemoved(
@@ skipping 46 matching lines @@
     const std::string& device_path, bool success) {
   if (success) {
     notifications_->HideNotification(FileBrowserNotifications::FORMAT_START,
                                      device_path);
     notifications_->ShowNotification(FileBrowserNotifications::FORMAT_SUCCESS,
                                      device_path);
     // Hide it after a couple of seconds.
     notifications_->HideNotificationDelayed(
         FileBrowserNotifications::FORMAT_SUCCESS, device_path, 4000);
     // MountPath auto-detects filesystem format if second argument is empty.
+    // The third argument (mount path) is not used in a disk mount operation.

[tbarzic, 2012/04/27 05:44:21]

Isn't the third arg label to be used for the mounted archive (mount path should
be determined by cros-disks)?

[hshi, 2012/04/27 18:48:56]

Sorry for the typo, the 3rd arg should be mount label, not mount path.

     DiskMountManager::GetInstance()->MountPath(device_path, std::string(),
+                                               std::string(),
                                                chromeos::MOUNT_TYPE_DEVICE);
   } else {
     notifications_->HideNotification(FileBrowserNotifications::FORMAT_START,
                                      device_path);
     notifications_->ShowNotification(FileBrowserNotifications::FORMAT_FAIL,
                                      device_path);
   }
 }
 
 // FileBrowserEventRouter::WatcherDelegate methods.
@@ skipping 120 matching lines @@
   return scoped_refptr<RefcountedProfileKeyedService>(
       new FileBrowserEventRouter(profile));
 }
 
 bool FileBrowserEventRouterFactory::ServiceHasOwnInstanceInIncognito() {
   // Explicitly and always allow this router in guest login mode.   see
   // chrome/browser/profiles/profile_keyed_base_factory.h comment
   // for the details.
   return true;
 }