Remove granting permissions on behalf of Files app.

Files app is not available for some tests, and granting full permissions was an
overkill, as we grant only permissions to requested resources.

TEST=All current tests pass.
BUG=440674

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

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

11 files changed, 65 insertions, 126 deletions

diff --git a/chrome/browser/chromeos/file_manager/fileapi_util.cc b/chrome/browser/chromeos/file_manager/fileapi_util.cc
index e7a1a4b..17d34b3 100644
--- a/chrome/browser/chromeos/file_manager/fileapi_util.cc
+++ b/chrome/browser/chromeos/file_manager/fileapi_util.cc
@@ -217,30 +217,6 @@ void OnConvertFileDefinitionDone(
   callback.Run(entry_definition_list->at(0));
 }
 
-// Used to implement CheckIfDirectoryExists().
-void CheckIfDirectoryExistsOnIOThread(
-    scoped_refptr<storage::FileSystemContext> file_system_context,
-    const GURL& url,
-    const storage::FileSystemOperationRunner::StatusCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  storage::FileSystemURL file_system_url = file_system_context->CrackURL(url);
-  file_system_context->operation_runner()->DirectoryExists(
-      file_system_url, callback);
-}
-
-// Used by GetMetadataForPath
-void GetMetadataOnIOThread(
-    scoped_refptr<storage::FileSystemContext> file_system_context,
-    const GURL& url,
-    const storage::FileSystemOperationRunner::GetMetadataCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  const storage::FileSystemURL file_system_url =
-      file_system_context->CrackURL(url);
-  file_system_context->operation_runner()->GetMetadata(file_system_url,
-                                                       callback);
-}
-
 // Checks if the |file_path| points non-native location or not.
 bool IsUnderNonNativeLocalPath(const storage::FileSystemContext& context,
                                const base::FilePath& file_path) {
@@ -556,38 +532,42 @@ void ConvertSelectedFileInfoListToFileChooserFileInfoList(
 
 void CheckIfDirectoryExists(
     scoped_refptr<storage::FileSystemContext> file_system_context,
-    const GURL& url,
+    const base::FilePath& directory_path,
     const storage::FileSystemOperationRunner::StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // Check the existence of directory using file system API implementation on
-  // behalf of the file manager app. We need to grant access beforehand.
-  storage::ExternalFileSystemBackend* backend =
+  storage::ExternalFileSystemBackend* const backend =
       file_system_context->external_backend();
   DCHECK(backend);
-  backend->GrantFullAccessToExtension(kFileManagerAppId);
+  const storage::FileSystemURL internal_url =
+      backend->CreateInternalURL(file_system_context.get(), directory_path);
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&CheckIfDirectoryExistsOnIOThread, file_system_context, url,
-                 google_apis::CreateRelayCallback(callback)));
+      base::Bind(base::IgnoreResult(
+                     &storage::FileSystemOperationRunner::DirectoryExists),
+                 file_system_context->operation_runner()->AsWeakPtr(),
+                 internal_url, google_apis::CreateRelayCallback(callback)));
 }
 
 void GetMetadataForPath(
     scoped_refptr<storage::FileSystemContext> file_system_context,
-    const GURL& url,
+    const base::FilePath& entry_path,
     const storage::FileSystemOperationRunner::GetMetadataCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   storage::ExternalFileSystemBackend* const backend =
       file_system_context->external_backend();
   DCHECK(backend);
-  backend->GrantFullAccessToExtension(kFileManagerAppId);
+  const storage::FileSystemURL internal_url =
+      backend->CreateInternalURL(file_system_context.get(), entry_path);
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&GetMetadataOnIOThread, file_system_context, url,
-                 google_apis::CreateRelayCallback(callback)));
+      base::Bind(
+          base::IgnoreResult(&storage::FileSystemOperationRunner::GetMetadata),
+          file_system_context->operation_runner()->AsWeakPtr(), internal_url,
+          google_apis::CreateRelayCallback(callback)));
 }
 
 storage::FileSystemURL CreateIsolatedURLFromVirtualPath(
diff --git a/chrome/browser/chromeos/file_manager/fileapi_util.h b/chrome/browser/chromeos/file_manager/fileapi_util.h
index a6ae85e..d7e7e6e 100644
--- a/chrome/browser/chromeos/file_manager/fileapi_util.h
+++ b/chrome/browser/chromeos/file_manager/fileapi_util.h
@@ -145,16 +145,16 @@ void ConvertSelectedFileInfoListToFileChooserFileInfoList(
     const SelectedFileInfoList& selected_info_list,
     const FileChooserFileInfoListCallback& callback);
 
-// Checks if a directory exists at |url|.
+// Checks if a directory exists at |directory_path| absolute path.
 void CheckIfDirectoryExists(
     scoped_refptr<storage::FileSystemContext> file_system_context,
-    const GURL& url,
+    const base::FilePath& directory_path,
     const storage::FileSystemOperationRunner::StatusCallback& callback);
 
-// Get metadata for object at |url|.
+// Get metadata for an entry at |entry_path| absolute path.
 void GetMetadataForPath(
     scoped_refptr<storage::FileSystemContext> file_system_context,
-    const GURL& url,
+    const base::FilePath& entry_path,
     const storage::FileSystemOperationRunner::GetMetadataCallback& callback);
 
 // Obtains isolated file system URL from |virtual_path| pointing a file in the
diff --git a/chrome/browser/chromeos/file_manager/filesystem_api_util.cc b/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
index 5c6238e..f11a58d 100644
--- a/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
+++ b/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
@@ -197,20 +197,8 @@ void IsNonNativeLocalPathDirectory(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(IsUnderNonNativeLocalPath(profile, path));
 
-  GURL url;
-  if (!util::ConvertAbsoluteFilePathToFileSystemUrl(
-           profile, path, kFileManagerAppId, &url)) {
-    // Posting to the current thread, so that we always call back asynchronously
-    // independent from whether or not the operation succeeds.
-    content::BrowserThread::PostTask(content::BrowserThread::UI,
-                                     FROM_HERE,
-                                     base::Bind(callback, false));
-    return;
-  }
-
   util::CheckIfDirectoryExists(
-      GetFileSystemContextForExtensionId(profile, kFileManagerAppId),
-      url,
+      GetFileSystemContextForExtensionId(profile, kFileManagerAppId), path,
       base::Bind(&BoolCallbackAsFileErrorCallback, callback));
 }
 
@@ -232,20 +220,18 @@ void PrepareNonNativeLocalFileForWritableApp(
     return;
   }
 
-  storage::FileSystemContext* const context =
+  scoped_refptr<storage::FileSystemContext> const file_system_context =
       GetFileSystemContextForExtensionId(profile, kFileManagerAppId);
-  DCHECK(context);
-
-  // Check the existence of a file using file system API implementation on
-  // behalf of the file manager app. We need to grant access beforehand.
-  context->external_backend()->GrantFullAccessToExtension(kFileManagerAppId);
+  DCHECK(file_system_context);
+  storage::ExternalFileSystemBackend* const backend =
+      file_system_context->external_backend();
+  DCHECK(backend);
+  const storage::FileSystemURL internal_url =
+      backend->CreateInternalURL(file_system_context.get(), path);
 
   content::BrowserThread::PostTask(
-      content::BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&PrepareFileOnIOThread,
-                 make_scoped_refptr(context),
-                 context->CrackURL(url),
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&PrepareFileOnIOThread, file_system_context, internal_url,
                  google_apis::CreateRelayCallback(callback)));
 }
 
