Refine DialogClientView button code and unit tests.

ui/views/window/dialog_client_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/window/dialog_client_view.h"
 
 #include <algorithm>
 
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/views/controls/button/label_button.h"
@@ skipping 25 matching lines @@
       ok_button_(NULL),
       cancel_button_(NULL),
       default_button_(NULL),
       focus_manager_(NULL),
       extra_view_(NULL),
       footnote_view_(NULL),
       notified_delegate_(false) {
 }
 
 DialogClientView::~DialogClientView() {
-  if (focus_manager_)
-    focus_manager_->RemoveFocusChangeListener(this);
-  focus_manager_ = NULL;
 }
 
 void DialogClientView::AcceptWindow() {
   // Only notify the delegate once. See |notified_delegate_|'s comment.
   if (!notified_delegate_ && GetDialogDelegate()->Accept(false)) {
     notified_delegate_ = true;
     Close();
   }
 }
 
 void DialogClientView::CancelWindow() {
   // Call the standard Close handler, which checks with the delegate before
   // proceeding. This checking _isn't_ done here, but in the WM_CLOSE handler,
   // so that the close box on the window also shares this code path.
   Close();
 }
 
 void DialogClientView::UpdateDialogButtons() {
   const int buttons = GetDialogDelegate()->GetDialogButtons();
   ui::Accelerator escape(ui::VKEY_ESCAPE, ui::EF_NONE);
   if (default_button_)
     default_button_->SetIsDefault(false);
   default_button_ = NULL;
 
   if (buttons & ui::DIALOG_BUTTON_OK) {
     if (!ok_button_) {
       ok_button_ = CreateDialogButton(ui::DIALOG_BUTTON_OK);
-      if (buttons & ui::DIALOG_BUTTON_CANCEL)
+      if (!(buttons & ui::DIALOG_BUTTON_CANCEL))
         ok_button_->AddAccelerator(escape);
       AddChildView(ok_button_);
     }
 
     UpdateButton(ok_button_, ui::DIALOG_BUTTON_OK);
   } else if (ok_button_) {
+    RemoveChildView(ok_button_);

[sky, 2013/06/17 20:30:14]

Is there a particular reason you are adding this? ~View removes from parent.

[msw, 2013/06/17 21:10:56]

Ah, solely out of ignorance; updated this and simplified tests.

     delete ok_button_;
     ok_button_ = NULL;
   }
 
   if (buttons & ui::DIALOG_BUTTON_CANCEL) {
     if (!cancel_button_) {
       cancel_button_ = CreateDialogButton(ui::DIALOG_BUTTON_CANCEL);
       cancel_button_->AddAccelerator(escape);
       AddChildView(cancel_button_);
     }
 
     UpdateButton(cancel_button_, ui::DIALOG_BUTTON_CANCEL);
   } else if (cancel_button_) {
+    RemoveChildView(cancel_button_);
     delete cancel_button_;
     cancel_button_ = NULL;
   }
 
   // Use the escape key to close the window if there are no dialog buttons.
   if (!has_dialog_buttons())
     AddAccelerator(escape);
   else
     ResetAccelerators();
 }
@@ skipping 159 matching lines @@
       set_background(views::Background::CreateSolidBackground(GetNativeTheme()->
           GetSystemColor(ui::NativeTheme::kColorId_DialogBackground)));
 
     focus_manager_ = GetFocusManager();
     if (focus_manager_)
       GetFocusManager()->AddFocusChangeListener(this);
 
     UpdateDialogButtons();
     CreateExtraView();
     CreateFootnoteView();
+  } else if (!details.is_add && details.child == this) {
+    if (focus_manager_)
+      focus_manager_->RemoveFocusChangeListener(this);
+    focus_manager_ = NULL;
+  } else if (!details.is_add) {
+    if (details.child == default_button_)
+      default_button_ = NULL;
+    if (details.child == ok_button_)
+      ok_button_ = NULL;
+    if (details.child == cancel_button_)
+      cancel_button_ = NULL;
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DialogClientView, ButtonListener implementation:
 
 void DialogClientView::ButtonPressed(Button* sender, const ui::Event& event) {
   // Check for a valid delegate to avoid handling events after destruction.
   if (!GetDialogDelegate())
     return;
@@ skipping 99 matching lines @@
       gfx::Insets(0, kButtonHEdgeMargin,
                   kButtonVEdgeMargin, kButtonHEdgeMargin);
 }
 
 void DialogClientView::Close() {
   GetWidget()->Close();
   GetDialogDelegate()->OnClose();
 }
 
 }  // namespace views