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

cbuiv: remove more DialogDelegate closure overrides

This change removes almost all the remaining overrides of ::Close() in cbuiv,
and some of the overrides of other methods. Replacements are either:

1) Deleted outright
2) Replaced with lambdas inlined in the constructor
3) Replaced with void OnDialog*() methods that are used for the new callback
   system

Bug: 1011446
Change-Id: Idfe315895958f7cc92db17a30fc4cf591e4ffec8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036943
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739090}
parent 9510b484
...@@ -32,24 +32,6 @@ base::string16 JavaScriptTabModalDialogViewViews::GetWindowTitle() const { ...@@ -32,24 +32,6 @@ base::string16 JavaScriptTabModalDialogViewViews::GetWindowTitle() const {
return title_; return title_;
} }
bool JavaScriptTabModalDialogViewViews::Cancel() {
if (dialog_callback_)
std::move(dialog_callback_).Run(false, base::string16());
return true;
}
bool JavaScriptTabModalDialogViewViews::Accept() {
if (dialog_callback_)
std::move(dialog_callback_).Run(true, message_box_view_->GetInputText());
return true;
}
bool JavaScriptTabModalDialogViewViews::Close() {
if (dialog_force_closed_callback_)
std::move(dialog_force_closed_callback_).Run();
return true;
}
bool JavaScriptTabModalDialogViewViews::ShouldShowCloseButton() const { bool JavaScriptTabModalDialogViewViews::ShouldShowCloseButton() const {
return false; return false;
} }
...@@ -91,6 +73,26 @@ JavaScriptTabModalDialogViewViews::JavaScriptTabModalDialogViewViews( ...@@ -91,6 +73,26 @@ JavaScriptTabModalDialogViewViews::JavaScriptTabModalDialogViewViews(
is_alert ? ui::DIALOG_BUTTON_OK is_alert ? ui::DIALOG_BUTTON_OK
: (ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL)); : (ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL));
DialogDelegate::set_accept_callback(base::BindOnce(
[](JavaScriptTabModalDialogViewViews* dialog) {
if (dialog->dialog_callback_)
std::move(dialog->dialog_callback_)
.Run(true, dialog->message_box_view_->GetInputText());
},
base::Unretained(this)));
DialogDelegate::set_cancel_callback(base::BindOnce(
[](JavaScriptTabModalDialogViewViews* dialog) {
if (dialog->dialog_callback_)
std::move(dialog->dialog_callback_).Run(false, base::string16());
},
base::Unretained(this)));
DialogDelegate::set_close_callback(base::BindOnce(
[](JavaScriptTabModalDialogViewViews* dialog) {
if (dialog->dialog_force_closed_callback_)
std::move(dialog->dialog_force_closed_callback_).Run();
},
base::Unretained(this)));
int options = views::MessageBoxView::DETECT_DIRECTIONALITY; int options = views::MessageBoxView::DETECT_DIRECTIONALITY;
if (dialog_type == content::JAVASCRIPT_DIALOG_TYPE_PROMPT) if (dialog_type == content::JAVASCRIPT_DIALOG_TYPE_PROMPT)
options |= views::MessageBoxView::HAS_PROMPT_FIELD; options |= views::MessageBoxView::HAS_PROMPT_FIELD;
......
...@@ -31,9 +31,6 @@ class JavaScriptTabModalDialogViewViews ...@@ -31,9 +31,6 @@ class JavaScriptTabModalDialogViewViews
// views::DialogDelegate: // views::DialogDelegate:
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
bool Cancel() override;
bool Accept() override;
bool Close() override;
// views::WidgetDelegate: // views::WidgetDelegate:
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
......
...@@ -92,8 +92,7 @@ bool OutdatedUpgradeBubbleView::ShouldShowCloseButton() const { ...@@ -92,8 +92,7 @@ bool OutdatedUpgradeBubbleView::ShouldShowCloseButton() const {
return true; return true;
} }
bool OutdatedUpgradeBubbleView::Accept() { void OutdatedUpgradeBubbleView::OnDialogAccepted() {
uma_recorded_ = true;
// Offset the +1 in the dtor. // Offset the +1 in the dtor.
--g_num_ignored_bubbles; --g_num_ignored_bubbles;
if (auto_update_enabled_) { if (auto_update_enabled_) {
...@@ -129,17 +128,6 @@ bool OutdatedUpgradeBubbleView::Accept() { ...@@ -129,17 +128,6 @@ bool OutdatedUpgradeBubbleView::Accept() {
base::BindOnce(&google_update::ElevateIfNeededToReenableUpdates)); base::BindOnce(&google_update::ElevateIfNeededToReenableUpdates));
#endif // defined(OS_WIN) #endif // defined(OS_WIN)
} }
return true;
}
bool OutdatedUpgradeBubbleView::Close() {
// DialogDelegate::Close() would call Accept(), as there is only one button.
// Prevent that and record UMA. Note in the past there was also a "Later"
// button, hence the name.
if (!uma_recorded_)
base::RecordAction(base::UserMetricsAction("OutdatedUpgradeBubble.Later"));
return true;
} }
void OutdatedUpgradeBubbleView::Init() { void OutdatedUpgradeBubbleView::Init() {
...@@ -169,5 +157,10 @@ OutdatedUpgradeBubbleView::OutdatedUpgradeBubbleView( ...@@ -169,5 +157,10 @@ OutdatedUpgradeBubbleView::OutdatedUpgradeBubbleView(
ui::DIALOG_BUTTON_OK, ui::DIALOG_BUTTON_OK,
l10n_util::GetStringUTF16(auto_update_enabled_ ? IDS_REINSTALL_APP l10n_util::GetStringUTF16(auto_update_enabled_ ? IDS_REINSTALL_APP
: IDS_REENABLE_UPDATES)); : IDS_REENABLE_UPDATES));
DialogDelegate::set_accept_callback(base::BindOnce(
&OutdatedUpgradeBubbleView::OnDialogAccepted, base::Unretained(this)));
DialogDelegate::set_close_callback(
base::BindOnce(&base::RecordAction,
base::UserMetricsAction("OutdatedUpgradeBubble.Later")));
chrome::RecordDialogCreation(chrome::DialogIdentifier::OUTDATED_UPGRADE); chrome::RecordDialogCreation(chrome::DialogIdentifier::OUTDATED_UPGRADE);
} }
...@@ -26,8 +26,6 @@ class OutdatedUpgradeBubbleView : public views::BubbleDialogDelegateView { ...@@ -26,8 +26,6 @@ class OutdatedUpgradeBubbleView : public views::BubbleDialogDelegateView {
void WindowClosing() override; void WindowClosing() override;
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
bool Accept() override;
bool Close() override;
void Init() override; void Init() override;
private: private:
...@@ -36,9 +34,7 @@ class OutdatedUpgradeBubbleView : public views::BubbleDialogDelegateView { ...@@ -36,9 +34,7 @@ class OutdatedUpgradeBubbleView : public views::BubbleDialogDelegateView {
bool auto_update_enabled); bool auto_update_enabled);
~OutdatedUpgradeBubbleView() override; ~OutdatedUpgradeBubbleView() override;
// Since Accept() may synchronously open a URL and deactivate the bubble void OnDialogAccepted();
// (which calls Close()), this prevents Close() recording UMA a second time.
bool uma_recorded_ = false;
// Identifies if auto-update is enabled or not. // Identifies if auto-update is enabled or not.
const bool auto_update_enabled_; const bool auto_update_enabled_;
......
...@@ -27,6 +27,21 @@ AutoSigninFirstRunDialogView::AutoSigninFirstRunDialogView( ...@@ -27,6 +27,21 @@ AutoSigninFirstRunDialogView::AutoSigninFirstRunDialogView(
l10n_util::GetStringUTF16(IDS_AUTO_SIGNIN_FIRST_RUN_OK)); l10n_util::GetStringUTF16(IDS_AUTO_SIGNIN_FIRST_RUN_OK));
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL, DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
l10n_util::GetStringUTF16(IDS_TURN_OFF)); l10n_util::GetStringUTF16(IDS_TURN_OFF));
using ControllerCallbackFn = void (CredentialManagerDialogController::*)();
auto call_controller = [](AutoSigninFirstRunDialogView* dialog,
ControllerCallbackFn func) {
if (dialog->controller_) {
(dialog->controller_->*func)();
}
};
DialogDelegate::set_accept_callback(
base::BindOnce(call_controller, base::Unretained(this),
&CredentialManagerDialogController::OnAutoSigninOK));
DialogDelegate::set_cancel_callback(
base::BindOnce(call_controller, base::Unretained(this),
&CredentialManagerDialogController::OnAutoSigninTurnOff));
chrome::RecordDialogCreation(chrome::DialogIdentifier::AUTO_SIGNIN_FIRST_RUN); chrome::RecordDialogCreation(chrome::DialogIdentifier::AUTO_SIGNIN_FIRST_RUN);
} }
...@@ -69,27 +84,6 @@ void AutoSigninFirstRunDialogView::WindowClosing() { ...@@ -69,27 +84,6 @@ void AutoSigninFirstRunDialogView::WindowClosing() {
controller_->OnCloseDialog(); controller_->OnCloseDialog();
} }
bool AutoSigninFirstRunDialogView::Cancel() {
// On Mac the button click event may be dispatched after the dialog was
// hidden. Thus, the controller can be NULL.
if (controller_)
controller_->OnAutoSigninTurnOff();
return true;
}
bool AutoSigninFirstRunDialogView::Accept() {
// On Mac the button click event may be dispatched after the dialog was
// hidden. Thus, the controller can be NULL.
if (controller_)
controller_->OnAutoSigninOK();
return true;
}
bool AutoSigninFirstRunDialogView::Close() {
// Do nothing rather than running Cancel(), which would turn off auto-signin.
return true;
}
void AutoSigninFirstRunDialogView::InitWindow() { void AutoSigninFirstRunDialogView::InitWindow() {
set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType( set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
views::TEXT, views::TEXT)); views::TEXT, views::TEXT));
......
...@@ -27,9 +27,6 @@ class AutoSigninFirstRunDialogView : public views::DialogDelegateView, ...@@ -27,9 +27,6 @@ class AutoSigninFirstRunDialogView : public views::DialogDelegateView,
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
gfx::Size CalculatePreferredSize() const override; gfx::Size CalculatePreferredSize() const override;
void WindowClosing() override; void WindowClosing() override;
bool Cancel() override;
bool Accept() override;
bool Close() override;
// Sets up the child views. // Sets up the child views.
void InitWindow(); void InitWindow();
......
...@@ -281,6 +281,8 @@ PasswordSaveUpdateView::PasswordSaveUpdateView( ...@@ -281,6 +281,8 @@ PasswordSaveUpdateView::PasswordSaveUpdateView(
} }
DialogDelegate::SetFootnoteView(CreateFooterView()); DialogDelegate::SetFootnoteView(CreateFooterView());
DialogDelegate::set_cancel_callback(base::BindOnce(
&PasswordSaveUpdateView::OnDialogCancelled, base::Unretained(this)));
UpdateDialogButtons(); UpdateDialogButtons();
} }
...@@ -309,18 +311,13 @@ bool PasswordSaveUpdateView::Accept() { ...@@ -309,18 +311,13 @@ bool PasswordSaveUpdateView::Accept() {
return true; return true;
} }
bool PasswordSaveUpdateView::Cancel() { void PasswordSaveUpdateView::OnDialogCancelled() {
UpdateUsernameAndPasswordInModel(); UpdateUsernameAndPasswordInModel();
if (is_update_bubble_) { if (is_update_bubble_) {
controller_.OnNopeUpdateClicked(); controller_.OnNopeUpdateClicked();
return true; } else {
controller_.OnNeverForThisSiteClicked();
} }
controller_.OnNeverForThisSiteClicked();
return true;
}
bool PasswordSaveUpdateView::Close() {
return true;
} }
void PasswordSaveUpdateView::ButtonPressed(views::Button* sender, void PasswordSaveUpdateView::ButtonPressed(views::Button* sender,
......
...@@ -57,8 +57,6 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase, ...@@ -57,8 +57,6 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase,
bool ShouldShowWindowIcon() const override; bool ShouldShowWindowIcon() const override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
bool Accept() override; bool Accept() override;
bool Cancel() override;
bool Close() override;
// View: // View:
void AddedToWidget() override; void AddedToWidget() override;
...@@ -69,6 +67,7 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase, ...@@ -69,6 +67,7 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase,
void ReplaceWithPromo(); void ReplaceWithPromo();
void UpdateDialogButtons(); void UpdateDialogButtons();
std::unique_ptr<views::View> CreateFooterView(); std::unique_ptr<views::View> CreateFooterView();
void OnDialogCancelled();
SaveUpdateBubbleController controller_; SaveUpdateBubbleController controller_;
......
...@@ -95,6 +95,15 @@ EnterpriseStartupDialogView::EnterpriseStartupDialogView( ...@@ -95,6 +95,15 @@ EnterpriseStartupDialogView::EnterpriseStartupDialogView(
DialogDelegate::set_draggable(true); DialogDelegate::set_draggable(true);
DialogDelegate::set_buttons(ui::DIALOG_BUTTON_OK); DialogDelegate::set_buttons(ui::DIALOG_BUTTON_OK);
DialogDelegate::SetExtraView(CreateLogoView()); DialogDelegate::SetExtraView(CreateLogoView());
DialogDelegate::set_accept_callback(
base::BindOnce(&EnterpriseStartupDialogView::RunDialogCallback,
base::Unretained(this), true));
DialogDelegate::set_cancel_callback(
base::BindOnce(&EnterpriseStartupDialogView::RunDialogCallback,
base::Unretained(this), false));
DialogDelegate::set_close_callback(
base::BindOnce(&EnterpriseStartupDialogView::RunDialogCallback,
base::Unretained(this), false));
SetBorder(views::CreateEmptyBorder(GetDialogInsets())); SetBorder(views::CreateEmptyBorder(GetDialogInsets()));
CreateDialogWidget(this, nullptr, nullptr)->Show(); CreateDialogWidget(this, nullptr, nullptr)->Show();
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -177,19 +186,6 @@ void EnterpriseStartupDialogView::RunDialogCallback(bool was_accepted) { ...@@ -177,19 +186,6 @@ void EnterpriseStartupDialogView::RunDialogCallback(bool was_accepted) {
#endif #endif
} }
bool EnterpriseStartupDialogView::Accept() {
RunDialogCallback(true);
return true;
}
bool EnterpriseStartupDialogView::Cancel() {
RunDialogCallback(false);
return true;
}
bool EnterpriseStartupDialogView::Close() {
return Cancel();
}
bool EnterpriseStartupDialogView::ShouldShowWindowTitle() const { bool EnterpriseStartupDialogView::ShouldShowWindowTitle() const {
return false; return false;
} }
......
...@@ -42,9 +42,6 @@ class EnterpriseStartupDialogView : public views::DialogDelegateView { ...@@ -42,9 +42,6 @@ class EnterpriseStartupDialogView : public views::DialogDelegateView {
void RunDialogCallback(bool was_accepted); void RunDialogCallback(bool was_accepted);
// override views::DialogDelegateView // override views::DialogDelegateView
bool Accept() override;
bool Cancel() override;
bool Close() override;
bool ShouldShowWindowTitle() const override; bool ShouldShowWindowTitle() const override;
ui::ModalType GetModalType() const override; ui::ModalType GetModalType() const override;
......
...@@ -128,10 +128,6 @@ void QRCodeGeneratorBubble::WindowClosing() { ...@@ -128,10 +128,6 @@ void QRCodeGeneratorBubble::WindowClosing() {
} }
} }
bool QRCodeGeneratorBubble::Close() {
return Cancel();
}
const char* QRCodeGeneratorBubble::GetClassName() const { const char* QRCodeGeneratorBubble::GetClassName() const {
return "QRCodeGeneratorBubble"; return "QRCodeGeneratorBubble";
} }
......
...@@ -63,7 +63,6 @@ class QRCodeGeneratorBubble : public QRCodeGeneratorBubbleView, ...@@ -63,7 +63,6 @@ class QRCodeGeneratorBubble : public QRCodeGeneratorBubbleView,
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
void WindowClosing() override; void WindowClosing() override;
bool Close() override;
const char* GetClassName() const override; const char* GetClassName() const override;
// views::BubbleDialogDelegateView: // views::BubbleDialogDelegateView:
......
...@@ -75,12 +75,6 @@ bool RelaunchRecommendedBubbleView::Accept() { ...@@ -75,12 +75,6 @@ bool RelaunchRecommendedBubbleView::Accept() {
return false; return false;
} }
bool RelaunchRecommendedBubbleView::Close() {
base::RecordAction(base::UserMetricsAction("RelaunchRecommended_Close"));
return true;
}
base::string16 RelaunchRecommendedBubbleView::GetWindowTitle() const { base::string16 RelaunchRecommendedBubbleView::GetWindowTitle() const {
return relaunch_recommended_timer_.GetWindowTitle(); return relaunch_recommended_timer_.GetWindowTitle();
} }
...@@ -156,6 +150,10 @@ RelaunchRecommendedBubbleView::RelaunchRecommendedBubbleView( ...@@ -156,6 +150,10 @@ RelaunchRecommendedBubbleView::RelaunchRecommendedBubbleView(
ui::DIALOG_BUTTON_OK, ui::DIALOG_BUTTON_OK,
l10n_util::GetStringUTF16(IDS_RELAUNCH_ACCEPT_BUTTON)); l10n_util::GetStringUTF16(IDS_RELAUNCH_ACCEPT_BUTTON));
DialogDelegate::set_close_callback(
base::BindOnce(&base::RecordAction,
base::UserMetricsAction("RelaunchRecommended_Close")));
chrome::RecordDialogCreation(chrome::DialogIdentifier::RELAUNCH_RECOMMENDED); chrome::RecordDialogCreation(chrome::DialogIdentifier::RELAUNCH_RECOMMENDED);
set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType( set_margins(ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(
views::TEXT, views::TEXT)); views::TEXT, views::TEXT));
......
...@@ -33,7 +33,6 @@ class RelaunchRecommendedBubbleView : public LocationBarBubbleDelegateView { ...@@ -33,7 +33,6 @@ class RelaunchRecommendedBubbleView : public LocationBarBubbleDelegateView {
// LocationBarBubbleDelegateView: // LocationBarBubbleDelegateView:
bool Accept() override; bool Accept() override;
bool Close() override;
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
gfx::ImageSkia GetWindowIcon() override; gfx::ImageSkia GetWindowIcon() override;
......
...@@ -205,10 +205,6 @@ bool TaskManagerView::Accept() { ...@@ -205,10 +205,6 @@ bool TaskManagerView::Accept() {
return false; return false;
} }
bool TaskManagerView::Close() {
return true;
}
bool TaskManagerView::IsDialogButtonEnabled(ui::DialogButton button) const { bool TaskManagerView::IsDialogButtonEnabled(ui::DialogButton button) const {
const ui::ListSelectionModel::SelectedIndices& selections( const ui::ListSelectionModel::SelectedIndices& selections(
tab_table_->selection_model().selected_indices()); tab_table_->selection_model().selected_indices());
...@@ -293,6 +289,12 @@ TaskManagerView::TaskManagerView() ...@@ -293,6 +289,12 @@ TaskManagerView::TaskManagerView()
DialogDelegate::set_button_label( DialogDelegate::set_button_label(
ui::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_TASK_MANAGER_KILL)); ui::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_TASK_MANAGER_KILL));
// Avoid calling Accept() when closing the dialog, since Accept() here means
// "kill task" (!).
// TODO(ellyjones): Remove this once the Accept() override is removed from
// this class.
DialogDelegate::set_close_callback(base::DoNothing());
Init(); Init();
chrome::RecordDialogCreation(chrome::DialogIdentifier::TASK_MANAGER); chrome::RecordDialogCreation(chrome::DialogIdentifier::TASK_MANAGER);
} }
......
...@@ -63,7 +63,6 @@ class TaskManagerView : public TableViewDelegate, ...@@ -63,7 +63,6 @@ class TaskManagerView : public TableViewDelegate,
gfx::ImageSkia GetWindowIcon() override; gfx::ImageSkia GetWindowIcon() override;
std::string GetWindowName() const override; std::string GetWindowName() const override;
bool Accept() override; bool Accept() override;
bool Close() override;
bool IsDialogButtonEnabled(ui::DialogButton button) const override; bool IsDialogButtonEnabled(ui::DialogButton button) const override;
void WindowClosing() override; void WindowClosing() 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