Add per-origin caching of the most recently selected directory

- adds FILE_SYSTEM_LAST_PICKED_DIRECTORY website setting
- prerequisite to the implementation of suggested filenames and
  directories for the file chooser

Bug: 1139907
Change-Id: I886cc4c6552cefbf3780e20d5b216ad460646e22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478901Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Auto-Submit: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825357}

chrome/android/javatests/src/org/chromium/chrome/browser/site_settings/WebsitePermissionsFetcherTest.java

@@ -475,7 +475,7 @@ public class WebsitePermissionsFetcherTest {
         // If the ContentSettingsType.NUM_TYPES value changes *and* a new value has been exposed on
         // Android, then please update this code block to include a test for your new type.
         // Otherwise, just update count in the assert.
-        Assert.assertEquals(66, ContentSettingsType.NUM_TYPES);
+        Assert.assertEquals(67, ContentSettingsType.NUM_TYPES);
         websitePreferenceBridge.addContentSettingException(
                 new ContentSettingException(ContentSettingsType.COOKIES, googleOrigin,
                         ContentSettingValues.DEFAULT, preferenceSource));

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -787,6 +787,10 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
         ContentSettingsType::INTENT_PICKER_DISPLAY, delete_begin_, delete_end_,
         website_settings_filter);
+
+    host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
+        ContentSettingsType::FILE_SYSTEM_LAST_PICKED_DIRECTORY, delete_begin,
+        delete_end, website_settings_filter);
 #endif
 
     PermissionDecisionAutoBlockerFactory::GetForProfile(profile_)

chrome/browser/native_file_system/chrome_native_file_system_permission_context.cc

@@ -9,9 +9,11 @@
 
 #include "base/base_paths.h"
 #include "base/bind.h"
+#include "base/files/file_path.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/path_service.h"
 #include "base/task/thread_pool.h"
