Add special-casing for hosted docs in DriveFS for browser open dialogs.

Hosted docs need to be redirected to their URLs to open successfully.
Hosted docs opened from the files app apply this transformation, but
files selected via the open dialog do not. Add this special-casing for
open dialog opens as well.

Bug: 906902
No-try: true
Change-Id: Ie64a6d25c97a3954cfa6d7e2694f29e191777257
Reviewed-on: https://chromium-review.googlesource.com/c/1343473
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sergei Datsenko <dats@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611070}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -710,6 +710,7 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
             .WithBrowser()
             .InIncognito()
             .EnableDriveFs(),
+        TestCase("openFileDialogDriveHostedDoc").WithBrowser(),
         TestCase("openFileDialogCancelDrive").WithBrowser().DisableDriveFs(),
         TestCase("openFileDialogCancelDrive").WithBrowser().EnableDriveFs(),
         TestCase("openFileDialogEscapeDrive").WithBrowser().DisableDriveFs(),

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -54,6 +54,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/service_manager_connection.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/api/test/test_api.h"
 #include "extensions/browser/app_window/app_window.h"
@@ -1608,6 +1609,15 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
     return;
   }
 
+  if (name == "runSelectFileDialog") {
+    browser()->OpenFile();
+    content::TestNavigationObserver observer(
+        browser()->tab_strip_model()->GetActiveWebContents(), 1);
+    observer.Wait();
+    *output = observer.last_navigation_url().spec();
+    return;
+  }
+
   FAIL() << "Unknown test message: " << name;
 }
 

chrome/browser/chromeos/fileapi/external_file_url_util.cc

