Files.app: Stop to use file system URLs having externalfile:// scheme origin internally.

Priviously in the external file protocol handler, for accessing files via file
system API, we uses file system URLs having externalfile:// scheme origin
internally.  And we added special check to pass the origin in IsAccessAllowed
check.

This CL removes the special check. Instead the CL generated isolated file system
URL to access files.

BUG=367027
TEST=unit_tests --gtest_filter=*ExternalFileURL*

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

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

chrome/browser/chromeos/fileapi/external_file_url_request_job.cc

@@ -25,9 +25,11 @@
 #include "net/http/http_util.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_status.h"
+#include "storage/browser/fileapi/external_mount_points.h"
 #include "storage/browser/fileapi/file_system_backend.h"
 #include "storage/browser/fileapi/file_system_context.h"
 #include "storage/browser/fileapi/file_system_operation_runner.h"
+#include "storage/browser/fileapi/isolated_context.h"
 
 using content::BrowserThread;
 
@@ -37,6 +39,37 @@ namespace {
 const char kMimeTypeForRFC822[] = "message/rfc822";
 const char kMimeTypeForMHTML[] = "multipart/related";
 
+storage::FileSystemURL CreateIsolatedURLFromVirtualPath(
+    const storage::FileSystemContext& context,
+    const base::FilePath& virtual_path) {
+  std::string file_system_id;
+  storage::FileSystemType file_system_type;
+  base::FilePath path;
+  {
+    std::string cracked_id;
+    storage::FileSystemMountOption option;
+    storage::ExternalMountPoints::GetSystemInstance()->CrackVirtualPath(
+        virtual_path,
+        &file_system_id,
+        &file_system_type,
+        &cracked_id,
+        &path,
+        &option);
+  }
+  if (!IsExternalFileURLType(file_system_type))
+    return storage::FileSystemURL();
+  std::string register_name;
+  const std::string isolated_file_system_id =
+      storage::IsolatedContext::GetInstance()->RegisterFileSystemForPath(
+          file_system_type, file_system_id, path, &register_name);
+  storage::FileSystemURL file_system_url = context.CreateCrackedFileSystemURL(
+      GURL(),
+      storage::kFileSystemTypeIsolated,
+      base::FilePath(isolated_file_system_id).Append(register_name));
+  DCHECK(file_system_url.is_valid());
+  return file_system_url;
+}
+
 // Helper for obtaining FileSystemContext, FileSystemURL, and mime type on the
 // UI thread.
 class URLHelper {
@@ -79,19 +112,17 @@ class URLHelper {
     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);
+    file_system_url_ = CreateIsolatedURLFromVirtualPath(*context, virtual_path);
 
     // Check if the obtained path providing external file URL or not.
-    if (FileSystemURLToExternalFileURL(file_system_url_).is_empty()) {
+    if (!file_system_url_.is_valid()) {
       ReplyResult(net::ERR_INVALID_URL);
       return;
     }
 
+    isolated_file_system_scope_.reset(
+        new ExternalFileURLRequestJob::IsolatedFileSystemScope(
+            file_system_url_.filesystem_id()));
     file_system_context_ = context;
 
     extensions::app_file_handler_util::GetMimeTypeForLocalPath(
@@ -116,19 +147,23 @@ class URLHelper {
   void ReplyResult(net::Error error) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-    BrowserThread::PostTask(BrowserThread::IO,
-                            FROM_HERE,
-                            base::Bind(callback_,
-                                       error,
-                                       file_system_context_,
-                                       file_system_url_,
-                                       mime_type_));
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(callback_,
+                   error,
+                   file_system_context_,
+                   base::Passed(&isolated_file_system_scope_),
+                   file_system_url_,
+                   mime_type_));
   }
 
   void* const profile_id_;
   const GURL url_;
   const ExternalFileURLRequestJob::HelperCallback callback_;
   scoped_refptr<storage::FileSystemContext> file_system_context_;
+  scoped_ptr<ExternalFileURLRequestJob::IsolatedFileSystemScope>
+      isolated_file_system_scope_;
   storage::FileSystemURL file_system_url_;
   std::string mime_type_;
 
@@ -137,6 +172,15 @@ class URLHelper {
 
 }  // namespace
 
+ExternalFileURLRequestJob::IsolatedFileSystemScope::IsolatedFileSystemScope(
+    const std::string& file_system_id)
+    : file_system_id_(file_system_id) {
+}
+
+ExternalFileURLRequestJob::IsolatedFileSystemScope::~IsolatedFileSystemScope() {
+  storage::IsolatedContext::GetInstance()->RevokeFileSystem(file_system_id_);
+}
+
 ExternalFileURLRequestJob::ExternalFileURLRequestJob(
     void* profile_id,
     net::URLRequest* request,
@@ -195,6 +239,7 @@ void ExternalFileURLRequestJob::Start() {
 void ExternalFileURLRequestJob::OnHelperResultObtained(
     net::Error error,
     const scoped_refptr<storage::FileSystemContext>& file_system_context,
+    scoped_ptr<IsolatedFileSystemScope> isolated_file_system_scope,
     const storage::FileSystemURL& file_system_url,
     const std::string& mime_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -207,6 +252,7 @@ void ExternalFileURLRequestJob::OnHelperResultObtained(
 
   DCHECK(file_system_context.get());
   file_system_context_ = file_system_context;
+  isolated_file_system_scope_ = isolated_file_system_scope.Pass();
   file_system_url_ = file_system_url;
   mime_type_ = mime_type;
 
@@ -279,6 +325,7 @@ void ExternalFileURLRequestJob::Kill() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   stream_reader_.reset();
+  isolated_file_system_scope_.reset();
   file_system_context_ = NULL;
   net::URLRequestJob::Kill();
   weak_ptr_factory_.InvalidateWeakPtrs();

chrome/browser/chromeos/fileapi/external_file_url_request_job.h

@@ -29,15 +29,29 @@ namespace chromeos {
 // requests for drive resources and FileSystem.  It exposes content URLs
 // formatted as drive:<drive-file-path>.
 // The methods should be run on IO thread.
+// TODO(hirono): After removing MHTML support, stop to use the special
+// externalfile: scheme and use filesystem: URL directly.  crbug.com/415455
 class ExternalFileURLRequestJob : public net::URLRequestJob {
  public:
+  // Scope of isolated file system.
+  class IsolatedFileSystemScope {
+   public:
+    explicit IsolatedFileSystemScope(const std::string& file_system_id);
+    ~IsolatedFileSystemScope();
+
+   private:
+    std::string file_system_id_;
+    DISALLOW_COPY_AND_ASSIGN(IsolatedFileSystemScope);
+  };
+
   // Callback to take results from an internal helper defined in
   // drive_url_request_job.cc.
-  typedef base::Callback<
-      void(net::Error,
-           const scoped_refptr<storage::FileSystemContext>& file_system_context,
-           const storage::FileSystemURL& file_system_url,
-           const std::string& mime_type)> HelperCallback;
+  typedef base::Callback<void(
+      net::Error,
+      const scoped_refptr<storage::FileSystemContext>& file_system_context,
+      scoped_ptr<IsolatedFileSystemScope> isolated_file_system_scope,
+      const storage::FileSystemURL& file_system_url,
+      const std::string& mime_type)> HelperCallback;
 
   ExternalFileURLRequestJob(void* profile_id,
                             net::URLRequest* request,
@@ -64,6 +78,7 @@ class ExternalFileURLRequestJob : public net::URLRequestJob {
   void OnHelperResultObtained(
       net::Error error,
       const scoped_refptr<storage::FileSystemContext>& file_system_context,
+      scoped_ptr<IsolatedFileSystemScope> isolated_file_system_scope,
       const storage::FileSystemURL& file_system_url,
       const std::string& mime_type);
 
@@ -84,6 +99,7 @@ class ExternalFileURLRequestJob : public net::URLRequestJob {
   int64 remaining_bytes_;
 
   scoped_refptr<storage::FileSystemContext> file_system_context_;
+  scoped_ptr<IsolatedFileSystemScope> isolated_file_system_scope_;
   storage::FileSystemURL file_system_url_;
   std::string mime_type_;
   scoped_ptr<storage::FileStreamReader> stream_reader_;

chrome/browser/chromeos/fileapi/external_file_url_util.cc

@@ -24,23 +24,23 @@ using content::BrowserThread;
 
 namespace chromeos {
 
+bool IsExternalFileURLType(storage::FileSystemType type) {
+  return type == storage::kFileSystemTypeDrive ||
+         type == storage::kFileSystemTypeDeviceMediaAsFileStorage ||
+         type == storage::kFileSystemTypeProvided;
+}
+
 GURL FileSystemURLToExternalFileURL(
     const storage::FileSystemURL& file_system_url) {
-  if (file_system_url.mount_type() != storage::kFileSystemTypeExternal)
+  if (file_system_url.mount_type() != storage::kFileSystemTypeExternal ||
+      !IsExternalFileURLType(file_system_url.type())) {
     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();
   }
+
+  return GURL(base::StringPrintf(
+      "%s:%s",
+      chrome::kExternalFileScheme,
+      file_system_url.virtual_path().AsUTF8Unsafe().c_str()));
 }
 
 base::FilePath ExternalFileURLToVirtualPath(const GURL& url) {

chrome/browser/chromeos/fileapi/external_file_url_util.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_CHROMEOS_FILEAPI_EXTERNAL_FILE_URL_UTIL_H_
 #define CHROME_BROWSER_CHROMEOS_FILEAPI_EXTERNAL_FILE_URL_UTIL_H_
 
+#include "storage/common/fileapi/file_system_types.h"
+
 class GURL;
 class Profile;
 
@@ -18,6 +20,9 @@ class FileSystemURL;
 
 namespace chromeos {
 
+// Returns whether the external file URL is provided for the |type| or not.
+bool IsExternalFileURLType(storage::FileSystemType type);
+
 // 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.

chrome/browser/chromeos/fileapi/external_file_url_util_unittest.cc

@@ -80,15 +80,4 @@ TEST_F(ExternalFileURLUtilTest, FilePathToExternalFileURL) {
                 .AsUTF8Unsafe());
 }
 
-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-test-user-hash/"
-                       "file.txt")).is_valid());
-}
-
 }  // namespace chromeos

chrome/browser/chromeos/fileapi/file_system_backend.cc

@@ -181,12 +181,6 @@ bool FileSystemBackend::IsAccessAllowed(
     return true;
   }
 
-  // Grant access for URL having "externalfile:" scheme. The URL
-  // filesystem:externalfile:/xxx cannot be parsed directly. The URL is created
-  // only by DriveURLRequestJob.
-  if (url.origin().scheme() == chrome::kExternalFileScheme)
-    return true;
-
   // Check first to make sure this extension has fileBrowserHander permissions.
   if (!special_storage_policy_.get() ||
       !special_storage_policy_->IsFileHandler(extension_id))