Remove DialogDelegate::OnClosed() which is redundant with

ui/views/window/dialog_client_view.cc

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 569944542f94a7623b6c4c9f96f84cc393728a72..d6be8f4724e78a0b46c787e44e3ec588e7e5b1b2 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -58,25 +58,24 @@ DialogClientView::DialogClientView(Widget* owner, View* contents_view)
       cancel_button_(NULL),
       extra_view_(NULL),
       footnote_view_(NULL),
-      notified_delegate_(false) {
-}
+      delegate_allowed_close_(false) {}
 
 DialogClientView::~DialogClientView() {
 }
 
 void DialogClientView::AcceptWindow() {
-  // Only notify the delegate once. See |notified_delegate_|'s comment.
-  if (!notified_delegate_ && GetDialogDelegate()->Accept(false)) {
-    notified_delegate_ = true;
-    Close();
+  // Only notify the delegate once. See |delegate_allowed_close_|'s comment.
+  if (!delegate_allowed_close_ && GetDialogDelegate()->Accept(false)) {
+    delegate_allowed_close_ = true;
+    GetWidget()->Close();
   }
 }
 
 void DialogClientView::CancelWindow() {
-  // Only notify the delegate once. See |notified_delegate_|'s comment.
-  if (!notified_delegate_ && GetDialogDelegate()->Cancel()) {
-    notified_delegate_ = true;
-    Close();
+  // Only notify the delegate once. See |delegate_allowed_close_|'s comment.
+  if (!delegate_allowed_close_ && GetDialogDelegate()->Cancel()) {
+    delegate_allowed_close_ = true;
+    GetWidget()->Close();
   }
 }
 
@@ -122,17 +121,11 @@ void DialogClientView::UpdateDialogButtons() {
 // DialogClientView, ClientView overrides:
 
 bool DialogClientView::CanClose() {
-  if (notified_delegate_)
-    return true;
-
-  // The dialog is closing but no Accept or Cancel action has been performed
-  // before: it's a Close action.
-  if (GetDialogDelegate()->Close()) {
-    notified_delegate_ = true;
-    GetDialogDelegate()->OnClosed();
-    return true;
-  }
-  return false;
+  // If the dialog is closing but no Accept or Cancel action has been performed
+  // before, it's a Close action.
+  delegate_allowed_close_ =

[msw, 2016/02/22 18:44:04]

nit: |=

[Evan Stade, 2016/02/22 22:10:18]

I don't think that has the same behavior. We specifically want to avoid calling
GetDialogDelegate()->Close() if |delegate_allowed_close_| is already true. Since
perhaps that is subtle, I changed this code slightly to make it more obvious.

+      delegate_allowed_close_ || GetDialogDelegate()->Close();
+  return delegate_allowed_close_;
 }
 
 DialogClientView* DialogClientView::AsDialogClientView() {
@@ -239,7 +232,7 @@ void DialogClientView::Layout() {
 
 bool DialogClientView::AcceleratorPressed(const ui::Accelerator& accelerator) {
   DCHECK_EQ(accelerator.key_code(), ui::VKEY_ESCAPE);
-  Close();
+  GetWidget()->Close();
   return true;
 }
 
@@ -294,7 +287,7 @@ DialogClientView::DialogClientView(View* contents_view)
       cancel_button_(NULL),
       extra_view_(NULL),
       footnote_view_(NULL),
-      notified_delegate_(false) {}
+      delegate_allowed_close_(false) {}
 
 DialogDelegate* DialogClientView::GetDialogDelegate() const {
   return GetWidget()->widget_delegate()->AsDialogDelegate();
@@ -375,9 +368,4 @@ gfx::Insets DialogClientView::GetButtonRowInsets() const {
                   kButtonVEdgeMarginNew, kButtonHEdgeMarginNew);
 }
 
-void DialogClientView::Close() {
-  GetWidget()->Close();
-  GetDialogDelegate()->OnClosed();
-}
-
 }  // namespace views