@@ -9,10 +9,14 @@
 
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "chrome/browser/chromeos/drive/drive_integration_service.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 "chromeos/components/drivefs/mojom/drivefs.mojom.h"
+#include "components/drive/drive_api_util.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
@@ -23,6 +27,24 @@
 using content::BrowserThread;
 
 namespace chromeos {
+namespace {
+
+void ExtractHostedFileUrl(base::OnceCallback<void(GURL)> callback,
+                          drive::FileError error,
+                          drivefs::mojom::FileMetadataPtr metadata) {
+  if (error != drive::FILE_ERROR_OK) {
+    std::move(callback).Run({});
+    return;
+  }
+  if (metadata->type != drivefs::mojom::FileMetadata::Type::kHosted) {
+    std::move(callback).Run({});
+    return;
+  }
+  GURL hosted_url(metadata->alternate_url);
+  std::move(callback).Run(hosted_url.is_valid() ? hosted_url : GURL());
+}
+
+}  // namespace
 
 bool IsExternalFileURLType(storage::FileSystemType type) {
   return type == storage::kFileSystemTypeDrive ||
@@ -78,4 +100,51 @@ GURL CreateExternalFileURLFromPath(Profile* profile,
   return FileSystemURLToExternalFileURL(file_system_url);
 }
 
+void ResolveExternalFileUrlFromPath(Profile* profile,
+                                    const base::FilePath& path,
+                                    base::OnceCallback<void(GURL)> callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  GURL raw_file_system_url;
+  if (!file_manager::util::ConvertAbsoluteFilePathToFileSystemUrl(
+          profile, path, file_manager::kFileManagerAppId,
+          &raw_file_system_url)) {
+    std::move(callback).Run({});
+    return;
+  }
+
+  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()) {
+    std::move(callback).Run({});
+    return;
+  }
+
+  auto external_file_url = FileSystemURLToExternalFileURL(file_system_url);
+  if (!external_file_url.is_empty()) {
+    std::move(callback).Run(std::move(external_file_url));
+    return;
+  }
+
+  if (file_system_url.type() != storage::kFileSystemTypeDriveFs ||
+      !drive::util::HasHostedDocumentExtension(path)) {
+    std::move(callback).Run({});
+    return;
+  }
+
+  drive::DriveIntegrationService* integration_service =
+      drive::util::GetIntegrationServiceByProfile(profile);
+  base::FilePath mount_relative_path;
+  if (!integration_service || !integration_service->GetDriveFsInterface() ||
+      !integration_service->GetRelativeDrivePath(path, &mount_relative_path)) {
+    std::move(callback).Run({});
+    return;
+  }
+  integration_service->GetDriveFsInterface()->GetMetadata(
+      mount_relative_path,
+      base::BindOnce(&ExtractHostedFileUrl, std::move(callback)));
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/fileapi/external_file_url_util.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_FILEAPI_EXTERNAL_FILE_URL_UTIL_H_
 #define CHROME_BROWSER_CHROMEOS_FILEAPI_EXTERNAL_FILE_URL_UTIL_H_
 
+#include "base/callback_forward.h"
 #include "storage/common/fileapi/file_system_types.h"
 
 class GURL;
@@ -41,6 +42,15 @@ GURL VirtualPathToExternalFileURL(const base::FilePath& virtual_path);
 GURL CreateExternalFileURLFromPath(Profile* profile,
                                    const base::FilePath& path);
 
+// Obtains, from a file path (e.g. /special/drive-xxx/root/sample.txt), an
+// external file URL (e.g. external:drive/root/sample.txt), if the |path| points
+// an external location (drive, MTP, or FSP), or its hosted file URL if |path|
+// refers to a hosted doc (e.g. gdoc) in DriveFS. If neither condition applies,
+// |callback| is invoked with an empty GURL.
+void ResolveExternalFileUrlFromPath(Profile* profile,
+                                    const base::FilePath& path,
+                                    base::OnceCallback<void(GURL)> callback);
+
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_FILEAPI_EXTERNAL_FILE_URL_UTIL_H_

chrome/browser/ui/browser.cc

@@ -1986,18 +1986,29 @@ void Browser::FileSelectedWithExtraInfo(const ui::SelectedFileInfo& file_info,
 
   GURL url = net::FilePathToFileURL(file_info.local_path);
 
-#if defined(OS_CHROMEOS)
-  const GURL external_url =
-      chromeos::CreateExternalFileURLFromPath(profile_, file_info.file_path);
-  if (!external_url.is_empty())
-    url = external_url;
-#endif
-
   if (url.is_empty())
     return;
 
+#if defined(OS_CHROMEOS)
+  chromeos::ResolveExternalFileUrlFromPath(
+      profile_, file_info.file_path,
+      base::BindOnce(
+          [](base::WeakPtr<Browser> weak_this, GURL url, GURL external_url) {
+            if (!weak_this)
+              return;
+
+            if (!external_url.is_empty())
+              url = std::move(external_url);
+
+            weak_this->OpenURL(OpenURLParams(url, Referrer(),
+                                             WindowOpenDisposition::CURRENT_TAB,
+                                             ui::PAGE_TRANSITION_TYPED, false));
+          },
+          weak_factory_.GetWeakPtr(), std::move(url)));
+#else
   OpenURL(OpenURLParams(url, Referrer(), WindowOpenDisposition::CURRENT_TAB,
                         ui::PAGE_TRANSITION_TYPED, false));
+#endif
 }
 
 ///////////////////////////////////////////////////////////////////////////////

ui/file_manager/integration_tests/file_manager/background.js

@@ -302,18 +302,26 @@ function openNewWindow(appState, initialRoot, opt_callback) {
  * @param {Array<TestEntryInfo>} expectedSet Expected set of the entries.
  * @param {function(windowId:string):Promise} closeDialog Function to close the
  *     dialog.
+ * @param {boolean} useBrowserOpen Whether to launch the select file dialog via
+ *     a browser OpenFile() call.
  * @return {Promise} Promise to be fulfilled with the result entry of the
  *     dialog.
  */
 function openAndWaitForClosingDialog(
-    dialogParams, volumeName, expectedSet, closeDialog) {
+    dialogParams, volumeName, expectedSet, closeDialog,
+    useBrowserOpen = false) {
   var caller = getCaller();
-  var resultPromise = new Promise(function(fulfill) {
-    chrome.fileSystem.chooseEntry(
-        dialogParams,
-        function(entry) { fulfill(entry); });
-    chrome.test.assertTrue(!chrome.runtime.lastError, 'chooseEntry failed.');
-  });
+  var resultPromise;
+  if (useBrowserOpen) {
+    resultPromise = sendTestMessage({name: 'runSelectFileDialog'});
+  } else {
+    resultPromise = new Promise(function(fulfill) {
+      chrome.fileSystem.chooseEntry(dialogParams, function(entry) {
+        fulfill(entry);
+      });
+      chrome.test.assertTrue(!chrome.runtime.lastError, 'chooseEntry failed.');
+    });
+  }
 
   return remoteCall.waitForWindow('dialog#').then(function(windowId) {
     return remoteCall.waitForElement(windowId, '#file-list').

ui/file_manager/integration_tests/file_manager/file_dialog.js

@@ -70,13 +70,13 @@ function unloadOpenFileDialog(dialog, element = '.button-panel button.ok') {
  */
 function createFileEntryPromise(volume) {
   let localEntryPromise = addEntries(['local'], BASIC_LOCAL_ENTRY_SET);
-  let driveEntryPromise =
-      addEntries(['drive'], [ENTRIES.hello, ENTRIES.pinned]);
+  let driveEntryPromise = addEntries(
+      ['drive'], [ENTRIES.hello, ENTRIES.pinned, ENTRIES.testDocument]);
 
   return Promise.all([localEntryPromise, driveEntryPromise])
       .then(function returnVolumeEntrySet() {
         if (volume == 'drive')
-          return [ENTRIES.hello, ENTRIES.pinned];
+          return [ENTRIES.hello, ENTRIES.pinned, ENTRIES.testDocument];
         return BASIC_LOCAL_ENTRY_SET;
       });
 }
@@ -89,7 +89,7 @@ function createFileEntryPromise(volume) {
  * @param {!string} name File name to select in the dialog.
  * @return {!Promise} Promise to be fulfilled on success.
  */
-function openFileDialogClickOkButton(volume, name) {
+function openFileDialogClickOkButton(volume, name, expectedUrl = undefined) {
   const type = {type: 'openFile'};
 
   const okButton = '.button-panel button.ok:enabled';
@@ -97,10 +97,15 @@ function openFileDialogClickOkButton(volume, name) {
 
   return createFileEntryPromise(volume)
       .then(function openFileDialog(entrySet) {
-        return openAndWaitForClosingDialog(type, volume, entrySet, closer);
+        return openAndWaitForClosingDialog(
+            type, volume, entrySet, closer, !!expectedUrl);
       })
       .then(function expectDialogFileEntry(result) {
-        chrome.test.assertEq(name, result.name);
+        if (expectedUrl) {
+          chrome.test.assertEq(expectedUrl, result);
+        } else {
+          chrome.test.assertEq(name, result.name);
+        }
       });
 }
 
@@ -249,6 +254,16 @@ testcase.openFileDialogDriveOfflinePinned = function() {
   testPromise(openFileDialogClickOkButton('drive', TEST_DRIVE_PINNED_FILE));
 };
 
+/**
+ * Tests opening a hosted doc in the browser, ensuring it correctly navigates to
+ * the doc's URL.
+ */
+testcase.openFileDialogDriveHostedDoc = function() {
+  testPromise(openFileDialogClickOkButton(
+      'drive', ENTRIES.testDocument.nameText,
+      'https://document_alternate_link/Test%20Document'));
+};
+
 /**
  * Tests opening file dialog on Drive and closing it with Cancel button.
  */