From 59cb2068cd82bab3e4ea42f8756e244575019e24 Mon Sep 17 00:00:00 2001 From: xurui Date: Tue, 31 Dec 2024 16:19:26 +0800 Subject: [PATCH] =?UTF-8?q?=E4=BF=AE=E5=A4=8Dissue=5F1424527?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: xurui --- ui/views/window/dialog_delegate.cc | 63 ++++++++++++++---- ui/views/window/dialog_delegate.h | 35 +++++++--- ui/views/window/dialog_delegate_unittest.cc | 74 +++++++++++++++++++++ 3 files changed, 151 insertions(+), 21 deletions(-) diff --git a/ui/views/window/dialog_delegate.cc b/ui/views/window/dialog_delegate.cc index 89706052e1..5324b17894 100644 --- a/ui/views/window/dialog_delegate.cc +++ b/ui/views/window/dialog_delegate.cc @@ -33,6 +33,17 @@ namespace views { +namespace { + +bool HasCallback( + const absl::variant>& + callback) { + return absl::visit( + [](const auto& variant) { return static_cast(variant); }, callback); +} + +} // namespace + // Debug information for https://crbug.com/1215247. int g_instance_count = 0; @@ -153,22 +164,33 @@ bool DialogDelegate::IsDialogButtonEnabled(ui::DialogButton button) const { bool DialogDelegate::Cancel() { DCHECK(!already_started_close_); - if (cancel_callback_) - RunCloseCallback(std::move(cancel_callback_)); + if (HasCallback(cancel_callback_)) { + return RunCloseCallback(cancel_callback_); + } return true; } bool DialogDelegate::Accept() { DCHECK(!already_started_close_); - if (accept_callback_) - RunCloseCallback(std::move(accept_callback_)); + if (HasCallback(accept_callback_)) { + return RunCloseCallback(accept_callback_); + } return true; } -void DialogDelegate::RunCloseCallback(base::OnceClosure callback) { +bool DialogDelegate::RunCloseCallback( + absl::variant>& + callback) { DCHECK(!already_started_close_); - already_started_close_ = true; - std::move(callback).Run(); + if (absl::holds_alternative(callback)) { + already_started_close_ = true; + absl::get(std::move(callback)).Run(); + } else { + already_started_close_ = + absl::get>(callback).Run(); + } + + return already_started_close_; } View* DialogDelegate::GetInitiallyFocusedView() { @@ -211,11 +233,18 @@ void DialogDelegate::WindowWillClose() { if (already_started_close_) return; - bool new_callback_present = - close_callback_ || cancel_callback_ || accept_callback_; - - if (close_callback_) - RunCloseCallback(std::move(close_callback_)); + const bool new_callback_present = close_callback_ || + HasCallback(cancel_callback_) || + HasCallback(accept_callback_); + + if (close_callback_) { + // `RunCloseCallback` takes a non-const reference to this variant to support + // the accept and cancel callbacks. It doesn't make sense to be storing a + // variant for close callbacks, so we construct the variant here instead. + absl::variant> + close_callback_wrapped(std::move(close_callback_)); + RunCloseCallback(close_callback_wrapped); + } if (new_callback_present) return; @@ -363,10 +392,20 @@ void DialogDelegate::SetAcceptCallback(base::OnceClosure callback) { accept_callback_ = std::move(callback); } +void DialogDelegate::SetAcceptCallbackWithClose( + base::RepeatingCallback callback) { + accept_callback_ = std::move(callback); +} + void DialogDelegate::SetCancelCallback(base::OnceClosure callback) { cancel_callback_ = std::move(callback); } +void DialogDelegate::SetCancelCallbackWithClose( + base::RepeatingCallback callback) { + cancel_callback_ = std::move(callback); +} + void DialogDelegate::SetCloseCallback(base::OnceClosure callback) { close_callback_ = std::move(callback); } diff --git a/ui/views/window/dialog_delegate.h b/ui/views/window/dialog_delegate.h index bbe179c460..abbd7648f8 100644 --- a/ui/views/window/dialog_delegate.h +++ b/ui/views/window/dialog_delegate.h @@ -243,13 +243,25 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate { void SetButtonEnabled(ui::DialogButton button, bool enabled); // Called when the user presses the dialog's "OK" button or presses the dialog - // accept accelerator, if there is one. + // accept accelerator, if there is one. The dialog is closed after the + // callback is run. void SetAcceptCallback(base::OnceClosure callback); + // Called when the user presses the dialog's "OK" button or presses the dialog + // accept accelerator, if there is one. Callbacks can return true to close the + // dialog, false to leave the dialog open. + void SetAcceptCallbackWithClose(base::RepeatingCallback callback); + // Called when the user presses the dialog's "Cancel" button or presses the - // dialog close accelerator (which is always VKEY_ESCAPE). + // dialog close accelerator (which is always VKEY_ESCAPE). The dialog is + // closed after the callback is run. void SetCancelCallback(base::OnceClosure callback); + // Called when the user presses the dialog's "Cancel" button or presses the + // dialog close accelerator (which is always VKEY_ESCAPE). Callbacks can + // return true to close the dialog, false to leave the dialog open. + void SetCancelCallbackWithClose(base::RepeatingCallback callback); + // Called when: // * The user presses the dialog's close button, if it has one // * The dialog's widget is closed via Widget::Close() @@ -340,9 +352,11 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate { std::unique_ptr DisownFootnoteView(); private: - // Runs a close callback, ensuring that at most one close callback is ever - // run. - void RunCloseCallback(base::OnceClosure callback); + // Runs a close callback, ensuring that at most one close callback is run + // if `callback` is a OnceClosure or returns true. + bool RunCloseCallback( + absl::variant>& + callback); // The margins between the content and the inside of the border. // TODO(crbug.com/733040): Most subclasses assume they must set their own @@ -366,12 +380,15 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate { base::ObserverList::Unchecked observer_list_; // Callbacks for the dialog's actions: - base::OnceClosure accept_callback_; - base::OnceClosure cancel_callback_; + absl::variant> + accept_callback_; + absl::variant> + cancel_callback_; base::OnceClosure close_callback_; - // Whether any of the three callbacks just above has been delivered yet, *or* - // one of the Accept/Cancel methods have been called and returned true. + // Whether any of the three callbacks just above has been delivered yet and + // returned true, *or* one of the Accept/Cancel methods have been called and + // returned true. bool already_started_close_ = false; }; diff --git a/ui/views/window/dialog_delegate_unittest.cc b/ui/views/window/dialog_delegate_unittest.cc index 263f1902c2..d14ceeb0e9 100644 --- a/ui/views/window/dialog_delegate_unittest.cc +++ b/ui/views/window/dialog_delegate_unittest.cc @@ -135,6 +135,12 @@ class DialogTest : public ViewsTestBase { return widget; } + views::Widget* CreateDialogWidget(std::unique_ptr dialog) { + views::Widget* widget = DialogDelegate::CreateDialogWidget( + std::move(dialog), GetContext(), parent_widget_->GetNativeView()); + return widget; + } + void ShowDialog() { CreateDialogWidget(dialog_)->Show(); } void SimulateKeyPress(ui::KeyboardCode key) { @@ -573,4 +579,72 @@ TEST_F(DialogDelegateCloseTest, CloseParentWidgetDoesNotInvokeCloseCallback) { EXPECT_FALSE(closed); } +TEST_F(DialogTest, AcceptCallbackWithCloseDoesNotClose) { + test::WidgetTest::WidgetAutoclosePtr widget( + CreateDialogWidget(std::make_unique())); + auto* dialog = static_cast(widget->widget_delegate()); + + bool accepted = false; + dialog->SetAcceptCallbackWithClose(base::BindLambdaForTesting([&]() { + accepted = true; + return false; + })); + + EXPECT_FALSE(widget->IsClosed()); + dialog->AcceptDialog(); + EXPECT_FALSE(widget->IsClosed()); + EXPECT_TRUE(accepted); +} + +TEST_F(DialogTest, AcceptCallbackWithCloseDoesClose) { + test::WidgetTest::WidgetAutoclosePtr widget( + CreateDialogWidget(std::make_unique())); + auto* dialog = static_cast(widget->widget_delegate()); + + bool accepted = false; + dialog->SetAcceptCallbackWithClose(base::BindLambdaForTesting([&]() { + accepted = true; + return true; + })); + + EXPECT_FALSE(widget->IsClosed()); + dialog->AcceptDialog(); + EXPECT_TRUE(widget->IsClosed()); + EXPECT_TRUE(accepted); +} + +TEST_F(DialogTest, CancelCallbackWithCloseDoesNotClose) { + test::WidgetTest::WidgetAutoclosePtr widget( + CreateDialogWidget(std::make_unique())); + auto* dialog = static_cast(widget->widget_delegate()); + + bool canceled = false; + dialog->SetCancelCallbackWithClose(base::BindLambdaForTesting([&]() { + canceled = true; + return false; + })); + + EXPECT_FALSE(widget->IsClosed()); + dialog->CancelDialog(); + EXPECT_FALSE(widget->IsClosed()); + EXPECT_TRUE(canceled); +} + +TEST_F(DialogTest, CancelCallbackWithCloseDoesClose) { + test::WidgetTest::WidgetAutoclosePtr widget( + CreateDialogWidget(std::make_unique())); + auto* dialog = static_cast(widget->widget_delegate()); + + bool canceled = false; + dialog->SetCancelCallbackWithClose(base::BindLambdaForTesting([&]() { + canceled = true; + return true; + })); + + EXPECT_FALSE(widget->IsClosed()); + dialog->CancelDialog(); + EXPECT_TRUE(widget->IsClosed()); + EXPECT_TRUE(canceled); +} + } // namespace views -- Gitee