Commit d27cbd66 authored by Jason Lin's avatar Jason Lin Committed by Commit Bot

Prevent Closing Crostini Installer Dialog without Web Page Consent

Currently, the WebUI installer dialog can be closed directly in the overview
mode, and the web page will not notice it. We want to prevent this from
happening for a few reasons:

* The web page should have an opportunity to clean up before closing (e.g. call
  cancel()).
* In some installation states, we want to keep the dialog open.

This CL blocks closing requests to the dialog unless the web page proactively
request it.

bug: 929571
Test: Enable CrostiniWebUIInstaller flag, try closing installer dialog in different ways
Change-Id: I8f5643c42b807d2a3fcebdd5c56e7648a46ab86f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872084
Commit-Queue: Jason Lin <lxj@google.com>
Reviewed-by: default avatarcalamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708520}
parent 2f5b1a7f
......@@ -5,6 +5,7 @@
#include "chrome/browser/ui/webui/chromeos/crostini_installer/crostini_installer_dialog.h"
#include "chrome/browser/chromeos/crostini/crostini_manager.h"
#include "chrome/browser/ui/webui/chromeos/crostini_installer/crostini_installer_ui.h"
#include "chrome/common/webui_url_constants.h"
#include "ui/base/ui_base_types.h"
......@@ -55,20 +56,30 @@ bool CrostiniInstallerDialog::ShouldShowCloseButton() const {
return false;
}
bool CrostiniInstallerDialog::AcceleratorPressed(
const ui::Accelerator& accelerator) {
if (accelerator.key_code() == ui::VKEY_ESCAPE) {
// Prevent the dialog from being closed. The web page should control closing
// logic.
return true;
}
return SystemWebDialogDelegate::AcceleratorPressed(accelerator);
}
void CrostiniInstallerDialog::AdjustWidgetInitParams(
views::Widget::InitParams* params) {
params->z_order = ui::ZOrderLevel::kNormal;
}
bool CrostiniInstallerDialog::CanCloseDialog() const {
// TODO(929571): If other WebUI Dialogs also need to let the WebUI control
// closing logic, we should find a more general solution.
// Disallow closing without WebUI consent.
return installer_ui_ == nullptr || installer_ui_->can_close();
}
void CrostiniInstallerDialog::OnDialogShown(
content::WebUI* webui,
content::RenderViewHost* render_view_host) {
installer_ui_ = static_cast<CrostiniInstallerUI*>(webui->GetController());
return SystemWebDialogDelegate::OnDialogShown(webui, render_view_host);
}
void CrostiniInstallerDialog::OnCloseContents(content::WebContents* source,
bool* out_close_dialog) {
installer_ui_ = nullptr;
return SystemWebDialogDelegate::OnCloseContents(source, out_close_dialog);
}
} // namespace chromeos
......@@ -11,6 +11,8 @@ class Profile;
namespace chromeos {
class CrostiniInstallerUI;
class CrostiniInstallerDialog : public SystemWebDialogDelegate {
public:
static void Show(Profile* profile);
......@@ -22,10 +24,15 @@ class CrostiniInstallerDialog : public SystemWebDialogDelegate {
// SystemWebDialogDelegate:
void GetDialogSize(gfx::Size* size) const override;
bool ShouldShowCloseButton() const override;
bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
void AdjustWidgetInitParams(views::Widget::InitParams* params) override;
bool CanCloseDialog() const override;
void OnDialogShown(content::WebUI* webui,
content::RenderViewHost* render_view_host) override;
void OnCloseContents(content::WebContents* source,
bool* out_close_dialog) override;
Profile* profile_;
CrostiniInstallerUI* installer_ui_ = nullptr;
};
} // namespace chromeos
......
......@@ -133,6 +133,10 @@ CrostiniInstallerUI::CrostiniInstallerUI(content::WebUI* web_ui)
CrostiniInstallerUI::~CrostiniInstallerUI() = default;
bool CrostiniInstallerUI::can_close() {
return can_close_;
}
void CrostiniInstallerUI::BindPageHandlerFactory(
mojo::PendingReceiver<
chromeos::crostini_installer::mojom::PageHandlerFactory>
......@@ -155,11 +159,15 @@ void CrostiniInstallerUI::CreatePageHandler(
std::move(pending_page_handler), std::move(pending_page),
// Using Unretained(this) because |page_handler_| will not out-live
// |this|.
//
base::BindOnce(&CrostiniInstallerUI::OnWebUICloseDialog,
base::Unretained(this)));
}
void CrostiniInstallerUI::OnWebUICloseDialog() {
can_close_ = true;
// CloseDialog() is a no-op if we are not in a dialog (e.g. user
// access the page using the URL directly, which is not supported).
base::BindOnce(&CrostiniInstallerUI::CloseDialog, base::Unretained(this),
nullptr));
ui::MojoWebDialogUI::CloseDialog(nullptr);
}
} // namespace chromeos
......@@ -26,6 +26,8 @@ class CrostiniInstallerUI
explicit CrostiniInstallerUI(content::WebUI* web_ui);
~CrostiniInstallerUI() override;
bool can_close();
private:
void BindPageHandlerFactory(
mojo::PendingReceiver<
......@@ -39,9 +41,12 @@ class CrostiniInstallerUI
mojo::PendingReceiver<chromeos::crostini_installer::mojom::PageHandler>
pending_page_handler) override;
void OnWebUICloseDialog();
std::unique_ptr<CrostiniInstallerPageHandler> page_handler_;
mojo::Receiver<chromeos::crostini_installer::mojom::PageHandlerFactory>
page_factory_receiver_{this};
bool can_close_ = false;
DISALLOW_COPY_AND_ASSIGN(CrostiniInstallerUI);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment