Files.app: Enable externalfile: protocol for MTP volumes.

Previously drive paths are used to obtain 'externalfile:' URL and the URL is
converted back to the drive path.

The CL uses FileSystemURL instead of drive paths so that the app can generate
external file URLs for other than drive files, and enables external file URLs
for MTP and FSP volumes.

BUG=367027
TEST=manually

Review URL: https://codereview.chromium.org/589473002

Cr-Commit-Position: refs/heads/master@{#296901}

chrome/browser/chromeos/file_manager/file_browser_handlers.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/chromeos/file_manager/file_browser_handlers.h"
 
+#include <algorithm>
+#include <set>
+
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/i18n/case_conversion.h"
@@ -430,7 +433,6 @@ void FileBrowserHandlerExecutor::SetupHandlerHostFileAccessPermissions(
 // is used to handle certain action IDs of the file manager.
 bool ShouldBeOpenedWithBrowser(const std::string& extension_id,
                                const std::string& action_id) {
-
   return (extension_id == kFileManagerAppId &&
           (action_id == "view-pdf" ||
            action_id == "view-swf" ||
@@ -449,8 +451,7 @@ bool OpenFilesWithBrowser(Profile* profile,
   for (size_t i = 0; i < file_urls.size(); ++i) {
     const FileSystemURL& file_url = file_urls[i];
     if (chromeos::FileSystemBackend::CanHandleURL(file_url)) {
-      const base::FilePath& file_path = file_url.path();
-      num_opened += util::OpenFileWithBrowser(profile, file_path);
+      num_opened += util::OpenFileWithBrowser(profile, file_url) ? 1 : 0;
     }
   }
   return num_opened > 0;

chrome/browser/chromeos/file_manager/open_with_browser.cc

@@ -11,6 +11,7 @@
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
+#include "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
 #include "chrome/browser/chromeos/fileapi/external_file_url_util.h"
 #include "chrome/browser/drive/drive_api_util.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
@@ -129,29 +130,35 @@ GURL ReadUrlFromGDocOnBlockingPool(const base::FilePath& file_path) {
 
 }  // namespace
 
-bool OpenFileWithBrowser(Profile* profile, const base::FilePath& file_path) {
+bool OpenFileWithBrowser(Profile* profile,
+                         const storage::FileSystemURL& file_system_url) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(profile);
 
+  const base::FilePath file_path = file_system_url.path();
+
   // For things supported natively by the browser, we should open it
   // in a tab.
   if (IsViewableInBrowser(file_path) ||
       ShouldBeOpenedWithPlugin(profile, file_path.Extension())) {
-    GURL page_url = net::FilePathToFileURL(file_path);
-    // Override drive resource to point to internal handler instead of file URL.
-    if (drive::util::IsUnderDriveMountPoint(file_path)) {
-      page_url = chromeos::FilePathToExternalFileURL(
-          drive::util::ExtractDrivePath(file_path));
-    }
+    // Use external file URL if it is provided for the file system.
+    GURL page_url = chromeos::FileSystemURLToExternalFileURL(file_system_url);
+    if (page_url.is_empty())
+      page_url = net::FilePathToFileURL(file_path);
+
     OpenNewTab(profile, page_url);
     return true;
   }
 
   if (drive::util::HasHostedDocumentExtension(file_path)) {
-    if (drive::util::IsUnderDriveMountPoint(file_path)) {
-      // The file is on Google Docs. Open with drive URL.
-      GURL url = chromeos::FilePathToExternalFileURL(
-          drive::util::ExtractDrivePath(file_path));
+    if (file_manager::util::IsUnderNonNativeLocalPath(profile, file_path)) {
+      // The file is on a non-native volume. Use external file URL. If the file
+      // is on the drive volume, ExternalFileURLRequestJob redirects the URL to
+      // drive's web interface. Otherwise (e.g. MTP, FSP), the file is just
+      // downloaded in a browser tab.
+      const GURL url =
+          chromeos::FileSystemURLToExternalFileURL(file_system_url);
+      DCHECK(!url.is_empty());
       OpenNewTab(profile, url);
     } else {
       // The file is local (downloaded from an attachment or otherwise copied).

chrome/browser/chromeos/file_manager/open_with_browser.h

@@ -11,6 +11,10 @@
 
 class Profile;
 
+namespace storage {
+class FileSystemURL;
+}
+
 namespace file_manager {
 namespace util {
 
@@ -24,7 +28,8 @@ namespace util {
 //   needed.
 //
 // Returns false if failed to open. This happens if the file type is unknown.
-bool OpenFileWithBrowser(Profile* profile, const base::FilePath& file_path);
+bool OpenFileWithBrowser(Profile* profile,
+                         const storage::FileSystemURL& file_system_url);
 
 // Checks whether a pepper plugin for |file_extension| is enabled.
 bool ShouldBeOpenedWithPlugin(

chrome/browser/chromeos/fileapi/external_file_url_request_job.cc

@@ -37,26 +37,6 @@ namespace {
 const char kMimeTypeForRFC822[] = "message/rfc822";
 const char kMimeTypeForMHTML[] = "multipart/related";
 
-// Check if the |url| points a valid location or not.
-bool IsValidURL(const storage::FileSystemURL& url) {
-  switch (url.type()) {
-    case storage::kFileSystemTypeDrive: {
-      const base::FilePath my_drive_path =
-          drive::util::GetDriveMyDriveRootPath();
-      const base::FilePath drive_other_path =
-          drive::util::GetDriveGrandRootPath().Append(
-              drive::util::kDriveOtherDirName);
-      const base::FilePath url_drive_path =
-          drive::util::ExtractDrivePathFromFileSystemUrl(url);
-      return my_drive_path == url_drive_path ||
-             my_drive_path.IsParent(url_drive_path) ||
-             drive_other_path.IsParent(url_drive_path);
-    }
-    default:
-      return false;
-  }
-}
-
 // Helper for obtaining FileSystemContext, FileSystemURL, and mime type on the
 // UI thread.
 class URLHelper {
@@ -96,30 +76,18 @@ class URLHelper {
     DCHECK(context.get());
 
     // Obtain the absolute path in the file system.
-    base::FilePath path = drive::util::GetDriveMountPointPath(profile);
-    drive::util::GetDriveGrandRootPath().AppendRelativePath(
-        ExternalFileURLToFilePath(url_), &path);
-
-    storage::ExternalFileSystemBackend* const backend =
-        context->external_backend();
-    DCHECK(backend);
-
-    // Obtain the virtual path.
-    base::FilePath virtual_path;
-    if (!backend->GetVirtualPath(path, &virtual_path)) {
-      ReplyResult(net::ERR_FILE_NOT_FOUND);
-      return;
-    }
+    const base::FilePath virtual_path = ExternalFileURLToVirtualPath(url_);
 
     // Obtain the file system URL.
     // TODO(hirono): After removing MHTML support, stop to use the special
-
     // drive: scheme and use filesystem: URL directly.  crbug.com/415455
     file_system_url_ = context->CreateCrackedFileSystemURL(
         GURL(std::string(chrome::kExternalFileScheme) + ":"),
         storage::kFileSystemTypeExternal,
         virtual_path);
-    if (!IsValidURL(file_system_url_)) {
+
+    // Check if the obtained path providing external file URL or not.
+    if (FileSystemURLToExternalFileURL(file_system_url_).is_empty()) {
       ReplyResult(net::ERR_INVALID_URL);
       return;
     }

chrome/browser/chromeos/fileapi/external_file_url_request_job_unittest.cc

@@ -228,7 +228,7 @@ class ExternalFileURLRequestJobTest : public testing::Test {
 
 TEST_F(ExternalFileURLRequestJobTest, NonGetMethod) {
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
-      GURL("externalfile:drive/root/File 1.txt"),
+      GURL("externalfile:drive-test-user-hash/root/File 1.txt"),
       net::DEFAULT_PRIORITY,
       test_delegate_.get(),
       NULL));
@@ -242,7 +242,7 @@ TEST_F(ExternalFileURLRequestJobTest, NonGetMethod) {
 }
 
 TEST_F(ExternalFileURLRequestJobTest, RegularFile) {
-  const GURL kTestUrl("externalfile:drive/root/File 1.txt");
+  const GURL kTestUrl("externalfile:drive-test-user-hash/root/File 1.txt");
   const base::FilePath kTestFilePath("drive/root/File 1.txt");
 
   // For the first time, the file should be fetched from the server.
@@ -272,7 +272,7 @@ TEST_F(ExternalFileURLRequestJobTest, RegularFile) {
   {
     test_delegate_.reset(new TestDelegate);
     scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
-        GURL("externalfile:drive/root/File 1.txt"),
+        GURL("externalfile:drive-test-user-hash/root/File 1.txt"),
         net::DEFAULT_PRIORITY,
         test_delegate_.get(),
         NULL));
@@ -295,7 +295,9 @@ TEST_F(ExternalFileURLRequestJobTest, HostedDocument) {
   // Open a gdoc file.
   test_delegate_->set_quit_on_redirect(true);
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
-      GURL("externalfile:drive/root/Document 1 excludeDir-test.gdoc"),
+      GURL(
+          "externalfile:drive-test-user-hash/root/Document 1 "
+          "excludeDir-test.gdoc"),
       net::DEFAULT_PRIORITY,
       test_delegate_.get(),
       NULL));
@@ -310,11 +312,11 @@ TEST_F(ExternalFileURLRequestJobTest, HostedDocument) {
 }
 
 TEST_F(ExternalFileURLRequestJobTest, RootDirectory) {
-  scoped_ptr<net::URLRequest> request(
-      url_request_context_->CreateRequest(GURL("externalfile:drive/root"),
-                                          net::DEFAULT_PRIORITY,
-                                          test_delegate_.get(),
-                                          NULL));
+  scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
+      GURL("externalfile:drive-test-user-hash/root"),
+      net::DEFAULT_PRIORITY,
+      test_delegate_.get(),
+      NULL));
   request->Start();
 
   base::RunLoop().Run();
@@ -325,7 +327,7 @@ TEST_F(ExternalFileURLRequestJobTest, RootDirectory) {
 
 TEST_F(ExternalFileURLRequestJobTest, Directory) {
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
-      GURL("externalfile:drive/root/Directory 1"),
+      GURL("externalfile:drive-test-user-hash/root/Directory 1"),
       net::DEFAULT_PRIORITY,
       test_delegate_.get(),
       NULL));
@@ -339,7 +341,7 @@ TEST_F(ExternalFileURLRequestJobTest, Directory) {
 
 TEST_F(ExternalFileURLRequestJobTest, NonExistingFile) {
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
-      GURL("externalfile:drive/root/non-existing-file.txt"),
+      GURL("externalfile:drive-test-user-hash/root/non-existing-file.txt"),
       net::DEFAULT_PRIORITY,
       test_delegate_.get(),
       NULL));
@@ -367,7 +369,7 @@ TEST_F(ExternalFileURLRequestJobTest, WrongFormat) {
 
 TEST_F(ExternalFileURLRequestJobTest, Cancel) {
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
-      GURL("externalfile:drive/root/File 1.txt"),
+      GURL("externalfile:drive-test-user-hash/root/File 1.txt"),
       net::DEFAULT_PRIORITY,
       test_delegate_.get(),
       NULL));
@@ -382,7 +384,7 @@ TEST_F(ExternalFileURLRequestJobTest, Cancel) {
 }
 
 TEST_F(ExternalFileURLRequestJobTest, RangeHeader) {
-  const GURL kTestUrl("externalfile:drive/root/File 1.txt");
+  const GURL kTestUrl("externalfile:drive-test-user-hash/root/File 1.txt");
   const base::FilePath kTestFilePath("drive/root/File 1.txt");
 
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
@@ -405,7 +407,7 @@ TEST_F(ExternalFileURLRequestJobTest, RangeHeader) {
 }
 
 TEST_F(ExternalFileURLRequestJobTest, WrongRangeHeader) {
-  const GURL kTestUrl("externalfile:drive/root/File 1.txt");
+  const GURL kTestUrl("externalfile:drive-test-user-hash/root/File 1.txt");
 
   scoped_ptr<net::URLRequest> request(url_request_context_->CreateRequest(
       kTestUrl, net::DEFAULT_PRIORITY, test_delegate_.get(), NULL));

chrome/browser/chromeos/fileapi/external_file_url_util.cc

@@ -5,44 +5,73 @@
 #include "chrome/browser/chromeos/fileapi/external_file_url_util.h"
 
 #include <string>
+#include <vector>
 
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
-#include "chrome/browser/chromeos/drive/file_system_util.h"
+#include "chrome/browser/chromeos/file_manager/app_id.h"
+#include "chrome/browser/chromeos/file_manager/fileapi_util.h"
+#include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/storage_partition.h"
 #include "net/base/escape.h"
+#include "storage/browser/fileapi/file_system_url.h"
 
 using content::BrowserThread;
 
 namespace chromeos {
 
-GURL FilePathToExternalFileURL(const base::FilePath& path) {
-  std::string url(base::StringPrintf(
-      "%s:%s", chrome::kExternalFileScheme, path.AsUTF8Unsafe().c_str()));
-  return GURL(url);
+GURL FileSystemURLToExternalFileURL(
+    const storage::FileSystemURL& file_system_url) {
+  if (file_system_url.mount_type() != storage::kFileSystemTypeExternal)
+    return GURL();
+
+  switch (file_system_url.type()) {
+    case storage::kFileSystemTypeDrive:
+    case storage::kFileSystemTypeDeviceMediaAsFileStorage:
+    case storage::kFileSystemTypeProvided:
+      return GURL(base::StringPrintf(
+          "%s:%s",
+          chrome::kExternalFileScheme,
+          file_system_url.virtual_path().AsUTF8Unsafe().c_str()));
+
+    default:
+      return GURL();
+  }
 }
 
-base::FilePath ExternalFileURLToFilePath(const GURL& url) {
+base::FilePath ExternalFileURLToVirtualPath(const GURL& url) {
   if (!url.is_valid() || url.scheme() != chrome::kExternalFileScheme)
     return base::FilePath();
-  std::string path_string =
+  const std::string path_string =
       net::UnescapeURLComponent(url.GetContent(), net::UnescapeRule::NORMAL);
   return base::FilePath::FromUTF8Unsafe(path_string);
 }
 
 GURL CreateExternalFileURLFromPath(Profile* profile,
                                    const base::FilePath& path) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  if (!drive::util::IsUnderDriveMountPoint(path))
+  GURL raw_file_system_url;
+  if (!file_manager::util::ConvertAbsoluteFilePathToFileSystemUrl(
+          profile,
+          path,
+          file_manager::kFileManagerAppId,
+          &raw_file_system_url)) {
     return GURL();
+  }
 
-  drive::FileSystemInterface* file_system =
-      drive::util::GetFileSystemByProfile(profile);
-  if (!file_system)
+  const storage::FileSystemURL file_system_url =
+      file_manager::util::GetFileSystemContextForExtensionId(
+          profile, file_manager::kFileManagerAppId)
+          ->CrackURL(raw_file_system_url);
+  if (!file_system_url.is_valid())
     return GURL();
 
-  return FilePathToExternalFileURL(drive::util::ExtractDrivePath(path));
+  return FileSystemURLToExternalFileURL(file_system_url);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/fileapi/external_file_url_util.h

@@ -12,13 +12,20 @@ namespace base {
 class FilePath;
 }
 
+namespace storage {
+class FileSystemURL;
+}
+
 namespace chromeos {
 
-// Returns the external file resource url formatted as "externalfile:<path>"
-GURL FilePathToExternalFileURL(const base::FilePath& path);
+// Obtains the external file url formatted as "externalfile:<path>" from file
+// path. Returns empty URL if the file system does not provide the external file
+// URL.
+GURL FileSystemURLToExternalFileURL(
+    const storage::FileSystemURL& file_system_url);
 
-// Converts a externalfile: URL back to a path that can be passed to FileSystem.
-base::FilePath ExternalFileURLToFilePath(const GURL& url);
+// Converts a externalfile: URL back to a virtual path of FileSystemURL.
+base::FilePath ExternalFileURLToVirtualPath(const GURL& url);
 
 // Obtains external file URL (e.g. external:drive/root/sample.txt) from file
 // path (e.g. /special/drive-xxx/root/sample.txt), if the |path| points an

chrome/browser/chromeos/fileapi/external_file_url_util_unittest.cc

@@ -32,6 +32,18 @@ class ExternalFileURLUtilTest : public testing::Test {
     return testing_profile_manager_;
   }
 
+  storage::FileSystemURL CreateExpectedURL(const base::FilePath& path) {
+    return storage::FileSystemURL::CreateForTest(
+        GURL("chrome-extension://xxx"),
+        storage::kFileSystemTypeExternal,
+        base::FilePath("drive-test-user-hash").Append(path),
+        "",
+        storage::kFileSystemTypeDrive,
+        base::FilePath(),
+        "",
+        storage::FileSystemMountOption());
+  }
+
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfileManager testing_profile_manager_;
@@ -39,39 +51,44 @@ class ExternalFileURLUtilTest : public testing::Test {
 
 }  // namespace
 
-TEST(ExternalFileURLUtilTest, FilePathToExternalFileURL) {
-  base::FilePath path;
+TEST_F(ExternalFileURLUtilTest, FilePathToExternalFileURL) {
+  storage::FileSystemURL url;
 
   // Path with alphabets and numbers.
-  path = drive::util::GetDriveMyDriveRootPath().AppendASCII("foo/bar012.txt");
-  EXPECT_EQ(path, ExternalFileURLToFilePath(FilePathToExternalFileURL(path)));
+  url = CreateExpectedURL(base::FilePath("foo/bar012.txt"));
+  EXPECT_EQ(url.virtual_path(),
+            ExternalFileURLToVirtualPath(FileSystemURLToExternalFileURL(url)));
 
   // Path with symbols.
-  path = drive::util::GetDriveMyDriveRootPath().AppendASCII(
-      " !\"#$%&'()*+,-.:;<=>?@[\\]^_`{|}~");
-  EXPECT_EQ(path, ExternalFileURLToFilePath(FilePathToExternalFileURL(path)));
+  url = CreateExpectedURL(base::FilePath(" !\"#$%&'()*+,-.:;<=>?@[\\]^_`{|}~"));
+  EXPECT_EQ(url.virtual_path(),
+            ExternalFileURLToVirtualPath(FileSystemURLToExternalFileURL(url)));
 
   // Path with '%'.
-  path = drive::util::GetDriveMyDriveRootPath().AppendASCII("%19%20%21.txt");
-  EXPECT_EQ(path, ExternalFileURLToFilePath(FilePathToExternalFileURL(path)));
+  url = CreateExpectedURL(base::FilePath("%19%20%21.txt"));
+  EXPECT_EQ(url.virtual_path(),
+            ExternalFileURLToVirtualPath(FileSystemURLToExternalFileURL(url)));
 
   // Path with multi byte characters.
   base::string16 utf16_string;
   utf16_string.push_back(0x307b);  // HIRAGANA_LETTER_HO
   utf16_string.push_back(0x3052);  // HIRAGANA_LETTER_GE
-  path = drive::util::GetDriveMyDriveRootPath().Append(
+  url = CreateExpectedURL(
       base::FilePath::FromUTF8Unsafe(base::UTF16ToUTF8(utf16_string) + ".txt"));
-  EXPECT_EQ(path, ExternalFileURLToFilePath(FilePathToExternalFileURL(path)));
+  EXPECT_EQ(url.virtual_path().AsUTF8Unsafe(),
+            ExternalFileURLToVirtualPath(FileSystemURLToExternalFileURL(url))
+                .AsUTF8Unsafe());
 }
 
-TEST(ExternalFileURLUtilTest, ParseFileURLWithExternalFileOrigin) {
-  // filesystem:externalfile:/*** is used only internally. It should not parsed
+TEST_F(ExternalFileURLUtilTest, ParseFileURLWithExternalFileOrigin) {
+  // filesystem:externalfile:/xxx is used only internally. It should not parsed
   // directly.
   ASSERT_FALSE(storage::FileSystemURL::CreateForTest(
                    GURL("filesystem:externalfile:/")).is_valid());
-  ASSERT_FALSE(
-      storage::FileSystemURL::CreateForTest(
-          GURL("filesystem:externalfile:/external/drive/file.txt")).is_valid());
+  ASSERT_FALSE(storage::FileSystemURL::CreateForTest(
+                   GURL(
+                       "filesystem:externalfile:/external/drive-test-user-hash/"
+                       "file.txt")).is_valid());
 }
 
 }  // namespace chromeos

storage/browser/fileapi/file_system_url.cc

@@ -36,6 +36,25 @@ FileSystemURL FileSystemURL::CreateForTest(const GURL& origin,
   return FileSystemURL(origin, mount_type, virtual_path);
 }
 
+FileSystemURL FileSystemURL::CreateForTest(
+    const GURL& origin,
+    FileSystemType mount_type,
+    const base::FilePath& virtual_path,
+    const std::string& mount_filesystem_id,
+    FileSystemType cracked_type,
+    const base::FilePath& cracked_path,
+    const std::string& filesystem_id,
+    const FileSystemMountOption& mount_option) {
+  return FileSystemURL(origin,
+                       mount_type,
+                       virtual_path,
+                       mount_filesystem_id,
+                       cracked_type,
+                       cracked_path,
+                       filesystem_id,
+                       mount_option);
+}
+
 FileSystemURL::FileSystemURL(const GURL& url)
     : mount_type_(kFileSystemTypeUnknown),
       type_(kFileSystemTypeUnknown),

storage/browser/fileapi/file_system_url.h

@@ -86,6 +86,14 @@ class STORAGE_EXPORT FileSystemURL {
   static FileSystemURL CreateForTest(const GURL& origin,
                                      FileSystemType mount_type,
                                      const base::FilePath& virtual_path);
+  static FileSystemURL CreateForTest(const GURL& origin,
+                                     FileSystemType mount_type,
+                                     const base::FilePath& virtual_path,
+                                     const std::string& mount_filesystem_id,
+                                     FileSystemType cracked_type,
+                                     const base::FilePath& cracked_path,
+                                     const std::string& filesystem_id,
+                                     const FileSystemMountOption& mount_option);
 
   // Returns true if this instance represents a valid FileSystem URL.
   bool is_valid() const { return is_valid_; }