Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(107)

Unified Diff: ui/views/window/dialog_client_view.cc

Issue 2825243002: DialogClientView: Handle nested close requests gracefully.
Patch Set: Self review Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ui/views/window/dialog_client_view.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/views/window/dialog_client_view.cc
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc
index 3cd043e2eb325ade6518cbbba973ce97210df3ad..f3638cd6d56e20a2e11ce6083e311dd377c5a445 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -87,19 +87,11 @@ DialogClientView::DialogClientView(Widget* owner, View* contents_view)
DialogClientView::~DialogClientView() {}
void DialogClientView::AcceptWindow() {
- // Only notify the delegate once. See |delegate_allowed_close_|'s comment.
- if (!delegate_allowed_close_ && GetDialogDelegate()->Accept()) {
- delegate_allowed_close_ = true;
- GetWidget()->Close();
- }
+ CloseWithDelegateMethod(&DialogDelegate::Accept, true);
}
void DialogClientView::CancelWindow() {
- // Only notify the delegate once. See |delegate_allowed_close_|'s comment.
- if (!delegate_allowed_close_ && GetDialogDelegate()->Cancel()) {
- delegate_allowed_close_ = true;
- GetWidget()->Close();
- }
+ CloseWithDelegateMethod(&DialogDelegate::Cancel, true);
}
void DialogClientView::UpdateDialogButtons() {
@@ -111,10 +103,24 @@ void DialogClientView::UpdateDialogButtons() {
// DialogClientView, ClientView overrides:
bool DialogClientView::CanClose() {
+ if (delegate_allowed_close_) {
+ DCHECK(!asking_delegate_whether_to_close_);
+ return true;
+ }
+
+ // The delegate may perform an action that causes the Widget to lose focus and
+ // (for some bubbles) attempt to close again. Widget::Close() has protection
+ // against multiple calls to Widget::Close(), but that method is above
+ // CanClose() in the stack: CanClose() has not yet told the first
+ // Widget::Close() whether it may proceed, so that protection can't be used.
+ // Detect this situation and always return false from the nested call to
+ // CanClose() so that Widget::Close() bails out early.
+ if (asking_delegate_whether_to_close_)
+ return false;
+
// If the dialog is closing but no Accept or Cancel action has been performed
// before, it's a Close action.
- if (!delegate_allowed_close_)
- delegate_allowed_close_ = GetDialogDelegate()->Close();
+ CloseWithDelegateMethod(&DialogDelegate::Close, false);
return delegate_allowed_close_;
}
@@ -419,4 +425,20 @@ void DialogClientView::SetupViews() {
extra_view_->SetGroup(kButtonGroup);
}
+void DialogClientView::CloseWithDelegateMethod(bool (DialogDelegate::*method)(),
+ bool close) {
+ DCHECK(!asking_delegate_whether_to_close_);
+
+ // Only notify the delegate once. See |delegate_allowed_close_|'s comment.
+ if (delegate_allowed_close_)
+ return;
+
+ {
+ base::AutoReset<bool> auto_reset(&asking_delegate_whether_to_close_, true);
+ delegate_allowed_close_ = (GetDialogDelegate()->*method)();
+ }
+ if (close && delegate_allowed_close_)
+ GetWidget()->Close();
+}
+
} // namespace views
« no previous file with comments | « ui/views/window/dialog_client_view.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698