Prevent "WEB_APP" tasks being offered for non-native files on ChromeOS.

The NativeFileSystem API currently only supports "real" files, backed
by inodes. Attempting to open a non-native file results it it not being
found because there are no attempts to "uncrack" virtual file paths.

This CL causes file_manager::file_tasks::FindWebTasks() to early-exit
if any entries in the file selection are non-native files.

A testing chrome app is included that uses the file system provider
API as a basis for providing files that can be used for end-to-end
testing.

Bug: 1079065
Change-Id: I5ed5649eb85dcfb25cdfd4965770ae39c3c56dad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213652
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772108}

chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc

@@ -17,6 +17,7 @@
 #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/chromeos/file_manager/filesystem_api_util.h"
 #include "chrome/browser/chromeos/fileapi/file_system_backend.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
@@ -92,10 +93,14 @@ void MaybeAdjustTaskForGalleryAppRemoval(
   // is known to register as a handler. Although from a product perspective, the
   // Gallery should be entirely hidden, we still direct RAW images to Gallery
   // until chrome://media-app supports them.
-  if (std::find_if(urls.begin(), urls.end(), [](const auto& url) {
-        return file_manager::file_tasks::IsRawImage(url.path());
-      }) != urls.end()) {
-    return;
+  // Also filter out requests to open files on non-native files since
+  // TASK_TYPE_WEB_APP currently does not support these.
+  // See https://crbug.com/1079065.
+  for (const auto& url : urls) {
+    if (file_manager::file_tasks::IsRawImage(url.path()) ||
+        file_manager::util::IsNonNativeFileSystemType(url.type())) {
+      return;
+    }
   }
 
   auto* provider = web_app::WebAppProvider::Get(profile);

chrome/browser/chromeos/file_manager/file_tasks_browsertest.cc

@@ -4,9 +4,14 @@
 
 #include "base/bind.h"
 #include "base/path_service.h"
+#include "base/test/bind_test_util.h"
 #include "chrome/browser/chromeos/file_manager/app_id.h"
 #include "chrome/browser/chromeos/file_manager/file_manager_test_util.h"
 #include "chrome/browser/chromeos/file_manager/file_tasks.h"
+#include "chrome/browser/chromeos/file_manager/fileapi_util.h"
+#include "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
+#include "chrome/browser/chromeos/file_manager/volume_manager.h"
+#include "chrome/browser/chromeos/file_manager/volume_manager_observer.h"
 #include "chrome/browser/chromeos/web_applications/default_web_app_ids.h"
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
 #include "chrome/browser/profiles/profile.h"
@@ -19,6 +24,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/api/file_handlers/mime_util.h"
 #include "extensions/browser/entry_info.h"
 #include "extensions/browser/notification_types.h"
 #include "net/base/mime_util.h"
@@ -30,6 +36,8 @@ namespace file_manager {
 namespace file_tasks {
 namespace {
 
+constexpr char kImageProviderFilesystemId[] = "test-image-provider-fs";
+
 // A list of file extensions (`/` delimited) representing a selection of files
 // and the app expected to be the default to open these files.
 // A null app_id indicates there is no preferred default.
@@ -84,9 +92,19 @@ void VerifyTasks(int* remaining,
       << expectation.file_extensions;
 }
 
-// Installs a chrome app that handles .tiff.
-scoped_refptr<const extensions::Extension> InstallTiffHandlerChromeApp(
-    Profile* profile) {
+// Helper to quit a run loop after invoking VerifyTasks().
+void VerifyAsyncTask(int* remaining,
+                     Expectation expectation,
+                     const base::Closure& quit_closure,
+                     std::unique_ptr<std::vector<FullTaskDescriptor>> result) {
+  VerifyTasks(remaining, expectation, std::move(result));
+  quit_closure.Run();
+}
+
+// Installs a chrome app used for testing.
+scoped_refptr<const extensions::Extension> InstallTestingChromeApp(
+    Profile* profile,
+    const char* test_path_ascii) {
   base::ScopedAllowBlockingForTesting allow_io;
   content::WindowedNotificationObserver handler_ready(
       extensions::NOTIFICATION_EXTENSION_BACKGROUND_PAGE_READY,
@@ -95,7 +113,7 @@ scoped_refptr<const extensions::Extension> InstallTiffHandlerChromeApp(
 
   base::FilePath path;
   EXPECT_TRUE(base::PathService::Get(chrome::DIR_TEST_DATA, &path));
-  path = path.AppendASCII("extensions/api_test/file_browser/app_file_handler");
+  path = path.AppendASCII(test_path_ascii);
 
   auto extension = loader.LoadExtension(path);
   EXPECT_TRUE(extension);
@@ -103,6 +121,44 @@ scoped_refptr<const extensions::Extension> InstallTiffHandlerChromeApp(
   return extension;
 }
 
+// Installs a chrome app that handles .tiff.
+scoped_refptr<const extensions::Extension> InstallTiffHandlerChromeApp(
+    Profile* profile) {
+  return InstallTestingChromeApp(
+      profile, "extensions/api_test/file_browser/app_file_handler");
+}
+
+// Helper to exit a RunLoop the next time OnVolumeMounted() is invoked.
+class VolumeWaiter : public VolumeManagerObserver {
+ public:
+  VolumeWaiter(Profile* profile, const base::Closure& on_mount)
+      : profile_(profile), on_mount_(on_mount) {
+    VolumeManager::Get(profile_)->AddObserver(this);
+  }
+  ~VolumeWaiter() override {
+    VolumeManager::Get(profile_)->RemoveObserver(this);
+  }
+  void OnVolumeMounted(chromeos::MountError error_code,
+                       const Volume& volume) override {
+    on_mount_.Run();
+  }
+
+ private:
+  Profile* profile_;
+  base::Closure on_mount_;
+};
+
+// Installs a chrome app that provides a file system containing an image file.
+scoped_refptr<const extensions::Extension> InstallFileProviderChromeApp(
+    Profile* profile) {
+  base::RunLoop run_loop;
+  VolumeWaiter waiter(profile, run_loop.QuitClosure());
+  auto extension = InstallTestingChromeApp(
+      profile, "extensions/api_test/file_browser/image_provider");
+  run_loop.Run();
+  return extension;
+}
+
 class FileTasksBrowserTestBase : public InProcessBrowserTest {
  public:
   void SetUpOnMainThread() override {
@@ -372,5 +428,63 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
   TestExpectationsAgainstDefaultTasks({{"tiff", extension->id().c_str()}});
 }
 
+// Test expectations for files coming from provided file systems.
+IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
+                       ProvidedFileSystemFileSource) {
+  // The current test expectation: a GIF file in the provided file system called
+  // "pixel.gif" should open with Gallery.
+  // TODO(crbug/1079065): Open with MediaApp when the NativeFileSystem API has
+  // good support for ChromeOS special filesystems.
+  const char kTestFile[] = "pixel.gif";
+  Expectation test = {"gif", kGalleryAppId};
+  int remaining_expectations = 1;
+
+  Profile* profile = browser()->profile();
+  auto extension = InstallFileProviderChromeApp(profile);
+
+  VolumeManager* volume_manager = VolumeManager::Get(profile);
+  ASSERT_TRUE(volume_manager);
+  base::WeakPtr<Volume> volume;
+  for (auto& v : volume_manager->GetVolumeList()) {
+    if (v->file_system_id() == kImageProviderFilesystemId) {
+      volume = v;
+    }
+  }
+  ASSERT_TRUE(volume);
+
+  GURL url;
+  ASSERT_TRUE(util::ConvertAbsoluteFilePathToFileSystemUrl(
+      profile, volume->mount_path().AppendASCII(kTestFile), kFileManagerAppId,
+      &url));
+
+  // Note |url| differs slightly to the result of ToGURL() below. The colons
+  // either side of `:test-image-provider-fs:` become escaped as `%3A`.
+
+  storage::FileSystemURL filesystem_url =
+      util::GetFileSystemContextForExtensionId(profile, kFileManagerAppId)
+          ->CrackURL(url);
+
+  std::vector<GURL> urls = {filesystem_url.ToGURL()};
+  std::vector<extensions::EntryInfo> entries;
+
+  // We could add the mime type here, but since a "real" file is provided, we
+  // can get additional coverage of the mime determination. For non-native files
+  // this uses metadata only (not sniffing).
+  entries.emplace_back(filesystem_url.path(), "", false);
+
+  base::RunLoop run_loop;
+  auto verifier = base::BindOnce(&VerifyAsyncTask, &remaining_expectations,
+                                 test, run_loop.QuitClosure());
+  extensions::app_file_handler_util::GetMimeTypeForLocalPath(
+      profile, entries[0].path,
+      base::BindLambdaForTesting([&](const std::string& mime_type) {
+        entries[0].mime_type = mime_type;
+        EXPECT_EQ(entries[0].mime_type, "image/gif");
+        FindAllTypesOfTasks(profile, entries, urls, std::move(verifier));
+      }));
+  run_loop.Run();
+  EXPECT_EQ(remaining_expectations, 0);
+}
+
 }  // namespace file_tasks
 }  // namespace file_manager

chrome/browser/chromeos/file_manager/web_file_tasks.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/apps/app_service/launch_utils.h"
 #include "chrome/browser/chromeos/file_manager/file_tasks.h"
+#include "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/file_handler_manager.h"
@@ -37,6 +38,12 @@ void FindWebTasks(Profile* profile,
   DCHECK(!entries.empty());
   DCHECK(result_list);
 
+  // WebApps only support files backed by inodes. See https://crbug.com/1079065.
+  for (const auto& entry : entries) {
+    if (util::IsUnderNonNativeLocalPath(profile, entry.path))
+      return;
+  }
+
   web_app::WebAppProviderBase* provider =
       web_app::WebAppProviderBase::GetProviderBase(profile);
   web_app::AppRegistrar& registrar = provider->registrar();

chrome/test/data/extensions/api_test/file_browser/image_provider/background.js

+// Copyright 2020 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.
+
+'use strict';
+
+// A 1x1 transparent GIF in 42 bytes.
+const GIF_DATA = new Uint8Array([
+  71, 73, 70,  56,  57,  97, 1,   0, 1, 0, 128, 0,  0, 0,
+  0,  0,  255, 255, 255, 33, 249, 4, 1, 0, 0,   0,  0, 44,
+  0,  0,  0,   0,   1,   0,  1,   0, 0, 2, 1,   68, 0, 59
+]);
+const GIF_FILE = new File([GIF_DATA], 'pixel.gif', {type: 'image/gif'});
+
+const GIF_ENTRY = Object.freeze({
+  isDirectory: false,
+  name: GIF_FILE.name,
+  size: GIF_FILE.size,
+  modificationTime: new Date(),
+  mimeType: GIF_FILE.type,
+});
+const ROOT_ENTRY = Object.freeze({
+  isDirectory: true,
+  name: '',
+  size: 0,
+  modificationTime: new Date(),
+  mimeType: 'text/directory',
+});
+
+const ENTRY_PATHS = {
+  ['/']: ROOT_ENTRY,
+  [`/${GIF_ENTRY.name}`]: GIF_ENTRY
+};
+
+function trace(...args) {
+  console.log(...args);
+}
+
+function mountFileSystem() {
+  chrome.fileSystemProvider.mount({
+    fileSystemId: 'test-image-provider-fs',
+    displayName: 'Test Image Provider FS'
+  });
+}
+
+/** Copies and adjusts `entryTemplate` to suit the request in `options`. */
+function makeEntry(entryTemplate, options) {
+  const entry = {...entryTemplate};
+  for (const prop
+           of ['name', 'mimeType', 'modificationTime', 'isDirectory', 'size']) {
+    if (!options[prop]) {
+      delete entry[prop];
+    }
+  }
+  return entry;
+}
+
+/** Find an entry. Invokes `onError('NOT_FOUND')` if `entry` is unknown. */
+function findEntry(entryPath, onError, options, operation) {
+  trace(operation, entryPath, options);
+  const entry = ENTRY_PATHS[entryPath];
+  if (!entry) {
+    console.log(
+        `Request for '${entryPath}': NOT_FOUND. ${JSON.stringify(options)}`);
+    onError('NOT_FOUND');
+  }
+  return entry;
+}
+
+chrome.fileSystemProvider.onGetMetadataRequested.addListener(function(
+    options, onSuccess, onError) {
+  let entry = findEntry(options.entryPath, onError, options, 'metadata');
+  if (entry) {
+    onSuccess(makeEntry(entry, options));
+  }
+});
+
+chrome.fileSystemProvider.onOpenFileRequested.addListener(function(
+    options, onSuccess, onError) {
+  if (findEntry(options.filePath, onError, options, 'open')) {
+    trace('open-success');
+    onSuccess();
+  }
+});
+
+chrome.fileSystemProvider.onCloseFileRequested.addListener(function(
+    options, onSuccess, onError) {
+  trace('close-file', options);
+  onSuccess();
+  trace('close-success');
+});
+
+async function readGif(onSuccess) {
+  const arrayBuffer = await GIF_FILE.arrayBuffer();
+  onSuccess(arrayBuffer, false /* hasMore*/);
+  trace('read-success');
+}
+
+chrome.fileSystemProvider.onReadFileRequested.addListener(function(
+    options, onSuccess, onError) {
+  trace('read-file', options);
+  // Assume it's GIF for now.
+  readGif(onSuccess);
+});
+
+chrome.fileSystemProvider.onReadDirectoryRequested.addListener(function(
+    options, onSuccess, onError) {
+  trace('open, ', options);
+  // For anything other than root, return no entries.
+  if (options.directoryPath !== '/') {
+    onSuccess([], false /* hasMore */);
+    return;
+  }
+  const entries = [makeEntry(GIF_ENTRY, options)];
+  onSuccess(entries, false /* hasMore */);
+});
+
+chrome.fileSystemProvider.onUnmountRequested.addListener(function(
+    options, onSuccess, onError) {
+  trace('unmount', options);
+  chrome.fileSystemProvider.unmount(
+      {fileSystemId: options.fileSystemId}, onSuccess)
+});
+
+chrome.fileSystemProvider.onGetActionsRequested.addListener(function(
+    options, onSuccess, onError) {
+  trace('actions-requested', options);
+  onSuccess([]);
+});
+
+// Hook onInstalled rather than onLaunched so it appears immediately.
+chrome.runtime.onInstalled.addListener(mountFileSystem);

chrome/test/data/extensions/api_test/file_browser/image_provider/manifest.json

+{
+  // chrome-extension://pkplfbidichfdicaijlchgnapepdginl
+  "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtDfX9dHNh948bt00YhZBm3P6E5QLaOt+v8kXVtibQfiPtOD2FTScB/f0wX/EQWVO7BkaSOsRkTPcPIgocyMPYr2FLgqGLFlYT9nQpKJZUFNF5oJ5rG6Nv7ppf4zEB3j6da1IBRTz2yOZ+6O1TMZxol/V62/QcqrJeggsHTEPGLdr9Ua4b1Ka0xKJnJngZljsbw93FI1o+P9dAh5BS6wTPiZI/vmJVjvMTkSTnaZ3n9Go2t7A0XLcSxLcVyuLAd2mAvSN0mIviOukdM66wr7llif71nKuUt+4qvlr/r9HfwzN6pA4jkwhtS1UD+3CmB+wsHwsnohNcuu4FIQ6rgq/7QIDAQAB",
+  "name": "Testing File System Provider implementation",
+  "version": "0.1",
+  "manifest_version": 2,
+  "description": "Testing Provided File System files",
+  "permissions": [
+    "fileSystemProvider"
+  ],
+  "file_system_provider_capabilities": {
+    "source": "file"
+  },
+  "background": {
+    "scripts": ["background.js"]
+  }
+}