crostini: Rename ansible file selector to be a generic file selector
The file selector in the create container dialog will be used to select
both Ansible playbooks and Crostini backup files in a later CL. This CL
prepares for the later extension by renaming the existing selector to be
more generic. For details on the overall UI design, see
go/crostini-from-file.
Bug: b:234661593
Test: unit tests, manually tested loading a file in the create dialog
Change-Id: I78981d1d68f4985447d06ccaa81dbfb7277a6ee2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3835223
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Nic Hollingum <hollingum@google.com>
Commit-Queue: Sophia Lin <sophialin@google.com>
Cr-Commit-Position: refs/heads/main@{#1036934}
diff --git a/chrome/app/os_settings_strings.grdp b/chrome/app/os_settings_strings.grdp
index db192b3..8fb3178 100644
--- a/chrome/app/os_settings_strings.grdp
+++ b/chrome/app/os_settings_strings.grdp
@@ -1850,7 +1850,7 @@
<message name="IDS_SETTINGS_CROSTINI_PRECONFIGURED_CONTAINERS_ATTACH_PLAYBOOK_BUTTON_LABEL" desc="Label for button to attach an Ansible Playbook to preconfigure a Linux container.">
Select playbook
</message>
- <message name="IDS_SETTINGS_CROSTINI_ANSIBLE_PLAYBOOK_SELECT_DIALOG_TITLE" desc="The text used as the title for Ansible Playbook selection">
+ <message name="IDS_SETTINGS_CROSTINI_FILE_SELECTOR_DIALOG_TITLE" desc="The text used as the title for selecting a file for Crostini container creation">
Select an Ansible Playbook to add
</message>
<message name="IDS_SETTINGS_CROSTINI_DISK_RESIZE_SHOW_BUTTON" desc="The label of the button/link the user clicks to open the disk resizing dialogue." meaning="The label of the button/link the user clicks to open the disk resizing dialogue.">
diff --git a/chrome/app/os_settings_strings_grdp/IDS_SETTINGS_CROSTINI_ANSIBLE_PLAYBOOK_SELECT_DIALOG_TITLE.png.sha1 b/chrome/app/os_settings_strings_grdp/IDS_SETTINGS_CROSTINI_ANSIBLE_PLAYBOOK_SELECT_DIALOG_TITLE.png.sha1
deleted file mode 100644
index 7e0f1f2..0000000
--- a/chrome/app/os_settings_strings_grdp/IDS_SETTINGS_CROSTINI_ANSIBLE_PLAYBOOK_SELECT_DIALOG_TITLE.png.sha1
+++ /dev/null
@@ -1 +0,0 @@
-89f094df5a0b0aa30d170cc976b85ae0e26c5bad
\ No newline at end of file
diff --git a/chrome/app/os_settings_strings_grdp/IDS_SETTINGS_CROSTINI_FILE_SELECTOR_DIALOG_TITLE.png.sha1 b/chrome/app/os_settings_strings_grdp/IDS_SETTINGS_CROSTINI_FILE_SELECTOR_DIALOG_TITLE.png.sha1
new file mode 100644
index 0000000..a460627
--- /dev/null
+++ b/chrome/app/os_settings_strings_grdp/IDS_SETTINGS_CROSTINI_FILE_SELECTOR_DIALOG_TITLE.png.sha1
@@ -0,0 +1 @@
+a46bcebc79414f0330f60c36a0c71788259b1365
\ No newline at end of file
diff --git a/chrome/browser/ash/BUILD.gn b/chrome/browser/ash/BUILD.gn
index 5a1dbe89..6f14fe9 100644
--- a/chrome/browser/ash/BUILD.gn
+++ b/chrome/browser/ash/BUILD.gn
@@ -769,8 +769,6 @@
"chrome_browser_main_parts_ash.h",
"concierge_helper_service.cc",
"concierge_helper_service.h",
- "crostini/ansible/ansible_file_selector.cc",
- "crostini/ansible/ansible_file_selector.h",
"crostini/ansible/ansible_management_service.cc",
"crostini/ansible/ansible_management_service.h",
"crostini/ansible/ansible_management_service_factory.cc",
@@ -787,6 +785,8 @@
"crostini/crostini_export_import_status_tracker.h",
"crostini/crostini_features.cc",
"crostini/crostini_features.h",
+ "crostini/crostini_file_selector.cc",
+ "crostini/crostini_file_selector.h",
"crostini/crostini_force_close_watcher.cc",
"crostini/crostini_force_close_watcher.h",
"crostini/crostini_installer.cc",
diff --git a/chrome/browser/ash/crostini/ansible/ansible_file_selector.cc b/chrome/browser/ash/crostini/crostini_file_selector.cc
similarity index 67%
rename from chrome/browser/ash/crostini/ansible/ansible_file_selector.cc
rename to chrome/browser/ash/crostini/crostini_file_selector.cc
index a539f0c..4ae59419 100644
--- a/chrome/browser/ash/crostini/ansible/ansible_file_selector.cc
+++ b/chrome/browser/ash/crostini/crostini_file_selector.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/browser/ash/crostini/ansible/ansible_file_selector.h"
+#include "chrome/browser/ash/crostini/crostini_file_selector.h"
#include "base/path_service.h"
#include "chrome/browser/ui/browser_finder.h"
@@ -13,25 +13,27 @@
#include "ui/base/l10n/l10n_util.h"
namespace {
-ui::SelectFileDialog::FileTypeInfo GetAnsibleFileTypeInfo() {
+ui::SelectFileDialog::FileTypeInfo GetFileTypeInfo() {
ui::SelectFileDialog::FileTypeInfo file_type_info;
file_type_info.extensions.resize(1);
- file_type_info.extensions[0].push_back(FILE_PATH_LITERAL("yaml"));
+
+ // Allow Ansible playbooks (yaml) only.
+ file_type_info.extensions = {{"yaml"}};
return file_type_info;
}
} // namespace
namespace crostini {
-AnsibleFileSelector::AnsibleFileSelector(content::WebUI* web_ui)
+CrostiniFileSelector::CrostiniFileSelector(content::WebUI* web_ui)
: web_ui_(web_ui) {}
-AnsibleFileSelector::~AnsibleFileSelector() {
+CrostiniFileSelector::~CrostiniFileSelector() {
if (select_file_dialog_.get())
select_file_dialog_->ListenerDestroyed();
}
-void AnsibleFileSelector::SelectFile(
+void CrostiniFileSelector::SelectFile(
base::OnceCallback<void(const base::FilePath&)> selected_callback,
base::OnceCallback<void(void)> cancelled_callback) {
selected_callback_ = std::move(selected_callback);
@@ -44,32 +46,35 @@
// uploaded Ansible Playbook if the user has already "uploaded" a playbook
// before.
base::FilePath downloads_path;
- if (!base::PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &downloads_path))
+ if (!base::PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &downloads_path)) {
+ LOG(ERROR) << "Default Downloads path does not exist, cannot open file "
+ "selector for create container";
return;
+ }
- ui::SelectFileDialog::FileTypeInfo file_type_info(GetAnsibleFileTypeInfo());
+ ui::SelectFileDialog::FileTypeInfo file_type_info(GetFileTypeInfo());
select_file_dialog_->SelectFile(
ui::SelectFileDialog::SELECT_OPEN_FILE,
l10n_util::GetStringUTF16(
- IDS_SETTINGS_CROSTINI_ANSIBLE_PLAYBOOK_SELECT_DIALOG_TITLE),
+ IDS_SETTINGS_CROSTINI_FILE_SELECTOR_DIALOG_TITLE),
downloads_path, &file_type_info, 0, FILE_PATH_LITERAL(""),
GetBrowserWindow(), NULL);
}
-gfx::NativeWindow AnsibleFileSelector::GetBrowserWindow() {
+gfx::NativeWindow CrostiniFileSelector::GetBrowserWindow() {
Browser* browser =
chrome::FindBrowserWithWebContents(web_ui_->GetWebContents());
return browser ? browser->window()->GetNativeWindow()
: gfx::kNullNativeWindow;
}
-void AnsibleFileSelector::FileSelected(const base::FilePath& path,
- int index,
- void* params) {
+void CrostiniFileSelector::FileSelected(const base::FilePath& path,
+ int index,
+ void* params) {
std::move(selected_callback_).Run(path);
}
-void AnsibleFileSelector::FileSelectionCanceled(void* params) {
+void CrostiniFileSelector::FileSelectionCanceled(void* params) {
if (cancelled_callback_)
std::move(cancelled_callback_).Run();
}
diff --git a/chrome/browser/ash/crostini/ansible/ansible_file_selector.h b/chrome/browser/ash/crostini/crostini_file_selector.h
similarity index 65%
rename from chrome/browser/ash/crostini/ansible/ansible_file_selector.h
rename to chrome/browser/ash/crostini/crostini_file_selector.h
index c4d2aa547..4c80f88 100644
--- a/chrome/browser/ash/crostini/ansible/ansible_file_selector.h
+++ b/chrome/browser/ash/crostini/crostini_file_selector.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef CHROME_BROWSER_ASH_CROSTINI_ANSIBLE_ANSIBLE_FILE_SELECTOR_H_
-#define CHROME_BROWSER_ASH_CROSTINI_ANSIBLE_ANSIBLE_FILE_SELECTOR_H_
+#ifndef CHROME_BROWSER_ASH_CROSTINI_CROSTINI_FILE_SELECTOR_H_
+#define CHROME_BROWSER_ASH_CROSTINI_CROSTINI_FILE_SELECTOR_H_
#include "base/callback.h"
#include "base/memory/raw_ptr.h"
@@ -13,15 +13,15 @@
namespace crostini {
-// Helper class to open a file dialog to select an Ansible Playbook for
-// preconfigured containers.
-class AnsibleFileSelector : public ui::SelectFileDialog::Listener {
+// Helper class to open a file dialog to select a file from which the
+// container will be created.
+class CrostiniFileSelector : public ui::SelectFileDialog::Listener {
public:
- explicit AnsibleFileSelector(content::WebUI* web_ui);
- AnsibleFileSelector(const AnsibleFileSelector&) = delete;
- AnsibleFileSelector& operator=(const AnsibleFileSelector&) = delete;
+ explicit CrostiniFileSelector(content::WebUI* web_ui);
+ CrostiniFileSelector(const CrostiniFileSelector&) = delete;
+ CrostiniFileSelector& operator=(const CrostiniFileSelector&) = delete;
- ~AnsibleFileSelector() override;
+ ~CrostiniFileSelector() override;
// Opens a file selection dialog to choose an Ansible Playbook.
virtual void SelectFile(
@@ -46,4 +46,4 @@
} // namespace crostini
-#endif // CHROME_BROWSER_ASH_CROSTINI_ANSIBLE_ANSIBLE_FILE_SELECTOR_H_
+#endif // CHROME_BROWSER_ASH_CROSTINI_CROSTINI_FILE_SELECTOR_H_
diff --git a/chrome/browser/resources/settings/chromeos/crostini_page/crostini_browser_proxy.js b/chrome/browser/resources/settings/chromeos/crostini_page/crostini_browser_proxy.js
index 55deae52..f8401c4a6 100644
--- a/chrome/browser/resources/settings/chromeos/crostini_page/crostini_browser_proxy.js
+++ b/chrome/browser/resources/settings/chromeos/crostini_page/crostini_browser_proxy.js
@@ -267,10 +267,9 @@
* Opens file selector dialog to allow user to select an Ansible playbook
* to preconfigure their container.
*
- * @return {!Promise<string>} Returns a filepath to the selected Ansible
- * Playbook
+ * @return {!Promise<string>} Returns a filepath to the selected file.
*/
- applyAnsiblePlaybook() {}
+ openContainerFileSelector() {}
}
/** @type {?CrostiniBrowserProxy} */
@@ -452,7 +451,7 @@
}
/** @override */
- applyAnsiblePlaybook() {
- return sendWithPromise('applyAnsiblePlaybook');
+ openContainerFileSelector() {
+ return sendWithPromise('openContainerFileSelector');
}
}
diff --git a/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.html b/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.html
index 75bb6c45..0167ef4 100644
--- a/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.html
+++ b/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.html
@@ -101,7 +101,7 @@
<cr-button id="cancel" class="cancel-button" on-click="onCancelTap_">
$i18n{cancel}
</cr-button>
- <cr-button id="create" class="action-button" on-click="onCreateTap_"
+ <cr-button id="create" class="action-button" on-click="onCreateTap_"
disabled="[[disableCreateButton_]]">
$i18n{crostiniExtraContainersCreate}
</cr-button>
diff --git a/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.js b/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.js
index f340c94..6a7cc1b 100644
--- a/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.js
+++ b/chrome/browser/resources/settings/chromeos/crostini_page/crostini_extra_containers_create_dialog.js
@@ -190,7 +190,7 @@
/** @private */
async onAnsiblePlaybookUploadClick_() {
this.$.preconfiguredContainersInput.value =
- await this.browserProxy_.applyAnsiblePlaybook();
+ await this.browserProxy_.openContainerFileSelector();
}
/** @private */
diff --git a/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc b/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
index c2dc469..971fee3 100644
--- a/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
+++ b/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
@@ -177,8 +177,8 @@
base::BindRepeating(&CrostiniHandler::HandleStopContainer,
handler_weak_ptr_factory_.GetWeakPtr()));
web_ui()->RegisterMessageCallback(
- "applyAnsiblePlaybook",
- base::BindRepeating(&CrostiniHandler::HandleApplyAnsiblePlaybook,
+ "openContainerFileSelector",
+ base::BindRepeating(&CrostiniHandler::HandleOpenContainerFileSelector,
handler_weak_ptr_factory_.GetWeakPtr()));
}
}
@@ -812,20 +812,19 @@
}
}
-void CrostiniHandler::HandleApplyAnsiblePlaybook(
+void CrostiniHandler::HandleOpenContainerFileSelector(
const base::Value::List& args) {
CHECK_EQ(1U, args.size());
const std::string& callback_id = args[0].GetString();
- ansible_file_selector_ =
- std::make_unique<crostini::AnsibleFileSelector>(web_ui());
- ansible_file_selector_->SelectFile(
- base::BindOnce(&CrostiniHandler::OnAnsiblePlaybookSelected,
+ file_selector_ = std::make_unique<crostini::CrostiniFileSelector>(web_ui());
+ file_selector_->SelectFile(
+ base::BindOnce(&CrostiniHandler::OnContainerFileSelected,
handler_weak_ptr_factory_.GetWeakPtr(), callback_id),
base::DoNothing());
}
-void CrostiniHandler::OnAnsiblePlaybookSelected(const std::string& callback_id,
- const base::FilePath& path) {
+void CrostiniHandler::OnContainerFileSelected(const std::string& callback_id,
+ const base::FilePath& path) {
base::Value filePath(path.value());
ResolveJavascriptCallback(base::Value(callback_id), filePath);
}
diff --git a/chrome/browser/ui/webui/settings/chromeos/crostini_handler.h b/chrome/browser/ui/webui/settings/chromeos/crostini_handler.h
index 101c11fb..4658b61 100644
--- a/chrome/browser/ui/webui/settings/chromeos/crostini_handler.h
+++ b/chrome/browser/ui/webui/settings/chromeos/crostini_handler.h
@@ -8,8 +8,8 @@
#include <vector>
#include "base/memory/weak_ptr.h"
-#include "chrome/browser/ash/crostini/ansible/ansible_file_selector.h"
#include "chrome/browser/ash/crostini/crostini_export_import.h"
+#include "chrome/browser/ash/crostini/crostini_file_selector.h"
#include "chrome/browser/ash/crostini/crostini_manager.h"
#include "chrome/browser/ash/crostini/crostini_port_forwarder.h"
#include "chrome/browser/ash/guest_os/guest_id.h"
@@ -145,16 +145,16 @@
// Handle a request to stop a running lxd container
void HandleStopContainer(const base::Value::List& args);
- // Handle a request to upload an Ansible Playbook
- void HandleApplyAnsiblePlaybook(const base::Value::List& args);
- // Callback for AnsibleFileSelector
- void OnAnsiblePlaybookSelected(const std::string& callback_id,
- const base::FilePath& path);
+ // Handle a request to open a file selector
+ void HandleOpenContainerFileSelector(const base::Value::List& args);
+ // Callback for CrostiniFileSelector
+ void OnContainerFileSelected(const std::string& callback_id,
+ const base::FilePath& path);
Profile* profile_;
base::CallbackListSubscription adb_sideloading_device_policy_subscription_;
PrefChangeRegistrar pref_change_registrar_;
- std::unique_ptr<crostini::AnsibleFileSelector> ansible_file_selector_;
+ std::unique_ptr<crostini::CrostiniFileSelector> file_selector_;
// |handler_weak_ptr_factory_| is used for callbacks handling messages from
// the WebUI page, and certain observers. These callbacks usually have the