Change chrome.fileSystem.chooseEntry() behavior for extensions.

For extensions, instead of having a per-extension last-saved location,
use the browser downloads location.

Adjust some file_manager tests to pass with this change. In production,
file_manager is not affected by this because it is an app and not an
extension. However, some of its tests use a test extension, so the tests
are affected.

Bug: 1011587
Change-Id: I62d08a9b1855ee8e5454098d37ca5a8a685f9d22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2245686Reviewed-by: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782110}

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -45,6 +45,7 @@
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/smb_client/smb_service.h"
 #include "chrome/browser/chromeos/smb_client/smb_service_factory.h"
+#include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/sync_file_system/mock_remote_file_sync_service.h"
@@ -2452,6 +2453,14 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
     return;
   }
 
+  if (name == "setLastDownloadDir") {
+    base::FilePath downloads_path(util::GetDownloadsMountPointName(profile()));
+    downloads_path = downloads_path.AppendASCII("Downloads");
+    auto* download_prefs = DownloadPrefs::FromBrowserContext(profile());
+    download_prefs->SetSaveFilePath(downloads_path);
+    return;
+  }
+
   FAIL() << "Unknown test message: " << name;
 }
 

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/extensions/activity_log/activity_log.h"
 #include "chrome/browser/extensions/api/chrome_extensions_api_client.h"
 #include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
@@ -67,7 +68,6 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/extensions/updater/chromeos_extension_cache_delegate.h"
 #include "chrome/browser/extensions/updater/extension_cache_impl.h"
 #include "chromeos/constants/chromeos_switches.h"
@@ -564,6 +564,19 @@ bool ChromeExtensionsBrowserClient::ShouldForceWebRequestExtraHeaders(
   return apply_cors_mitigation_list;
 }
 
+base::FilePath ChromeExtensionsBrowserClient::GetSaveFilePath(
+    content::BrowserContext* context) {
+  DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext(context);
+  return download_prefs->SaveFilePath();
+}
+
+void ChromeExtensionsBrowserClient::SetLastSaveFilePath(
+    content::BrowserContext* context,
+    const base::FilePath& path) {
+  DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext(context);
+  download_prefs->SetSaveFilePath(path);
+}
+
 const MediaRouterExtensionAccessLogger*
 ChromeExtensionsBrowserClient::GetMediaRouterAccessLogger() const {
   return g_media_router_access_logger_for_testing

chrome/browser/extensions/chrome_extensions_browser_client.h

@@ -151,7 +151,9 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
       const std::string& scheme) const override;
   bool ShouldForceWebRequestExtraHeaders(
       content::BrowserContext* context) const override;
-
+  base::FilePath GetSaveFilePath(content::BrowserContext* context) override;
+  void SetLastSaveFilePath(content::BrowserContext* context,
+                           const base::FilePath& path) override;
   const MediaRouterExtensionAccessLogger* GetMediaRouterAccessLogger()
       const override;
 

extensions/browser/api/file_system/file_system_api.cc

@@ -497,9 +497,16 @@ void FileSystemChooseEntryFunction::FilesSelected(
   } else {
     last_choose_directory = paths[0].DirName();
   }
