Use ExtractDrivePath(path()) instead of virtual_path() for getting Drive path.

FileSystemURL::virtual_path is a superficial path specified by the user of
FileSystem API. It does not necessarily denote the 'virtual path' of Drive
files like drive/foo.txt, when the Drive filesystem is mounted onto of
another filesystem (e.g., onto an isolated filesystem). Basically,
virtual_path() should never be directly used inside the Drive filesystem.

BUG=none


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

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

chrome/browser/chromeos/drive/drive_file_system_proxy.cc

@@ -692,17 +692,7 @@ DriveFileSystemProxy::~DriveFileSystemProxy() {
 // static.
 bool DriveFileSystemProxy::ValidateUrl(
     const FileSystemURL& url, base::FilePath* file_path) {
-  // what platform you're on.
-  if (!url.is_valid() || url.type() != fileapi::kFileSystemTypeDrive)
-    return false;
-
-  // |url.virtual_path()| cannot be used directly because in the case the url is
-  // isolated file system url, the virtual path will be formatted as
-  // <isolated_file_system_id>/<file_name> and thus unusable by drive file
-  // system.
-  // TODO(kinaba): fix other uses of virtual_path() as in
-  // https://codereview.chromium.org/12483010/
-  *file_path = util::ExtractDrivePath(url.path());
+  *file_path = util::ExtractDrivePathFromFileSystemUrl(url);
   return !file_path->empty();
 }
 

chrome/browser/chromeos/drive/drive_file_system_util.cc

@@ -33,6 +33,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "net/base/escape.h"
 #include "net/base/network_change_notifier.h"
+#include "webkit/fileapi/file_system_url.h"
 
 using content::BrowserThread;
 
@@ -278,6 +279,13 @@ base::FilePath ExtractDrivePath(const base::FilePath& path) {
   return extracted;
 }
 
+base::FilePath ExtractDrivePathFromFileSystemUrl(
+    const fileapi::FileSystemURL& url) {
+  if (!url.is_valid() || url.type() != fileapi::kFileSystemTypeDrive)
+    return base::FilePath();
+  return ExtractDrivePath(url.path());
+}
+
 std::string EscapeCacheFileName(const std::string& filename) {
   // This is based on net/base/escape.cc: net::(anonymous namespace)::Escape
   std::string escaped;

chrome/browser/chromeos/drive/drive_file_system_util.h

@@ -19,6 +19,10 @@ namespace base {
 class FilePath;
 }
 
+namespace fileapi {
+class FileSystemURL;
+}
+
 namespace drive {
 
 class PlatformFileInfoProto;
@@ -112,9 +116,14 @@ base::FilePath ConvertToMyDriveNamespace(const base::FilePath& path);
 
 // Extracts the Drive path from the given path located under the Drive mount
 // point. Returns an empty path if |path| is not under the Drive mount point.
-// Examples: ExtractGDatPath("/special/drive/foo.txt") => "drive/foo.txt"
+// Examples: ExtractDrivePath("/special/drive/foo.txt") => "drive/foo.txt"
 base::FilePath ExtractDrivePath(const base::FilePath& path);
 
+// Extracts the Drive path (e.g., "drive/foo.txt") from the filesystem URL.
+// Returns an empty path if |url| does not point under Drive mount point.
+base::FilePath ExtractDrivePathFromFileSystemUrl(
+    const fileapi::FileSystemURL& url);
+
 // Escapes a file name in Drive cache.
 // Replaces percent ('%'), period ('.') and slash ('/') with %XX (hex)
 std::string EscapeCacheFileName(const std::string& filename);

chrome/browser/chromeos/drive/drive_file_system_util_unittest.cc

@@ -5,8 +5,16 @@
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 
 #include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/message_loop.h"
 #include "base/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/fileapi/external_mount_points.h"
+#include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_task_runners.h"
+#include "webkit/fileapi/file_system_url.h"
+#include "webkit/fileapi/isolated_context.h"
+#include "webkit/fileapi/mock_file_system_options.h"
 
 namespace drive {
 namespace util {
@@ -76,6 +84,73 @@ TEST(DriveFileSystemUtilTest, ExtractDrivePath) {
                 "/special/drive/subdir/foo.txt")));
 }
 
+TEST(DriveFileSystemUtilTest, ExtractDrivePathFromFileSystemUrl) {
+  // Set up file system context for testing.
+  base::ScopedTempDir temp_dir_;
+  ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+
+  MessageLoop message_loop;
+  scoped_refptr<fileapi::ExternalMountPoints> mount_points =
+      fileapi::ExternalMountPoints::CreateRefCounted();
+  scoped_refptr<fileapi::FileSystemContext> context(
+      new fileapi::FileSystemContext(
+          fileapi::FileSystemTaskRunners::CreateMockTaskRunners(),
+          mount_points,
+          NULL,  // special_storage_policy
+          NULL,  // quota_manager_proxy,
+          temp_dir_.path(),  // partition_path
+          fileapi::CreateAllowFileAccessOptions()));
+
+  // Type:"external" + virtual_path:"drive/foo/bar" resolves to "drive/foo/bar".
+  const std::string& drive_mount_name =
+      GetDriveMountPointPath().BaseName().AsUTF8Unsafe();
+  mount_points->RegisterRemoteFileSystem(
+      drive_mount_name,
+      fileapi::kFileSystemTypeDrive,
+      NULL,  // RemoteFileSystemProxyInterface
+      GetDriveMountPointPath());
+  EXPECT_EQ(
+      base::FilePath::FromUTF8Unsafe(drive_mount_name + "/foo/bar"),
+      ExtractDrivePathFromFileSystemUrl(context->CrackURL(GURL(
+          "filesystem:chrome-extension://dummy-id/external/" +
+          drive_mount_name + "/foo/bar"))));
+
+  // Virtual mount name should not affect the extracted path.
+  mount_points->RevokeFileSystem(drive_mount_name);
+  mount_points->RegisterRemoteFileSystem(
+      "drive2",
+      fileapi::kFileSystemTypeDrive,
+      NULL,  // RemoteFileSystemProxyInterface
+      GetDriveMountPointPath());
+  EXPECT_EQ(
+      base::FilePath::FromUTF8Unsafe(drive_mount_name + "/foo/bar"),
+      ExtractDrivePathFromFileSystemUrl(context->CrackURL(GURL(
+          "filesystem:chrome-extension://dummy-id/external/drive2/foo/bar"))));
+
+  // Type:"external" + virtual_path:"Downloads/foo" is not a Drive path.
+  mount_points->RegisterFileSystem(
+      "Downloads",
+      fileapi::kFileSystemTypeNativeLocal,
+      temp_dir_.path());
+  EXPECT_EQ(
+      base::FilePath(),
+      ExtractDrivePathFromFileSystemUrl(context->CrackURL(GURL(
+          "filesystem:chrome-extension://dummy-id/external/Downloads/foo"))));
+
+  // Type:"isolated" + virtual_path:"isolated_id/name" mapped on a Drive path.
+  std::string isolated_name;
+  std::string isolated_id =
+      fileapi::IsolatedContext::GetInstance()->RegisterFileSystemForPath(
+          fileapi::kFileSystemTypeNativeForPlatformApp,
+          GetDriveMountPointPath().AppendASCII("bar/buz"),
+          &isolated_name);
+  EXPECT_EQ(
+      base::FilePath::FromUTF8Unsafe(drive_mount_name + "/bar/buz"),
+      ExtractDrivePathFromFileSystemUrl(context->CrackURL(GURL(
+          "filesystem:chrome-extension://dummy-id/isolated/" +
+          isolated_id + "/" + isolated_name))));
+}
+
 TEST(DriveFileSystemUtilTest, EscapeUnescapeCacheFileName) {
   const std::string kUnescapedFileName(
       "tmp:`~!@#$%^&*()-_=+[{|]}\\\\;\',<.>/?");

chrome/browser/chromeos/drive/drive_task_executor.cc

@@ -49,9 +49,10 @@ bool DriveTaskExecutor::ExecuteAndNotify(
   std::vector<base::FilePath> raw_paths;
   for (std::vector<FileSystemURL>::const_iterator url = file_urls.begin();
        url != file_urls.end(); ++url) {
-    if (!url->is_valid() || url->type() != fileapi::kFileSystemTypeDrive)
+    base::FilePath path = util::ExtractDrivePathFromFileSystemUrl(*url);
+    if (path.empty())
       return false;
-    raw_paths.push_back(url->virtual_path());
+    raw_paths.push_back(path);
   }
 
   DriveSystemService* system_service =