Commit e44fc9de authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

cbuiv: remove some more DialogDelegate closure overrides

The ones in this CL are all fairly simple:

* PasswordReuseModalWarningDialog calls an internal callback in all three cases.
  The cases are *almost* simple enough to wrap them up directly like:

    set_accept_callback(base::BindOnce(
        done_callback_, WarningAction::CHANGE_PASSWORD));

  except that doing this creates multiple copies of done_callback_, which is a
  no-no since it is a OnceCallback. The fact that it actually is logically only
  invoked once but that this is invisible to the type system is annoying.
* SendTabToSelfBubbleViewImpl pointlessly overrode Close() to call Cancel(),
  which is what happens anyway when there are no buttons.
* SessionCrashedBubbleView had simple overrides that invoked methods on itself
  and then returned true, so these are just inlined now.
* SettingsResetPromptDialog just called into its controller and returned true in
  all three cases, so these are now inlined.
* StoragePressureBubbleView does some stuff and unconditionally returns true, so
  it now has the OnDialogAccepted pattern, but probably not in a way that will
  ever be factored out fully.
* TabModalConfirmDialogViews just calls its delegate in all three cases, so
  these are inlined.

Bug: 1011446
Change-Id: Ic736fbbf8e2e43ecefcdd3c0e890bc20ee0cc10f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2028171
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736595}
parent 78c886cb
......@@ -148,6 +148,25 @@ PasswordReuseModalWarningDialog::PasswordReuseModalWarningDialog(
ui::DIALOG_BUTTON_CANCEL,
l10n_util::GetStringUTF16(IDS_PAGE_INFO_IGNORE_PASSWORD_WARNING_BUTTON));
// The set_*_callback() methods below need a OnceCallback each and we only have one
// (done_callback_), so create a proxy callback that references done_callback_ and use it for each
// of the set_*_callback() callbacks. Note that since only one of the three callbacks can ever be
// invoked, done_callback_ is still run at most once.
auto make_done_callback = [this](safe_browsing::WarningAction value) {
return base::BindOnce(
[](OnWarningDone* callback, safe_browsing::WarningAction value) {
std::move(*callback).Run(value);
},
base::Unretained(&done_callback_), value);
};
DialogDelegate::set_accept_callback(
password_type_.account_type() != ReusedPasswordAccountType::SAVED_PASSWORD
? make_done_callback(WarningAction::CHANGE_PASSWORD)
: base::DoNothing());
DialogDelegate::set_cancel_callback(
make_done_callback(WarningAction::IGNORE_WARNING));
DialogDelegate::set_close_callback(make_done_callback(WarningAction::CLOSE));
// |service| maybe NULL in tests.
if (service_)
service_->AddObserver(this);
......@@ -263,25 +282,6 @@ bool PasswordReuseModalWarningDialog::ShouldShowWindowIcon() const {
return true;
}
bool PasswordReuseModalWarningDialog::Cancel() {
std::move(done_callback_).Run(WarningAction::IGNORE_WARNING);
return true;
}
bool PasswordReuseModalWarningDialog::Accept() {
if (password_type_.account_type() !=
ReusedPasswordAccountType::SAVED_PASSWORD)
std::move(done_callback_).Run(WarningAction::CHANGE_PASSWORD);
return true;
}
bool PasswordReuseModalWarningDialog::Close() {
if (done_callback_)
std::move(done_callback_).Run(WarningAction::CLOSE);
return true;
}
void PasswordReuseModalWarningDialog::OnGaiaPasswordChanged() {
GetWidget()->Close();
}
......
......@@ -47,9 +47,6 @@ class PasswordReuseModalWarningDialog
bool ShouldShowCloseButton() const override;
gfx::ImageSkia GetWindowIcon() override;
bool ShouldShowWindowIcon() const override;
bool Cancel() override;
bool Accept() override;
bool Close() override;
// ChromePasswordProtectionService::Observer:
void OnGaiaPasswordChanged() override;
......
......@@ -71,10 +71,6 @@ void SendTabToSelfBubbleViewImpl::WindowClosing() {
}
}
bool SendTabToSelfBubbleViewImpl::Close() {
return Cancel();
}
void SendTabToSelfBubbleViewImpl::ButtonPressed(views::Button* sender,
const ui::Event& event) {
DevicePressed(sender->tag());
......
......@@ -52,9 +52,6 @@ class SendTabToSelfBubbleViewImpl : public SendTabToSelfBubbleView,
base::string16 GetWindowTitle() const override;
void WindowClosing() override;
// views::DialogDelegate:
bool Close() override;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
......
......@@ -196,6 +196,13 @@ SessionCrashedBubbleView::SessionCrashedBubbleView(views::View* anchor_view,
DialogDelegate::set_button_label(
ui::DIALOG_BUTTON_CANCEL,
l10n_util::GetStringUTF16(IDS_SESSION_CRASHED_VIEW_STARTUP_PAGES_BUTTON));
DialogDelegate::set_accept_callback(
base::BindOnce(&SessionCrashedBubbleView::RestorePreviousSession,
base::Unretained(this)));
DialogDelegate::set_cancel_callback(base::BindOnce(
&SessionCrashedBubbleView::OpenStartupPages, base::Unretained(this)));
set_close_on_deactivate(false);
chrome::RecordDialogCreation(chrome::DialogIdentifier::SESSION_CRASHED);
......@@ -302,24 +309,6 @@ std::unique_ptr<views::View> SessionCrashedBubbleView::CreateUmaOptInView() {
return uma_view;
}
bool SessionCrashedBubbleView::Accept() {
RestorePreviousSession();
return true;
}
// The cancel button is used as an option to open the startup pages instead of
// restoring the previous session.
bool SessionCrashedBubbleView::Cancel() {
OpenStartupPages();
return true;
}
bool SessionCrashedBubbleView::Close() {
// Don't default to Accept() just because that's the only choice. Instead, do
// nothing.
return true;
}
void SessionCrashedBubbleView::StyledLabelLinkClicked(views::StyledLabel* label,
const gfx::Range& range,
int event_flags) {
......
......@@ -53,9 +53,6 @@ class SessionCrashedBubbleView : public SessionCrashedBubble,
bool ShouldShowWindowTitle() const override;
bool ShouldShowCloseButton() const override;
void OnWidgetDestroying(views::Widget* widget) override;
bool Accept() override;
bool Cancel() override;
bool Close() override;
// views::BubbleDialogDelegateView methods.
void Init() override;
......
......@@ -42,6 +42,15 @@ SettingsResetPromptDialog::SettingsResetPromptDialog(
DialogDelegate::set_button_label(
ui::DIALOG_BUTTON_OK,
l10n_util::GetStringUTF16(IDS_SETTINGS_RESET_PROMPT_ACCEPT_BUTTON_LABEL));
DialogDelegate::set_accept_callback(
base::BindOnce(&safe_browsing::SettingsResetPromptController::Accept,
base::Unretained(controller_)));
DialogDelegate::set_cancel_callback(
base::BindOnce(&safe_browsing::SettingsResetPromptController::Cancel,
base::Unretained(controller_)));
DialogDelegate::set_close_callback(
base::BindOnce(&safe_browsing::SettingsResetPromptController::Close,
base::Unretained(controller_)));
set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
views::TEXT, views::TEXT));
......@@ -95,32 +104,6 @@ base::string16 SettingsResetPromptDialog::GetWindowTitle() const {
return controller_->GetWindowTitle();
}
// DialogDelegate overrides.
bool SettingsResetPromptDialog::Accept() {
if (controller_) {
controller_->Accept();
controller_ = nullptr;
}
return true;
}
bool SettingsResetPromptDialog::Cancel() {
if (controller_) {
controller_->Cancel();
controller_ = nullptr;
}
return true;
}
bool SettingsResetPromptDialog::Close() {
if (controller_) {
controller_->Close();
controller_ = nullptr;
}
return true;
}
// View overrides.
gfx::Size SettingsResetPromptDialog::CalculatePreferredSize() const {
......
......@@ -40,20 +40,12 @@ class SettingsResetPromptDialog : public views::DialogDelegateView {
base::string16 GetWindowTitle() const override;
bool ShouldShowCloseButton() const override;
// views::DialogDelegate overrides.
bool Accept() override;
bool Cancel() override;
// We override |Close()| because we want to distinguish in our metrics between
// users clicking the cancel button versus dismissing the dialog in other
// ways.
bool Close() override;
// views::View overrides.
gfx::Size CalculatePreferredSize() const override;
private:
Browser* browser_;
safe_browsing::SettingsResetPromptController* controller_;
safe_browsing::SettingsResetPromptController* const controller_;
DISALLOW_COPY_AND_ASSIGN(SettingsResetPromptDialog);
};
......
......@@ -63,6 +63,8 @@ StoragePressureBubbleView::StoragePressureBubbleView(
ui::DIALOG_BUTTON_OK,
l10n_util::GetStringUTF16(
IDS_SETTINGS_STORAGE_PRESSURE_BUBBLE_VIEW_BUTTON_LABEL));
DialogDelegate::set_accept_callback(base::BindOnce(
&StoragePressureBubbleView::OnDialogAccepted, base::Unretained(this)));
}
base::string16 StoragePressureBubbleView::GetWindowTitle() const {
......@@ -70,18 +72,15 @@ base::string16 StoragePressureBubbleView::GetWindowTitle() const {
IDS_SETTINGS_STORAGE_PRESSURE_BUBBLE_VIEW_TITLE);
}
bool StoragePressureBubbleView::Accept() {
void StoragePressureBubbleView::OnDialogAccepted() {
// TODO(ellyjones): What is this doing here? The widget's about to close
// anyway?
GetWidget()->Close();
const GURL all_sites_gurl(kAllSitesContentSettingsUrl);
NavigateParams params(browser_, all_sites_gurl,
ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
Navigate(&params);
return true;
}
bool StoragePressureBubbleView::Close() {
return true;
}
void StoragePressureBubbleView::Init() {
......
......@@ -20,10 +20,10 @@ class StoragePressureBubbleView : public views::BubbleDialogDelegateView {
Browser* browser,
const url::Origin origin);
void OnDialogAccepted();
// views::DialogDelegate:
base::string16 GetWindowTitle() const override;
bool Accept() override;
bool Close() override;
// views::BubbleDialogDelegateView:
void Init() override;
......
......@@ -38,6 +38,16 @@ TabModalConfirmDialogViews::TabModalConfirmDialogViews(
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
delegate_->GetCancelButtonTitle());
DialogDelegate::set_accept_callback(
base::BindOnce(&TabModalConfirmDialogDelegate::Accept,
base::Unretained(delegate_.get())));
DialogDelegate::set_cancel_callback(
base::BindOnce(&TabModalConfirmDialogDelegate::Cancel,
base::Unretained(delegate_.get())));
DialogDelegate::set_close_callback(
base::BindOnce(&TabModalConfirmDialogDelegate::Close,
base::Unretained(delegate_.get())));
base::Optional<int> default_button = delegate_->GetDefaultDialogButton();
if (bool(default_button))
DialogDelegate::set_default_button(*default_button);
......@@ -64,21 +74,6 @@ base::string16 TabModalConfirmDialogViews::GetWindowTitle() const {
return delegate_->GetTitle();
}
bool TabModalConfirmDialogViews::Cancel() {
delegate_->Cancel();
return true;
}
bool TabModalConfirmDialogViews::Accept() {
delegate_->Accept();
return true;
}
bool TabModalConfirmDialogViews::Close() {
delegate_->Close();
return true;
}
// Tab-modal confirmation dialogs should not show an "X" close button in the top
// right corner. They should only have yes/no buttons.
bool TabModalConfirmDialogViews::ShouldShowCloseButton() const {
......
......@@ -37,9 +37,6 @@ class TabModalConfirmDialogViews : public TabModalConfirmDialog,
// views::DialogDelegate:
base::string16 GetWindowTitle() const override;
bool Cancel() override;
bool Accept() override;
bool Close() override;
bool ShouldShowCloseButton() const override;
views::View* GetContentsView() override;
void DeleteDelegate() override;
......
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