-  file_system_api::SetLastChooseEntryDirectory(
-      ExtensionPrefs::Get(browser_context()), extension()->id(),
-      last_choose_directory);
+
+  if (extension_->is_extension()) {
+    ExtensionsBrowserClient::Get()->SetLastSaveFilePath(browser_context(),
+                                                        last_choose_directory);
+  } else {
+    file_system_api::SetLastChooseEntryDirectory(
+        ExtensionPrefs::Get(browser_context()), extension()->id(),
+        last_choose_directory);
+  }
+
   if (is_directory_) {
     // Get the WebContents for the app window to be the parent window of the
     // confirmation dialog if necessary.
@@ -618,10 +625,10 @@ void FileSystemChooseEntryFunction::BuildFileTypeInfo(
     ui::SelectFileDialog::FileTypeInfo* file_type_info,
     const base::FilePath::StringType& suggested_extension,
     const AcceptOptions* accepts,
-    const bool* acceptsAllTypes) {
+    const bool* accepts_all_types) {
   file_type_info->include_all_files = true;
-  if (acceptsAllTypes)
-    file_type_info->include_all_files = *acceptsAllTypes;
+  if (accepts_all_types)
+    file_type_info->include_all_files = *accepts_all_types;
 
   bool need_suggestion =
       !file_type_info->include_all_files && !suggested_extension.empty();
@@ -744,8 +751,14 @@ ExtensionFunction::ResponseAction FileSystemChooseEntryFunction::Run() {
 
   file_type_info.allowed_paths = ui::SelectFileDialog::FileTypeInfo::ANY_PATH;
 
-  base::FilePath previous_path = file_system_api::GetLastChooseEntryDirectory(
-      ExtensionPrefs::Get(browser_context()), extension()->id());
+  base::FilePath previous_path;
+  if (extension_->is_extension()) {
+    previous_path =
+        ExtensionsBrowserClient::Get()->GetSaveFilePath(browser_context());
+  } else {
+    previous_path = file_system_api::GetLastChooseEntryDirectory(
+        ExtensionPrefs::Get(browser_context()), extension()->id());
+  }
 
   if (previous_path.empty()) {
     SetInitialPathAndShowPicker(previous_path, suggested_name, file_type_info,

extensions/browser/api/file_system/file_system_api.h

@@ -140,7 +140,7 @@ class FileSystemChooseEntryFunction : public FileSystemEntryFunction {
       ui::SelectFileDialog::FileTypeInfo* file_type_info,
       const base::FilePath::StringType& suggested_extension,
       const AcceptOptions* accepts,
-      const bool* acceptsAllTypes);
+      const bool* accepts_all_types);
   static void BuildSuggestion(const std::string* opt_name,
                               base::FilePath* suggested_name,
                               base::FilePath::StringType* suggested_extension);

extensions/browser/extensions_browser_client.cc

@@ -104,6 +104,15 @@ bool ExtensionsBrowserClient::ShouldForceWebRequestExtraHeaders(
   return false;
 }
 
+base::FilePath ExtensionsBrowserClient::GetSaveFilePath(
+    content::BrowserContext* context) {
+  return base::FilePath();
+}
+
+void ExtensionsBrowserClient::SetLastSaveFilePath(
+    content::BrowserContext* context,
+    const base::FilePath& path) {}
+
 const MediaRouterExtensionAccessLogger*
 ExtensionsBrowserClient::GetMediaRouterAccessLogger() const {
   return nullptr;

extensions/browser/extensions_browser_client.h

@@ -349,6 +349,11 @@ class ExtensionsBrowserClient {
   virtual bool ShouldForceWebRequestExtraHeaders(
       content::BrowserContext* context) const;
 
+  // Gets and sets the last save (download) path for a given context.
+  virtual base::FilePath GetSaveFilePath(content::BrowserContext* context);
+  virtual void SetLastSaveFilePath(content::BrowserContext* context,
+                                   const base::FilePath& path);
+
   // Retrieves the media router access logger for this session.
   virtual const MediaRouterExtensionAccessLogger* GetMediaRouterAccessLogger()
       const;

ui/file_manager/integration_tests/file_manager/file_dialog.js

@@ -563,6 +563,9 @@ testcase.saveFileDialogExtensionNotAddedWhenProvided = async () => {
  * crbug.com/917975 crbug.com/983507.
  */
 testcase.openFileDialogFileListShowContextMenu = async () => {
+  // Make sure the file picker will open to Downloads.
+  sendBrowserTestCommand({name: 'setLastDownloadDir'}, () => {});
+
   // Add entries to Downloads.
   await addEntries(['local'], BASIC_LOCAL_ENTRY_SET);
 
@@ -639,6 +642,9 @@ testcase.openFileDialogSelectAllDisabled = async () => {
  * dialog. crbug.com/937251
  */
 testcase.openMultiFileDialogSelectAllEnabled = async () => {
+  // Make sure the file picker will open to Downloads.
+  sendBrowserTestCommand({name: 'setLastDownloadDir'}, () => {});
+
   // Open file picker dialog with support for selecting multiple files.
   chrome.fileSystem.chooseEntry(
       {type: 'openFile', acceptsMultiple: true}, (entry) => {});