+#include "base/util/values/values_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
@@ -34,6 +36,9 @@ namespace {
 
 using HandleType = content::NativeFileSystemPermissionContext::HandleType;
 
+// Dictionary key for the FILE_SYSTEM_LAST_PICKED_DIRECTORY website setting.
+const char kLastPickedDirectoryKey[] = "default-path";
+
 void ShowNativeFileSystemRestrictedDirectoryDialogOnUIThread(
     content::GlobalFrameRoutingId frame_id,
     const url::Origin& origin,
@@ -432,3 +437,34 @@ bool ChromeNativeFileSystemPermissionContext::OriginHasWriteAccess(
   NOTREACHED();
   return false;
 }
+
+void ChromeNativeFileSystemPermissionContext::SetLastPickedDirectory(
+    const url::Origin& origin,
+    const base::FilePath& path) {
+  base::Value dict(base::Value::Type::DICTIONARY);
+  dict.SetKey(kLastPickedDirectoryKey, util::FilePathToValue(path));
+
+  content_settings_->SetWebsiteSettingDefaultScope(
+      origin.GetURL(), origin.GetURL(),
+      ContentSettingsType::FILE_SYSTEM_LAST_PICKED_DIRECTORY,
+      base::Value::ToUniquePtrValue(std::move(dict)));
+}
+
+base::FilePath ChromeNativeFileSystemPermissionContext::GetLastPickedDirectory(
+    const url::Origin& origin) {
+  std::unique_ptr<base::Value> value = content_settings()->GetWebsiteSetting(
+      origin.GetURL(), origin.GetURL(),
+      ContentSettingsType::FILE_SYSTEM_LAST_PICKED_DIRECTORY, /*info=*/nullptr);
+  if (!value)
+    return base::FilePath();
+
+  return util::ValueToFilePath(value->FindKey(kLastPickedDirectoryKey))
+      .value_or(base::FilePath());
+}
+
+base::FilePath ChromeNativeFileSystemPermissionContext::GetDefaultDirectory() {
+  base::FilePath default_path;
+  // On failure, |default_path| will remain empty.
+  base::PathService::Get(chrome::DIR_USER_DOCUMENTS, &default_path);
+  return default_path;
+}

chrome/browser/native_file_system/chrome_native_file_system_permission_context.h

@@ -60,6 +60,11 @@ class ChromeNativeFileSystemPermissionContext
   bool CanObtainReadPermission(const url::Origin& origin) override;
   bool CanObtainWritePermission(const url::Origin& origin) override;
 
+  void SetLastPickedDirectory(const url::Origin& origin,
+                              const base::FilePath& path) override;
+  base::FilePath GetLastPickedDirectory(const url::Origin& origin) override;
+  base::FilePath GetDefaultDirectory() override;
+
   ContentSetting GetReadGuardContentSetting(const url::Origin& origin);
   ContentSetting GetWriteGuardContentSetting(const url::Origin& origin);
 

chrome/browser/native_file_system/chrome_native_file_system_permission_context_unittest.cc

@@ -416,4 +416,49 @@ TEST_F(ChromeNativeFileSystemPermissionContextTest, PolicyWriteBlockedForUrls) {
   EXPECT_TRUE(permission_context()->CanObtainWritePermission(kTestOrigin2));
 }
 
+TEST_F(ChromeNativeFileSystemPermissionContextTest, GetLastPickedDirectory) {
+  EXPECT_EQ(permission_context()->GetLastPickedDirectory(kTestOrigin),
+            base::FilePath());
+}
+
+TEST_F(ChromeNativeFileSystemPermissionContextTest, SetLastPickedDirectory) {
+  EXPECT_EQ(permission_context()->GetLastPickedDirectory(kTestOrigin),
+            base::FilePath());
+
+  permission_context()->SetLastPickedDirectory(kTestOrigin, kTestPath);
+  auto path = permission_context()->GetLastPickedDirectory(kTestOrigin);
+  EXPECT_EQ(path, kTestPath);
+
+  auto new_path = path.AppendASCII("baz");
+  permission_context()->SetLastPickedDirectory(kTestOrigin, new_path);
+  EXPECT_EQ(permission_context()->GetLastPickedDirectory(kTestOrigin),
+            new_path);
+}
+
+TEST_F(ChromeNativeFileSystemPermissionContextTest,
+       SetLastPickedDirectory_NewPermissionContext) {
+  EXPECT_EQ(permission_context()->GetLastPickedDirectory(kTestOrigin),
+            base::FilePath());
+
+  const base::FilePath path = base::FilePath(FILE_PATH_LITERAL("/baz/bar"));
+
+  permission_context()->SetLastPickedDirectory(kTestOrigin, path);
+  ASSERT_EQ(permission_context()->GetLastPickedDirectory(kTestOrigin), path);
+
+  TestNativeFileSystemPermissionContext new_permission_context(
+      browser_context());
+  EXPECT_EQ(new_permission_context.GetLastPickedDirectory(kTestOrigin), path);
+
+  auto new_path = path.AppendASCII("foo");
+  new_permission_context.SetLastPickedDirectory(kTestOrigin, new_path);
+  EXPECT_EQ(permission_context()->GetLastPickedDirectory(kTestOrigin),
+            new_path);
+}
+
+TEST_F(ChromeNativeFileSystemPermissionContextTest, GetDefaultDirectory) {
+  base::ScopedPathOverride user_documents_override(
+      chrome::DIR_USER_DOCUMENTS, temp_dir_.GetPath(), true, true);
+  EXPECT_EQ(permission_context_->GetDefaultDirectory(), temp_dir_.GetPath());
+}
+
 #endif  // !defined(OS_ANDROID)

chrome/browser/ui/webui/settings/site_settings_helper.cc

@@ -149,6 +149,7 @@ const ContentSettingsTypeNameEntry kContentSettingsTypeGroupNames[] = {
     {ContentSettingsType::CAMERA_PAN_TILT_ZOOM, nullptr},
     {ContentSettingsType::INSECURE_PRIVATE_NETWORK, nullptr},
     {ContentSettingsType::PERMISSION_AUTOREVOCATION_DATA, nullptr},
+    {ContentSettingsType::FILE_SYSTEM_LAST_PICKED_DIRECTORY, nullptr},
 };
 static_assert(base::size(kContentSettingsTypeGroupNames) ==
                   // ContentSettingsType starts at -1, so add 1 here.

components/content_settings/core/browser/website_settings_registry.cc

@@ -226,6 +226,11 @@ void WebsiteSettingsRegistry::Init() {
            WebsiteSettingsInfo::SINGLE_ORIGIN_ONLY_SCOPE,
            DESKTOP | PLATFORM_ANDROID,
            WebsiteSettingsInfo::DONT_INHERIT_IN_INCOGNITO);
+  Register(ContentSettingsType::FILE_SYSTEM_LAST_PICKED_DIRECTORY,
+           "file-system-last-picked-directory", nullptr,
+           WebsiteSettingsInfo::UNSYNCABLE, WebsiteSettingsInfo::NOT_LOSSY,
+           WebsiteSettingsInfo::SINGLE_ORIGIN_ONLY_SCOPE, DESKTOP,
+           WebsiteSettingsInfo::DONT_INHERIT_IN_INCOGNITO);
 }
 
 }  // namespace content_settings

components/content_settings/core/common/content_settings.cc

@@ -93,6 +93,7 @@ constexpr HistogramValue kHistogramValue[] = {
     {ContentSettingsType::INSECURE_PRIVATE_NETWORK, 70},
     {ContentSettingsType::FONT_ACCESS, 71},
     {ContentSettingsType::PERMISSION_AUTOREVOCATION_DATA, 72},
+    {ContentSettingsType::FILE_SYSTEM_LAST_PICKED_DIRECTORY, 73},
 };
 
 }  // namespace

