Add SelectFileDialogExtension::SelectFileWithFileManagerParams
This allows us to pass show_android_picker_apps (a ChromeOS-specific
param) to SelectFileDialogExtension (SelectFileDialog impl for ChromeOS)
without modifying common SelectFileDialog interface.
Related discussion:
https://docs.google.com/a/google.com/document/d/1wy9b_VEi4mj1BU3LcI7ebMdDnlhwYnzgPqSt-hn7xow/edit?disco=AAAAC1VaFP4
BUG=b:130204519
TEST=unit_tests --gtest_filter="FileManagerUrlUtilTest.*
Change-Id: Ie39ced3c092ecaeee644b41f79fd89f0751fa734
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1564355
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Satoshi Niwa <niwa@chromium.org>
Auto-Submit: Satoshi Niwa <niwa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655152}
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.cc b/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.cc
index 02dc656..59778376 100644
--- a/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.cc
+++ b/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.cc
@@ -171,7 +171,12 @@
BuildFileTypeInfo(request, &file_type_info);
base::FilePath default_path = GetInitialFilePath(request);
- dialog_holder_->SelectFile(dialog_type, default_path, &file_type_info);
+ // Android picker apps should be shown in GET_CONTENT mode.
+ bool show_android_picker_apps =
+ request->action_type == mojom::SelectFilesActionType::GET_CONTENT;
+
+ dialog_holder_->SelectFile(dialog_type, default_path, &file_type_info,
+ show_android_picker_apps);
}
void ArcSelectFilesHandler::FileSelected(const base::FilePath& path,
@@ -274,14 +279,15 @@
void SelectFileDialogHolder::SelectFile(
ui::SelectFileDialog::Type type,
const base::FilePath& default_path,
- const ui::SelectFileDialog::FileTypeInfo* file_types) {
- select_file_dialog_->SelectFile(
+ const ui::SelectFileDialog::FileTypeInfo* file_types,
+ bool show_android_picker_apps) {
+ select_file_dialog_->SelectFileWithFileManagerParams(
type,
/*title=*/base::string16(), default_path, file_types,
/*file_type_index=*/0,
/*default_extension=*/base::FilePath::StringType(),
/*owning_window=*/nullptr,
- /*params=*/nullptr);
+ /*params=*/nullptr, show_android_picker_apps);
}
void SelectFileDialogHolder::ExecuteJavaScript(
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h b/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h
index 513daad..236baae 100644
--- a/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h
+++ b/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h
@@ -82,7 +82,8 @@
virtual void SelectFile(ui::SelectFileDialog::Type type,
const base::FilePath& default_path,
- const ui::SelectFileDialog::FileTypeInfo* file_types);
+ const ui::SelectFileDialog::FileTypeInfo* file_types,
+ bool show_android_picker_apps);
virtual void ExecuteJavaScript(
const std::string& script,
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler_unittest.cc b/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler_unittest.cc
index c0062b5..16975f2 100644
--- a/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler_unittest.cc
+++ b/chrome/browser/chromeos/arc/fileapi/arc_select_files_handler_unittest.cc
@@ -69,10 +69,11 @@
explicit MockSelectFileDialogHolder(ui::SelectFileDialog::Listener* listener)
: SelectFileDialogHolder(listener) {}
~MockSelectFileDialogHolder() override = default;
- MOCK_METHOD3(SelectFile,
+ MOCK_METHOD4(SelectFile,
void(ui::SelectFileDialog::Type type,
const base::FilePath& default_path,
- const ui::SelectFileDialog::FileTypeInfo* file_types));
+ const ui::SelectFileDialog::FileTypeInfo* file_types,
+ bool show_android_picker_apps));
MOCK_METHOD2(ExecuteJavaScript,
void(const std::string&, JavaScriptResultCallback));
};
@@ -110,12 +111,15 @@
void CallSelectFilesAndCheckDialogType(
SelectFilesActionType request_action_type,
bool request_allow_multiple,
- SelectFileDialog::Type expected_dialog_type) {
+ SelectFileDialog::Type expected_dialog_type,
+ bool expected_show_android_picker_apps) {
SelectFilesRequestPtr request = SelectFilesRequest::New();
request->action_type = request_action_type;
request->allow_multiple = request_allow_multiple;
- EXPECT_CALL(*mock_dialog_holder_, SelectFile(expected_dialog_type, _, _))
+ EXPECT_CALL(*mock_dialog_holder_,
+ SelectFile(expected_dialog_type, _, _,
+ expected_show_android_picker_apps))
.Times(1);
SelectFilesCallback callback;
@@ -150,18 +154,21 @@
TEST_F(ArcSelectFilesHandlerTest, SelectFiles_DialogType) {
CallSelectFilesAndCheckDialogType(SelectFilesActionType::GET_CONTENT, false,
- SelectFileDialog::SELECT_OPEN_FILE);
+ SelectFileDialog::SELECT_OPEN_FILE, true);
CallSelectFilesAndCheckDialogType(SelectFilesActionType::GET_CONTENT, true,
- SelectFileDialog::SELECT_OPEN_MULTI_FILE);
+ SelectFileDialog::SELECT_OPEN_MULTI_FILE,
+ true);
CallSelectFilesAndCheckDialogType(SelectFilesActionType::OPEN_DOCUMENT, false,
- SelectFileDialog::SELECT_OPEN_FILE);
+ SelectFileDialog::SELECT_OPEN_FILE, false);
CallSelectFilesAndCheckDialogType(SelectFilesActionType::OPEN_DOCUMENT, true,
- SelectFileDialog::SELECT_OPEN_MULTI_FILE);
- CallSelectFilesAndCheckDialogType(SelectFilesActionType::OPEN_DOCUMENT_TREE,
- false,
- SelectFileDialog::SELECT_EXISTING_FOLDER);
+ SelectFileDialog::SELECT_OPEN_MULTI_FILE,
+ false);
+ CallSelectFilesAndCheckDialogType(
+ SelectFilesActionType::OPEN_DOCUMENT_TREE, false,
+ SelectFileDialog::SELECT_EXISTING_FOLDER, false);
CallSelectFilesAndCheckDialogType(SelectFilesActionType::CREATE_DOCUMENT,
- true, SelectFileDialog::SELECT_SAVEAS_FILE);
+ true, SelectFileDialog::SELECT_SAVEAS_FILE,
+ false);
}
TEST_F(ArcSelectFilesHandlerTest, SelectFiles_FileTypeInfo) {
@@ -179,8 +186,9 @@
EXPECT_CALL(
*mock_dialog_holder_,
- SelectFile(
- _, _, testing::Pointee(FileTypeInfoMatcher(expected_file_type_info))))
+ SelectFile(_, _,
+ testing::Pointee(FileTypeInfoMatcher(expected_file_type_info)),
+ _))
.Times(1);
base::MockCallback<SelectFilesCallback> callback;
@@ -199,7 +207,7 @@
"/special/arc-documents-provider/testing.provider/doc:root");
EXPECT_CALL(*mock_dialog_holder_,
- SelectFile(_, FilePathMatcher(expected_file_path), _))
+ SelectFile(_, FilePathMatcher(expected_file_path), _, _))
.Times(1);
base::MockCallback<SelectFilesCallback> callback;
diff --git a/chrome/browser/chromeos/file_manager/url_util.cc b/chrome/browser/chromeos/file_manager/url_util.cc
index 63ccb8b..37e65f5c 100644
--- a/chrome/browser/chromeos/file_manager/url_util.cc
+++ b/chrome/browser/chromeos/file_manager/url_util.cc
@@ -78,7 +78,8 @@
const std::string& target_name,
const ui::SelectFileDialog::FileTypeInfo* file_types,
int file_type_index,
- const base::FilePath::StringType& default_extension) {
+ const base::FilePath::StringType& default_extension,
+ bool show_android_picker_apps) {
base::DictionaryValue arg_value;
arg_value.SetString("type", GetDialogTypeAsString(type));
arg_value.SetString("title", title);
@@ -86,6 +87,7 @@
arg_value.SetString("selectionURL", selection_url.spec());
arg_value.SetString("targetName", target_name);
arg_value.SetString("defaultExtension", default_extension);
+ arg_value.SetBoolean("showAndroidPickerApps", show_android_picker_apps);
if (file_types) {
auto types_list = std::make_unique<base::ListValue>();
diff --git a/chrome/browser/chromeos/file_manager/url_util.h b/chrome/browser/chromeos/file_manager/url_util.h
index f531582..8666925 100644
--- a/chrome/browser/chromeos/file_manager/url_util.h
+++ b/chrome/browser/chromeos/file_manager/url_util.h
@@ -29,7 +29,8 @@
const std::string& target_name,
const ui::SelectFileDialog::FileTypeInfo* file_types,
int file_type_index,
- const base::FilePath::StringType& default_extension);
+ const base::FilePath::StringType& default_extension,
+ bool show_android_picker_apps);
} // namespace util
} // namespace file_manager
diff --git a/chrome/browser/chromeos/file_manager/url_util_unittest.cc b/chrome/browser/chromeos/file_manager/url_util_unittest.cc
index 5a0463e..ed054f9e 100644
--- a/chrome/browser/chromeos/file_manager/url_util_unittest.cc
+++ b/chrome/browser/chromeos/file_manager/url_util_unittest.cc
@@ -9,6 +9,7 @@
#include "base/files/file_path.h"
#include "base/json/json_reader.h"
#include "base/json/json_writer.h"
+#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "chromeos/constants/chromeos_features.h"
@@ -38,17 +39,6 @@
return pretty_json;
}
-base::Value MakeSimpleDictionary(
- std::vector<std::pair<std::string, std::string>> entries) {
- std::vector<std::pair<std::string, std::unique_ptr<base::Value>>> storage;
- storage.reserve(entries.size());
- for (auto& entry : entries) {
- storage.emplace_back(std::move(entry.first), std::make_unique<base::Value>(
- std::move(entry.second)));
- }
- return base::Value(std::move(storage));
-}
-
TEST(FileManagerUrlUtilTest, GetFileManagerMainPageUrl) {
EXPECT_EQ("chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/main.html",
GetFileManagerMainPageUrl().spec());
@@ -61,7 +51,9 @@
GURL("filesystem:chrome-extension://abc/Downloads/foo.txt"), "foo.txt",
nullptr, // No file types
0, // Hence no file type index.
- FILE_PATH_LITERAL("txt"));
+ FILE_PATH_LITERAL("txt"),
+ false // show_android_picker_apps
+ );
EXPECT_EQ(extensions::kExtensionScheme, url.scheme());
EXPECT_EQ("hhaomjibdihmijegdhdafkllkbggdgoj", url.host());
EXPECT_EQ("/main.html", url.path());
@@ -69,21 +61,25 @@
EXPECT_TRUE(url.query().find("+") == std::string::npos);
EXPECT_TRUE(url.query().find("%20") != std::string::npos);
// With DriveFS, Drive is always allowed where native paths are.
- EXPECT_EQ(MakeSimpleDictionary({
- {"allowedPaths",
- base::FeatureList::IsEnabled(chromeos::features::kDriveFs)
- ? "nativeOrDrivePath"
- : "nativePath"},
- {"currentDirectoryURL",
- "filesystem:chrome-extension://abc/Downloads/"},
- {"defaultExtension", "txt"},
- {"selectionURL",
- "filesystem:chrome-extension://abc/Downloads/foo.txt"},
- {"targetName", "foo.txt"},
- {"title", "some title"},
- {"type", "open-file"},
- }),
- ParseJsonQueryString(url.query()));
+ std::string allowed_paths =
+ base::FeatureList::IsEnabled(chromeos::features::kDriveFs)
+ ? "nativeOrDrivePath"
+ : "nativePath";
+ EXPECT_EQ(base::StringPrintf(
+ "{\n"
+ " \"allowedPaths\": \"%s\",\n"
+ " \"currentDirectoryURL\": "
+ "\"filesystem:chrome-extension://abc/Downloads/\",\n"
+ " \"defaultExtension\": \"txt\",\n"
+ " \"selectionURL\": "
+ "\"filesystem:chrome-extension://abc/Downloads/foo.txt\",\n"
+ " \"showAndroidPickerApps\": false,\n"
+ " \"targetName\": \"foo.txt\",\n"
+ " \"title\": \"some title\",\n"
+ " \"type\": \"open-file\"\n"
+ "}\n",
+ allowed_paths.c_str()),
+ PrettyPrintEscapedJson(url.query()));
}
TEST(FileManagerUrlUtilTest,
@@ -105,14 +101,14 @@
file_types.allowed_paths = ui::SelectFileDialog::FileTypeInfo::ANY_PATH;
const GURL url = GetFileManagerMainPageUrlWithParams(
- ui::SelectFileDialog::SELECT_OPEN_FILE,
- base::UTF8ToUTF16("some title"),
+ ui::SelectFileDialog::SELECT_OPEN_FILE, base::UTF8ToUTF16("some title"),
GURL("filesystem:chrome-extension://abc/Downloads/"),
- GURL("filesystem:chrome-extension://abc/Downloads/foo.txt"),
- "foo.txt",
+ GURL("filesystem:chrome-extension://abc/Downloads/foo.txt"), "foo.txt",
&file_types,
1, // The file type index is 1-based.
- FILE_PATH_LITERAL("txt"));
+ FILE_PATH_LITERAL("txt"),
+ true // show_android_picker_apps
+ );
EXPECT_EQ(extensions::kExtensionScheme, url.scheme());
EXPECT_EQ("hhaomjibdihmijegdhdafkllkbggdgoj", url.host());
EXPECT_EQ("/main.html", url.path());
@@ -129,6 +125,7 @@
" \"includeAllFiles\": false,\n"
" \"selectionURL\": "
"\"filesystem:chrome-extension://abc/Downloads/foo.txt\",\n"
+ " \"showAndroidPickerApps\": true,\n"
" \"targetName\": \"foo.txt\",\n"
" \"title\": \"some title\",\n"
" \"type\": \"open-file\",\n"
diff --git a/chrome/browser/ui/views/select_file_dialog_extension.cc b/chrome/browser/ui/views/select_file_dialog_extension.cc
index 8c93be1..509ecbc 100644
--- a/chrome/browser/ui/views/select_file_dialog_extension.cc
+++ b/chrome/browser/ui/views/select_file_dialog_extension.cc
@@ -293,46 +293,7 @@
return NULL;
}
-bool SelectFileDialogExtension::IsResizeable() const {
- DCHECK(extension_dialog_.get());
- return extension_dialog_->CanResize();
-}
-
-void SelectFileDialogExtension::NotifyListener() {
- if (!listener_)
- return;
- switch (selection_type_) {
- case CANCEL:
- listener_->FileSelectionCanceled(params_);
- break;
- case SINGLE_FILE:
- listener_->FileSelectedWithExtraInfo(selection_files_[0],
- selection_index_,
- params_);
- break;
- case MULTIPLE_FILES:
- listener_->MultiFilesSelectedWithExtraInfo(selection_files_, params_);
- break;
- default:
- NOTREACHED();
- break;
- }
-}
-
-void SelectFileDialogExtension::AddPending(RoutingID routing_id) {
- PendingDialog::GetInstance()->Add(routing_id, this);
-}
-
-// static
-bool SelectFileDialogExtension::PendingExists(RoutingID routing_id) {
- return PendingDialog::GetInstance()->Find(routing_id).get() != NULL;
-}
-
-bool SelectFileDialogExtension::HasMultipleFileTypeChoicesImpl() {
- return has_multiple_file_type_choices_;
-}
-
-void SelectFileDialogExtension::SelectFileImpl(
+void SelectFileDialogExtension::SelectFileWithFileManagerParams(
Type type,
const base::string16& title,
const base::FilePath& default_path,
@@ -340,7 +301,8 @@
int file_type_index,
const base::FilePath::StringType& default_extension,
gfx::NativeWindow owner_window,
- void* params) {
+ void* params,
+ bool show_android_picker_apps) {
if (owner_window_) {
LOG(ERROR) << "File dialog already in use!";
return;
@@ -422,14 +384,9 @@
GURL file_manager_url =
file_manager::util::GetFileManagerMainPageUrlWithParams(
- type,
- title,
- current_directory_url,
- selection_url,
- default_path.BaseName().value(),
- file_types,
- file_type_index,
- default_extension);
+ type, title, current_directory_url, selection_url,
+ default_path.BaseName().value(), file_types, file_type_index,
+ default_extension, show_android_picker_apps);
ExtensionDialog* dialog = ExtensionDialog::Show(
file_manager_url,
@@ -451,3 +408,55 @@
routing_id_ = routing_id;
owner_window_ = owner_window;
}
+
+void SelectFileDialogExtension::SelectFileImpl(
+ Type type,
+ const base::string16& title,
+ const base::FilePath& default_path,
+ const FileTypeInfo* file_types,
+ int file_type_index,
+ const base::FilePath::StringType& default_extension,
+ gfx::NativeWindow owner_window,
+ void* params) {
+ SelectFileWithFileManagerParams(
+ type, title, default_path, file_types, file_type_index, default_extension,
+ owner_window, params, false /* show_android_picker_apps */);
+}
+
+bool SelectFileDialogExtension::IsResizeable() const {
+ DCHECK(extension_dialog_.get());
+ return extension_dialog_->CanResize();
+}
+
+void SelectFileDialogExtension::NotifyListener() {
+ if (!listener_)
+ return;
+ switch (selection_type_) {
+ case CANCEL:
+ listener_->FileSelectionCanceled(params_);
+ break;
+ case SINGLE_FILE:
+ listener_->FileSelectedWithExtraInfo(selection_files_[0],
+ selection_index_, params_);
+ break;
+ case MULTIPLE_FILES:
+ listener_->MultiFilesSelectedWithExtraInfo(selection_files_, params_);
+ break;
+ default:
+ NOTREACHED();
+ break;
+ }
+}
+
+void SelectFileDialogExtension::AddPending(RoutingID routing_id) {
+ PendingDialog::GetInstance()->Add(routing_id, this);
+}
+
+// static
+bool SelectFileDialogExtension::PendingExists(RoutingID routing_id) {
+ return PendingDialog::GetInstance()->Find(routing_id).get() != NULL;
+}
+
+bool SelectFileDialogExtension::HasMultipleFileTypeChoicesImpl() {
+ return has_multiple_file_type_choices_;
+}
diff --git a/chrome/browser/ui/views/select_file_dialog_extension.h b/chrome/browser/ui/views/select_file_dialog_extension.h
index 3145aff..af9a221 100644
--- a/chrome/browser/ui/views/select_file_dialog_extension.h
+++ b/chrome/browser/ui/views/select_file_dialog_extension.h
@@ -64,6 +64,18 @@
// For testing, so we can inject JavaScript into the contained view.
content::RenderViewHost* GetRenderViewHost();
+ // Call SelectFile with params specific to Chrome OS file manager.
+ void SelectFileWithFileManagerParams(
+ Type type,
+ const base::string16& title,
+ const base::FilePath& default_path,
+ const FileTypeInfo* file_types,
+ int file_type_index,
+ const base::FilePath::StringType& default_extension,
+ gfx::NativeWindow owning_window,
+ void* params,
+ bool show_android_picker_apps);
+
protected:
// SelectFileDialog implementation.
void SelectFileImpl(Type type,