diff --git a/chrome/browser/chromeos/file_manager/open_util.cc b/chrome/browser/chromeos/file_manager/open_util.cc
index b21834e..9eeb9cc4 100644
--- a/chrome/browser/chromeos/file_manager/open_util.cc
+++ b/chrome/browser/chromeos/file_manager/open_util.cc
@@ -158,6 +158,7 @@ void OpenItem(Profile* profile,
               const platform_util::OpenOperationCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+  // This is unfortunately necessary as file browser handlers operate on URLs.
   GURL url;
   if (!ConvertAbsoluteFilePathToFileSystemUrl(profile, file_path,
                                               kFileManagerAppId, &url)) {
@@ -166,7 +167,7 @@ void OpenItem(Profile* profile,
   }
 
   GetMetadataForPath(
-      GetFileSystemContextForExtensionId(profile, kFileManagerAppId), url,
+      GetFileSystemContextForExtensionId(profile, kFileManagerAppId), file_path,
       base::Bind(&OpenItemWithMetadata, profile, file_path, url, expected_type,
                  callback));
 }
diff --git a/chrome/browser/chromeos/fileapi/file_access_permissions.cc b/chrome/browser/chromeos/fileapi/file_access_permissions.cc
index bfa1c53..fee041a 100644
--- a/chrome/browser/chromeos/fileapi/file_access_permissions.cc
+++ b/chrome/browser/chromeos/fileapi/file_access_permissions.cc
@@ -8,23 +8,10 @@
 
 namespace chromeos {
 
-namespace {
-
-// Empty path is prefix of any other paths, hence it represents full permission.
-base::FilePath FullPermission() { return base::FilePath(); }
-
-}  // namespace
-
 FileAccessPermissions::FileAccessPermissions() {}
 
 FileAccessPermissions::~FileAccessPermissions() {}
 
-void FileAccessPermissions::GrantFullAccessPermission(
-    const std::string& extension_id) {
-  base::AutoLock locker(lock_);
-  path_map_[extension_id].insert(FullPermission());
-}
-
 void FileAccessPermissions::GrantAccessPermission(
     const std::string& extension_id, const base::FilePath& path) {
   DCHECK(!path.empty());
@@ -40,9 +27,6 @@ bool FileAccessPermissions::HasAccessPermission(
     return false;
   const PathSet& path_set = path_map_iter->second;
 
-  if (path_set.find(FullPermission()) != path_set.end())
-    return true;
-
   // Check this file and walk up its directory tree to find if this extension
   // has access to it.
   base::FilePath current_path = path.StripTrailingSeparators();
diff --git a/chrome/browser/chromeos/fileapi/file_access_permissions.h b/chrome/browser/chromeos/fileapi/file_access_permissions.h
index 3377e66..ed2eaae 100644
--- a/chrome/browser/chromeos/fileapi/file_access_permissions.h
+++ b/chrome/browser/chromeos/fileapi/file_access_permissions.h
@@ -22,8 +22,6 @@ class FileAccessPermissions {
   FileAccessPermissions();
   virtual ~FileAccessPermissions();
 
-  // Grants |extension_id| full access to all paths.
-  void GrantFullAccessPermission(const std::string& extension_id);
   // Grants |extension_id| access to |path|.
   void GrantAccessPermission(const std::string& extension_id,
                              const base::FilePath& path);
diff --git a/chrome/browser/chromeos/fileapi/file_access_permissions_unittest.cc b/chrome/browser/chromeos/fileapi/file_access_permissions_unittest.cc
index 400ad8e..f0f466c 100644
--- a/chrome/browser/chromeos/fileapi/file_access_permissions_unittest.cc
+++ b/chrome/browser/chromeos/fileapi/file_access_permissions_unittest.cc
@@ -47,12 +47,6 @@ TEST(FileAccessPermissionsTest, FileAccessChecks) {
   EXPECT_TRUE(permissions.HasAccessPermission(extension2, good_file));
   EXPECT_TRUE(permissions.HasAccessPermission(extension2, bad_file));
 
-  // After granting full access, it can access all directories and files.
-  permissions.GrantFullAccessPermission(extension1);
-  EXPECT_TRUE(permissions.HasAccessPermission(extension1, good_dir));
-  EXPECT_TRUE(permissions.HasAccessPermission(extension1, good_file));
-  EXPECT_TRUE(permissions.HasAccessPermission(extension1, bad_file));
-
   // After revoking rights for extensions, they should not be able to access
   // any file system element anymore.
   permissions.RevokePermissions(extension1);
diff --git a/chrome/browser/chromeos/fileapi/file_system_backend.cc b/chrome/browser/chromeos/fileapi/file_system_backend.cc
index 28c1793..a0a7580 100644
--- a/chrome/browser/chromeos/fileapi/file_system_backend.cc
+++ b/chrome/browser/chromeos/fileapi/file_system_backend.cc
@@ -173,6 +173,10 @@ bool FileSystemBackend::IsAccessAllowed(
   if (!CanHandleURL(url))
     return false;
 
+  // If there is no origin set, then it's an internal access.
+  if (url.origin().is_empty())
+    return true;
+
   std::string extension_id = url.origin().host();
   // TODO(mtomasz): Temporarily whitelist TimeScapes. Remove this in M-31.
   // See: crbug.com/271946
@@ -190,17 +194,6 @@ bool FileSystemBackend::IsAccessAllowed(
                                                        url.virtual_path());
 }
 
-void FileSystemBackend::GrantFullAccessToExtension(
-    const std::string& extension_id) {
-  if (!special_storage_policy_.get())
-    return;
-  if (!special_storage_policy_->IsFileHandler(extension_id)) {
-    NOTREACHED();
-    return;
-  }
-  file_access_permissions_->GrantFullAccessPermission(extension_id);
-}
-
 void FileSystemBackend::GrantFileAccessToExtension(
     const std::string& extension_id, const base::FilePath& virtual_path) {
   if (!special_storage_policy_.get())
@@ -396,16 +389,15 @@ scoped_ptr<storage::FileStreamWriter> FileSystemBackend::CreateFileStreamWriter(
   return scoped_ptr<storage::FileStreamWriter>();
 }
 
-bool FileSystemBackend::GetVirtualPath(
-    const base::FilePath& filesystem_path,
-    base::FilePath* virtual_path) {
+bool FileSystemBackend::GetVirtualPath(const base::FilePath& filesystem_path,
+                                       base::FilePath* virtual_path) const {
   return mount_points_->GetVirtualPath(filesystem_path, virtual_path) ||
          system_mount_points_->GetVirtualPath(filesystem_path, virtual_path);
 }
 
 void FileSystemBackend::GetRedirectURLForContents(
     const storage::FileSystemURL& url,
-    const storage::URLCallback& callback) {
+    const storage::URLCallback& callback) const {
   DCHECK(url.is_valid());
 
   if (!IsAccessAllowed(url))
@@ -432,4 +424,15 @@ void FileSystemBackend::GetRedirectURLForContents(
   callback.Run(GURL());
 }
 
+storage::FileSystemURL FileSystemBackend::CreateInternalURL(
+    storage::FileSystemContext* context,
+    const base::FilePath& entry_path) const {
+  base::FilePath virtual_path;
+  if (!GetVirtualPath(entry_path, &virtual_path))
+    return storage::FileSystemURL();
+
+  return context->CreateCrackedFileSystemURL(
+      GURL() /* origin */, storage::kFileSystemTypeExternal, virtual_path);
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/fileapi/file_system_backend.h b/chrome/browser/chromeos/fileapi/file_system_backend.h
index 8bc61c8..529a1dc 100644
--- a/chrome/browser/chromeos/fileapi/file_system_backend.h
+++ b/chrome/browser/chromeos/fileapi/file_system_backend.h
@@ -10,6 +10,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "storage/browser/fileapi/file_system_backend.h"
 #include "storage/browser/fileapi/task_runner_bound_observer_list.h"
@@ -128,14 +129,17 @@ class FileSystemBackend : public storage::ExternalFileSystemBackend {
   // storage::ExternalFileSystemBackend overrides.
   bool IsAccessAllowed(const storage::FileSystemURL& url) const override;
   std::vector<base::FilePath> GetRootDirectories() const override;
-  void GrantFullAccessToExtension(const std::string& extension_id) override;
   void GrantFileAccessToExtension(const std::string& extension_id,
                                   const base::FilePath& virtual_path) override;
   void RevokeAccessForExtension(const std::string& extension_id) override;
   bool GetVirtualPath(const base::FilePath& filesystem_path,
-                      base::FilePath* virtual_path) override;
-  void GetRedirectURLForContents(const storage::FileSystemURL& url,
-                                 const storage::URLCallback& callback) override;
+                      base::FilePath* virtual_path) const override;
+  void GetRedirectURLForContents(
+      const storage::FileSystemURL& url,
+      const storage::URLCallback& callback) const override;
+  storage::FileSystemURL CreateInternalURL(
+      storage::FileSystemContext* context,
+      const base::FilePath& entry_path) const override;
 
  private:
   scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy_;
diff --git a/chrome/browser/chromeos/fileapi/file_system_backend_unittest.cc b/chrome/browser/chromeos/fileapi/file_system_backend_unittest.cc
index 3dc7d3c..cf304963 100644
--- a/chrome/browser/chromeos/fileapi/file_system_backend_unittest.cc
+++ b/chrome/browser/chromeos/fileapi/file_system_backend_unittest.cc
@@ -167,19 +167,6 @@ TEST(ChromeOSFileSystemBackendTest, AccessPermissions) {
       CreateFileSystemURL(extension, "system/foo1",
                           system_mount_points.get())));
 
-  backend.GrantFullAccessToExtension(extension);
-  // The extension should be able to access restricted file system after it was
-  // granted full access.
-  EXPECT_TRUE(backend.IsAccessAllowed(
-      CreateFileSystemURL(extension, "oem/foo", mount_points.get())));
-  // The extension which was granted full access should be able to access any
-  // path on current file systems.
-  EXPECT_TRUE(backend.IsAccessAllowed(
-      CreateFileSystemURL(extension, "removable/foo1", mount_points.get())));
-  EXPECT_TRUE(backend.IsAccessAllowed(
-      CreateFileSystemURL(extension, "system/foo1",
-                          system_mount_points.get())));
-
   // The extension cannot access new mount points.
   // TODO(tbarzic): This should probably be changed.
   ASSERT_TRUE(
diff --git a/storage/browser/fileapi/file_system_backend.h b/storage/browser/fileapi/file_system_backend.h
index 1f626d3..58333b3 100644
--- a/storage/browser/fileapi/file_system_backend.h
+++ b/storage/browser/fileapi/file_system_backend.h
@@ -165,10 +165,6 @@ class ExternalFileSystemBackend : public FileSystemBackend {
   // provider. This list is used to set appropriate child process file access
   // permissions.
   virtual std::vector<base::FilePath> GetRootDirectories() const = 0;
-  // Grants access to all external file system from extension identified with
-  // |extension_id|. TODO(mtomasz): Remove, as full access should never be
-  // necessary. Use GrantFileAccessToExtension() instead.
-  virtual void GrantFullAccessToExtension(const std::string& extension_id) = 0;
   // Grants access to |virtual_path| from |origin_url|.
   virtual void GrantFileAccessToExtension(
       const std::string& extension_id,
@@ -179,12 +175,18 @@ class ExternalFileSystemBackend : public FileSystemBackend {
   // Gets virtual path by known filesystem path. Returns false when filesystem
   // path is not exposed by this provider.
   virtual bool GetVirtualPath(const base::FilePath& file_system_path,
-                              base::FilePath* virtual_path) = 0;
+                              base::FilePath* virtual_path) const = 0;
   // Gets a redirect URL for contents. e.g. Google Drive URL for hosted
   // documents. Returns empty URL if the entry does not have the redirect URL.
   virtual void GetRedirectURLForContents(
       const storage::FileSystemURL& url,
-      const storage::URLCallback& callback) = 0;
+      const storage::URLCallback& callback) const = 0;
+  // Creates an internal File System URL for performing internal operations such
+  // as confirming if a file or a directory exist before granting the final
+  // permission to the entry. The path must be an absolute path.
+  virtual storage::FileSystemURL CreateInternalURL(
+      storage::FileSystemContext* context,
+      const base::FilePath& entry_path) const = 0;
 };
 
 }  // namespace storage