components/content_settings/core/common/content_settings_types.h

@@ -229,6 +229,10 @@ enum class ContentSettingsType : int32_t {
   // types).
   PERMISSION_AUTOREVOCATION_DATA,
 
+  // Stores per-origin state of the most recently selected directory for the use
+  // by the File System Access API.
+  FILE_SYSTEM_LAST_PICKED_DIRECTORY,
+
   NUM_TYPES,
 };
 

content/browser/file_system_access/file_system_chooser.cc

@@ -133,10 +133,12 @@ ui::SelectFileDialog::FileTypeInfo ConvertAcceptsToFileTypeInfo(
 FileSystemChooser::Options::Options(
     blink::mojom::ChooseFileSystemEntryType type,
     std::vector<blink::mojom::ChooseFileSystemEntryAcceptsOptionPtr> accepts,
-    bool include_accepts_all)
+    bool include_accepts_all,
+    base::FilePath default_path)
     : type_(type),
       file_types_(ConvertAcceptsToFileTypeInfo(accepts, include_accepts_all)),
-      default_file_type_index_(file_types_.extensions.empty() ? 0 : 1) {}
+      default_file_type_index_(file_types_.extensions.empty() ? 0 : 1),
+      default_path_(std::move(default_path)) {}
 
 // static
 void FileSystemChooser::CreateAndShow(
@@ -179,9 +181,8 @@ void FileSystemChooser::CreateAndShow(
   DCHECK_NE(dialog_type, ui::SelectFileDialog::SELECT_NONE);
 
   listener->dialog_->SelectFile(
-      dialog_type, /*title=*/base::string16(),
-      /*default_path=*/base::FilePath(), &options.file_type_info(),
-      options.default_file_type_index(),
+      dialog_type, /*title=*/base::string16(), options.default_path(),
+      &options.file_type_info(), options.default_file_type_index(),
       /*default_extension=*/base::FilePath::StringType(),
       web_contents ? web_contents->GetTopLevelNativeWindow() : nullptr,
       /*params=*/nullptr);

content/browser/file_system_access/file_system_chooser.h

@@ -41,7 +41,8 @@ class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener {
     Options(blink::mojom::ChooseFileSystemEntryType type,
             std::vector<blink::mojom::ChooseFileSystemEntryAcceptsOptionPtr>
                 accepts,
-            bool include_accepts_all);
+            bool include_accepts_all,
+            base::FilePath default_directory);
     Options(const Options&) = default;
     Options& operator=(const Options&) = default;
 
@@ -50,11 +51,13 @@ class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener {
       return file_types_;
     }
     int default_file_type_index() const { return default_file_type_index_; }
+    const base::FilePath& default_path() const { return default_path_; }
 
    private:
     blink::mojom::ChooseFileSystemEntryType type_;
     ui::SelectFileDialog::FileTypeInfo file_types_;
     int default_file_type_index_ = 0;
+    base::FilePath default_path_;
   };
 
   static void CreateAndShow(WebContents* web_contents,

content/browser/file_system_access/file_system_chooser_browsertest.cc

@@ -483,6 +483,12 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, OpenDirectory_DenyAccess) {
   EXPECT_CALL(permission_context, CanObtainReadPermission(origin))
       .WillOnce(testing::Return(true));
 
+  EXPECT_CALL(permission_context, GetLastPickedDirectory(origin))
+      .WillOnce(testing::Return(base::FilePath()));
+  EXPECT_CALL(permission_context, GetDefaultDirectory())
+      .WillOnce(testing::Return(base::FilePath()));
+  EXPECT_CALL(permission_context, SetLastPickedDirectory(origin, test_dir));
+
   EXPECT_CALL(
       permission_context,
       ConfirmSensitiveDirectoryAccess_(
@@ -545,6 +551,16 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest,
       shell()->web_contents()->GetMainFrame()->GetProcess()->GetID(),
       shell()->web_contents()->GetMainFrame()->GetRoutingID());
 
+  EXPECT_CALL(permission_context, CanObtainReadPermission(origin))
+      .WillOnce(testing::Return(true));
+  EXPECT_CALL(permission_context, CanObtainWritePermission(origin))
+      .WillOnce(testing::Return(true));
+
+  EXPECT_CALL(permission_context, GetLastPickedDirectory(origin))
+      .WillOnce(testing::Return(base::FilePath()));
+  EXPECT_CALL(permission_context, GetDefaultDirectory())
+      .WillOnce(testing::Return(base::FilePath()));
+
   EXPECT_CALL(
       permission_context,
       ConfirmSensitiveDirectoryAccess_(
@@ -553,15 +569,6 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest,
           frame_id, testing::_))
       .WillOnce(RunOnceCallback<5>(SensitiveDirectoryResult::kAbort));
 
-  EXPECT_CALL(permission_context,
-              CanObtainReadPermission(url::Origin::Create(
-                  embedded_test_server()->GetURL("/title1.html"))))
-      .WillOnce(testing::Return(true));
-  EXPECT_CALL(permission_context,
-              CanObtainWritePermission(url::Origin::Create(
-                  embedded_test_server()->GetURL("/title1.html"))))
-      .WillOnce(testing::Return(true));
-
   ASSERT_TRUE(
       NavigateToURL(shell(), embedded_test_server()->GetURL("/title1.html")));
   auto result = EvalJs(shell(), "self.showSaveFilePicker()");
@@ -605,6 +612,16 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest,
       shell()->web_contents()->GetMainFrame()->GetProcess()->GetID(),
       shell()->web_contents()->GetMainFrame()->GetRoutingID());
 
+  EXPECT_CALL(permission_context, CanObtainReadPermission(origin))
+      .WillOnce(testing::Return(true));
+  EXPECT_CALL(permission_context, CanObtainWritePermission(origin))
+      .WillOnce(testing::Return(true));
+
+  EXPECT_CALL(permission_context, GetLastPickedDirectory(origin))
+      .WillOnce(testing::Return(base::FilePath()));
+  EXPECT_CALL(permission_context, GetDefaultDirectory())
+      .WillOnce(testing::Return(base::FilePath()));
+
   EXPECT_CALL(
       permission_context,
       ConfirmSensitiveDirectoryAccess_(
@@ -613,11 +630,6 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest,
           frame_id, testing::_))
       .WillOnce(RunOnceCallback<5>(SensitiveDirectoryResult::kAbort));
 
-  EXPECT_CALL(permission_context, CanObtainReadPermission(origin))
-      .WillOnce(testing::Return(true));
-  EXPECT_CALL(permission_context, CanObtainWritePermission(origin))
-      .WillOnce(testing::Return(true));
-
   ASSERT_TRUE(
       NavigateToURL(shell(), embedded_test_server()->GetURL("/title1.html")));
   auto result = EvalJs(shell(), "self.showSaveFilePicker()");

content/browser/file_system_access/file_system_chooser_unittest.cc

@@ -33,7 +33,7 @@ class FileSystemChooserTest : public testing::Test {
         /*web_contents=*/nullptr,
         FileSystemChooser::Options(
             blink::mojom::ChooseFileSystemEntryType::kOpenFile,
-            std::move(accepts), include_accepts_all),
+            std::move(accepts), include_accepts_all, base::FilePath()),
         base::BindLambdaForTesting(
             [&](blink::mojom::NativeFileSystemErrorPtr,
                 std::vector<FileSystemChooser::ResultEntry> entries) {

content/browser/file_system_access/mock_native_file_system_permission_context.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_BROWSER_FILE_SYSTEM_ACCESS_MOCK_NATIVE_FILE_SYSTEM_PERMISSION_CONTEXT_H_
 #define CONTENT_BROWSER_FILE_SYSTEM_ACCESS_MOCK_NATIVE_FILE_SYSTEM_PERMISSION_CONTEXT_H_
 
+#include "base/files/file_path.h"
 #include "content/public/browser/native_file_system_permission_context.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -57,6 +58,12 @@ class MockNativeFileSystemPermissionContext
 
   MOCK_METHOD1(CanObtainReadPermission, bool(const url::Origin& origin));
   MOCK_METHOD1(CanObtainWritePermission, bool(const url::Origin& origin));
+
+  MOCK_METHOD2(SetLastPickedDirectory,
+               void(const url::Origin& origin, const base::FilePath& path));
+  MOCK_METHOD1(GetLastPickedDirectory,
+               base::FilePath(const url::Origin& origin));
+  MOCK_METHOD0(GetDefaultDirectory, base::FilePath());
 };
 
 }  // namespace content

content/browser/file_system_access/native_file_system_manager_impl.cc

@@ -310,8 +310,28 @@ void NativeFileSystemManagerImpl::ChooseEntries(
     return;
   }
 
+  // TODO(https://crbug.com/1142824): Check if path exists.
+  base::FilePath default_directory;
+  if (permission_context_) {
+    default_directory =
+        permission_context_->GetLastPickedDirectory(context.origin);
+    if (default_directory.empty()) {
+      default_directory = permission_context_->GetDefaultDirectory();
+    }
+  }
+
+  // TODO(https://crbug.com/1019408): Append suggested filename to the default
+  // directory.
   FileSystemChooser::Options options(type, std::move(accepts),
-                                     include_accepts_all);
+                                     include_accepts_all, default_directory);
+
+  if (auto_file_picker_result_for_test_) {
+    DidChooseEntries(context, options, std::move(callback),
+                     native_file_system_error::Ok(),
+                     {*auto_file_picker_result_for_test_});
+    return;
+  }
+
   ShowFilePickerOnUIThread(
       context.origin, context.frame_id, options,
       base::BindOnce(&NativeFileSystemManagerImpl::DidChooseEntries,
@@ -888,6 +908,16 @@ void NativeFileSystemManagerImpl::DidVerifySensitiveDirectoryAccess(
     return;
   }
 
+  if (permission_context_ && !entries.empty()) {
+    auto picked_directory =
+        options.type() ==
+                blink::mojom::ChooseFileSystemEntryType::kOpenDirectory
+            ? entries.front().path
+            : entries.front().path.DirName();
+    permission_context_->SetLastPickedDirectory(binding_context.origin,
+                                                picked_directory);
+  }
+
   if (options.type() ==
       blink::mojom::ChooseFileSystemEntryType::kOpenDirectory) {
     DCHECK_EQ(entries.size(), 1u);

content/browser/file_system_access/native_file_system_manager_impl.h

@@ -205,8 +205,13 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
     permission_context_ = permission_context;
   }
 
-  // Remove |token| from |transfer_tokens_|. It is an error to try to remove a
-  // token that doesn't exist.
+  void SetFilePickerResultForTesting(
+      base::Optional<FileSystemChooser::ResultEntry> result_entry) {
+    auto_file_picker_result_for_test_ = result_entry;
+  }
+
+  // Remove |token| from |transfer_tokens_|. It is an error to try to remove
+  // a token that doesn't exist.
   void RemoveToken(const base::UnguessableToken& token);
 
   // Remove `token` from `drag_drop_tokens_`. It is an error to try to remove a
@@ -354,6 +359,9 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
            std::unique_ptr<NativeFileSystemDragDropTokenImpl>>
       drag_drop_tokens_;
 
+  base::Optional<FileSystemChooser::ResultEntry>
+      auto_file_picker_result_for_test_;
+
   base::WeakPtrFactory<NativeFileSystemManagerImpl> weak_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(NativeFileSystemManagerImpl);
 };

content/public/browser/native_file_system_permission_context.h

@@ -116,6 +116,14 @@ class NativeFileSystemPermissionContext {
   // shown if there is no need to ask for it.
   virtual bool CanObtainWritePermission(const url::Origin& origin) = 0;
 
+  // Store the directory recently chosen using a file picker.
+  virtual void SetLastPickedDirectory(const url::Origin& origin,
+                                      const base::FilePath& path) = 0;
+  // Returns the directory recently chosen using a file picker.
+  virtual base::FilePath GetLastPickedDirectory(const url::Origin& origin) = 0;
+  // Return the default directory used by the File System Access API.
+  virtual base::FilePath GetDefaultDirectory() = 0;
+
  protected:
   virtual ~NativeFileSystemPermissionContext() = default;
 };