Refine DialogClientView button code and unit tests.

Add an escape accelerator to the ok button only if there is *not* a cancel button.
Move FocusManager/ChangeListener cleanup to ViewHierarchyChanged.
Clear ok/cancel/default button pointers on ViewHierarchyChanged removal.

Remove view_unittest MockMenuModel and related use.
( these were left unused by http://crrev.com/87490 )

Move tests and helpers to dialog_delegate_unittest, cleanup.
Merge, cleanup, and add tests, no loss of coverage.

BUG=241451,166075
TEST=Dialogs with just an [Ok] button will accept on [Esc]; no crashes.
R=sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/17127003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206950 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 248 insertions, 303 deletions

diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc
index 27263b4..53b28f3 100644
--- a/ui/views/view_unittest.cc
+++ b/ui/views/view_unittest.cc
@@ -15,7 +15,6 @@
 #include "ui/base/events/event.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/models/simple_menu_model.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animator.h"
@@ -23,9 +22,6 @@
 #include "ui/gfx/path.h"
 #include "ui/gfx/transform.h"
 #include "ui/views/background.h"
-#include "ui/views/controls/button/button_dropdown.h"
-#include "ui/views/controls/button/checkbox.h"
-#include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/native/native_view_host.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/textfield/textfield.h"
@@ -1547,260 +1543,6 @@ TEST_F(ViewTest, DISABLED_RerouteMouseWheelTest) {
 #endif
 
 ////////////////////////////////////////////////////////////////////////////////
-// Dialogs' default button
-////////////////////////////////////////////////////////////////////////////////
-
-class MockMenuModel : public ui::MenuModel {
- public:
-  MOCK_CONST_METHOD0(HasIcons, bool());
-  MOCK_CONST_METHOD0(GetItemCount, int());
-  MOCK_CONST_METHOD1(GetTypeAt, ItemType(int index));
-  MOCK_CONST_METHOD1(GetSeparatorTypeAt, ui::MenuSeparatorType(int index));
-  MOCK_CONST_METHOD1(GetCommandIdAt, int(int index));
-  MOCK_CONST_METHOD1(GetLabelAt, string16(int index));
-  MOCK_CONST_METHOD1(IsItemDynamicAt, bool(int index));
-  MOCK_CONST_METHOD1(GetLabelFontAt, const gfx::Font* (int index));
-  MOCK_CONST_METHOD2(GetAcceleratorAt, bool(int index,
-      ui::Accelerator* accelerator));
-  MOCK_CONST_METHOD1(IsItemCheckedAt, bool(int index));
-  MOCK_CONST_METHOD1(GetGroupIdAt, int(int index));
-  MOCK_METHOD2(GetIconAt, bool(int index, gfx::Image* icon));
-  MOCK_CONST_METHOD1(GetButtonMenuItemAt, ui::ButtonMenuItemModel*(int index));
-  MOCK_CONST_METHOD1(IsEnabledAt, bool(int index));
-  MOCK_CONST_METHOD1(IsVisibleAt, bool(int index));
-  MOCK_CONST_METHOD1(GetSubmenuModelAt, MenuModel*(int index));
-  MOCK_METHOD1(HighlightChangedTo, void(int index));
-  MOCK_METHOD1(ActivatedAt, void(int index));
-  MOCK_METHOD2(ActivatedAt, void(int index, int disposition));
-  MOCK_METHOD0(MenuWillShow, void());
-  MOCK_METHOD0(MenuClosed, void());
-  MOCK_METHOD1(SetMenuModelDelegate, void(ui::MenuModelDelegate* delegate));
-  MOCK_CONST_METHOD0(GetMenuModelDelegate, ui::MenuModelDelegate*());
-  MOCK_METHOD3(GetModelAndIndexForCommandId, bool(int command_id,
-      MenuModel** model, int* index));
-};
-
-class TestDialog : public DialogDelegateView, public ButtonListener {
- public:
-  explicit TestDialog(MockMenuModel* mock_menu_model)
-      : button1_(NULL),
-        button2_(NULL),
-        checkbox_(NULL),
-        button_drop_(NULL),
-        last_pressed_button_(NULL),
-        mock_menu_model_(mock_menu_model),
-        canceled_(false),
-        oked_(false),
-        closeable_(false) {
-    button1_ = new LabelButton(this, ASCIIToUTF16("Button1"));
-    button2_ = new LabelButton(this, ASCIIToUTF16("Button2"));
-    checkbox_ = new Checkbox(ASCIIToUTF16("My checkbox"));
-    button_drop_ = new ButtonDropDown(this, mock_menu_model_);
-    AddChildView(button1_);
-    AddChildView(button2_);
-    AddChildView(checkbox_);
-    AddChildView(button_drop_);
-  }
-
-  virtual ~TestDialog() {}
-
-  void TearDown() {
-    // Now we can close safely.
-    closeable_ = true;
-    GetWidget()->Close();
-  }
-
-  // Prevent the dialog from really closing (so we can click the OK/Cancel
-  // buttons to our heart's content).
-  virtual bool Cancel() OVERRIDE {
-    canceled_ = true;
-    return closeable_;
-  }
-  virtual bool Accept() OVERRIDE {
-    oked_ = true;
-    return closeable_;
-  }
-
-  // ButtonListener implementation.
-  virtual void ButtonPressed(Button* sender, const ui::Event& event) OVERRIDE {
-    last_pressed_button_ = sender;
-  }
-
-  void ResetStates() {
-    oked_ = false;
-    canceled_ = false;
-    last_pressed_button_ = NULL;
-  }
-
-  // Set up expectations for methods that are called when an (empty) menu is
-  // shown from a drop down button.
-  void ExpectShowDropMenu() {
-    if (mock_menu_model_) {
-      EXPECT_CALL(*mock_menu_model_, HasIcons());
-      EXPECT_CALL(*mock_menu_model_, GetItemCount());
-      EXPECT_CALL(*mock_menu_model_, MenuClosed());
-    }
-  }
-
-  LabelButton* button1_;
-  LabelButton* button2_;
-  Checkbox* checkbox_;
-  ButtonDropDown* button_drop_;
-  Button* last_pressed_button_;
-  MockMenuModel* mock_menu_model_;
-
-  bool canceled_;
-  bool oked_;
-  bool closeable_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestDialog);
-};
-
-class DefaultButtonTest : public ViewTest {
- public:
-  enum ButtonID { OK, CANCEL, BUTTON1, BUTTON2, };
-
-  DefaultButtonTest() : dialog_(NULL) {}
-  virtual ~DefaultButtonTest() {}
-
-  virtual void SetUp() OVERRIDE {
-    ViewTest::SetUp();
-    dialog_ = new TestDialog(NULL);
-    DialogDelegate::CreateDialogWidget(dialog_, GetContext(), NULL)->Show();
-  }
-
-  virtual void TearDown() OVERRIDE {
-    dialog_->TearDown();
-    ViewTest::TearDown();
-  }
-
- protected:
-  void SimulatePressingEnterAndCheckDefaultButton(ButtonID button_id) {
-    ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_RETURN, 0, false);
-    dialog_->GetFocusManager()->OnKeyEvent(event);
-    switch (button_id) {
-      case OK:
-        EXPECT_TRUE(dialog_->oked_);
-        EXPECT_FALSE(dialog_->canceled_);
-        EXPECT_FALSE(dialog_->last_pressed_button_);
-        break;
-      case CANCEL:
-        EXPECT_FALSE(dialog_->oked_);
-        EXPECT_TRUE(dialog_->canceled_);
-        EXPECT_FALSE(dialog_->last_pressed_button_);
-        break;
-      case BUTTON1:
-        EXPECT_FALSE(dialog_->oked_);
-        EXPECT_FALSE(dialog_->canceled_);
-        EXPECT_TRUE(dialog_->last_pressed_button_ == dialog_->button1_);
-        break;
-      case BUTTON2:
-        EXPECT_FALSE(dialog_->oked_);
-        EXPECT_FALSE(dialog_->canceled_);
-        EXPECT_TRUE(dialog_->last_pressed_button_ == dialog_->button2_);
-        break;
-    }
-    dialog_->ResetStates();
-  }
-
-  TestDialog* dialog_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(DefaultButtonTest);
-};
-
-TEST_F(DefaultButtonTest, DialogDefaultButtonTest) {
-  DialogClientView* client_view = dialog_->GetDialogClientView();
-  LabelButton* ok_button = client_view->ok_button();
-  LabelButton* cancel_button = client_view->cancel_button();
-
-  // Window has just been shown, we expect the default button specified in the
-  // DialogDelegate.
-  EXPECT_TRUE(ok_button->is_default());
-
-  // Simulate pressing enter, that should trigger the OK button.
-  SimulatePressingEnterAndCheckDefaultButton(OK);
-
-  // Simulate focusing another button, it should become the default button.
-  client_view->OnWillChangeFocus(ok_button, dialog_->button1_);
-  EXPECT_FALSE(ok_button->is_default());
-  EXPECT_TRUE(dialog_->button1_->is_default());
-  // Simulate pressing enter, that should trigger button1.
-  SimulatePressingEnterAndCheckDefaultButton(BUTTON1);
-
-  // Now select something that is not a button, the OK should become the default
-  // button again.
-  client_view->OnWillChangeFocus(dialog_->button1_, dialog_->checkbox_);
-  EXPECT_TRUE(ok_button->is_default());
-  EXPECT_FALSE(dialog_->button1_->is_default());
-  SimulatePressingEnterAndCheckDefaultButton(OK);
-
-  // Select yet another button.
-  client_view->OnWillChangeFocus(dialog_->checkbox_, dialog_->button2_);
-  EXPECT_FALSE(ok_button->is_default());
-  EXPECT_FALSE(dialog_->button1_->is_default());
-  EXPECT_TRUE(dialog_->button2_->is_default());
-  SimulatePressingEnterAndCheckDefaultButton(BUTTON2);
-
-  // Focus nothing.
-  client_view->OnWillChangeFocus(dialog_->button2_, NULL);
-  EXPECT_TRUE(ok_button->is_default());
-  EXPECT_FALSE(dialog_->button1_->is_default());
-  EXPECT_FALSE(dialog_->button2_->is_default());
-  SimulatePressingEnterAndCheckDefaultButton(OK);
-
-  // Focus the cancel button.
-  client_view->OnWillChangeFocus(NULL, cancel_button);
-  EXPECT_FALSE(ok_button->is_default());
-  EXPECT_TRUE(cancel_button->is_default());
-  EXPECT_FALSE(dialog_->button1_->is_default());
-  EXPECT_FALSE(dialog_->button2_->is_default());
-  SimulatePressingEnterAndCheckDefaultButton(CANCEL);
-}
-
-class ButtonDropDownTest : public ViewTest {
- public:
-  ButtonDropDownTest() : dialog_(NULL), button_as_view_(NULL) {}
-  virtual ~ButtonDropDownTest() {}
-
-  virtual void SetUp() OVERRIDE {
-    ViewTest::SetUp();
-    dialog_ = new TestDialog(new MockMenuModel());
-    DialogDelegate::CreateDialogWidget(dialog_, GetContext(), NULL)->Show();
-    dialog_->button_drop_->SetBoundsRect(gfx::Rect(0, 0, 100, 100));
-    // We have to cast the button back into a View in order to invoke it's
-    // OnMouseReleased method.
-    button_as_view_ = static_cast<View*>(dialog_->button_drop_);
-  }
-
-  virtual void TearDown() OVERRIDE {
-    dialog_->TearDown();
-    ViewTest::TearDown();
-  }
-
-  TestDialog* dialog_;
-  // This is owned by dialog_.
-  View* button_as_view_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ButtonDropDownTest);
-};
-
-// Ensure that regular clicks on the drop down button still work. (i.e. - the
-// click events are processed and the listener gets the click)
-TEST_F(ButtonDropDownTest, RegularClickTest) {
-  gfx::Point point(1, 1);
-  ui::MouseEvent press_event(ui::ET_MOUSE_PRESSED, point, point,
-                             ui::EF_LEFT_MOUSE_BUTTON);
-  ui::MouseEvent release_event(ui::ET_MOUSE_RELEASED, point, point,
-                               ui::EF_LEFT_MOUSE_BUTTON);
-  button_as_view_->OnMousePressed(press_event);
-  button_as_view_->OnMouseReleased(release_event);
-  EXPECT_EQ(dialog_->last_pressed_button_, dialog_->button_drop_);
-}
-
-////////////////////////////////////////////////////////////////////////////////
 // Native view hierachy
 ////////////////////////////////////////////////////////////////////////////////
 class TestNativeViewHierarchy : public View {
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc
index 08d2cfc..abffb5a 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -43,9 +43,6 @@ DialogClientView::DialogClientView(Widget* owner, View* contents_view)
 }
 
 DialogClientView::~DialogClientView() {
-  if (focus_manager_)
-    focus_manager_->RemoveFocusChangeListener(this);
-  focus_manager_ = NULL;
 }
 
 void DialogClientView::AcceptWindow() {
@@ -73,7 +70,7 @@ void DialogClientView::UpdateDialogButtons() {
   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_);
     }
@@ -272,6 +269,17 @@ void DialogClientView::ViewHierarchyChanged(
     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;
   }
 }
 
diff --git a/ui/views/window/dialog_client_view_unittest.cc b/ui/views/window/dialog_client_view_unittest.cc
index e82d1ec..9f8c8c4 100644
--- a/ui/views/window/dialog_client_view_unittest.cc
+++ b/ui/views/window/dialog_client_view_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/ui_base_types.h"
+#include "ui/views/controls/button/label_button.h"
 #include "ui/views/test/test_views.h"
 #include "ui/views/window/dialog_client_view.h"
 #include "ui/views/window/dialog_delegate.h"
@@ -15,19 +16,15 @@ namespace views {
 class TestDialogClientView : public DialogClientView {
  public:
   TestDialogClientView(View* contents_view,
-                       DialogDelegate* delegate)
+                       DialogDelegate* dialog_delegate)
       : DialogClientView(contents_view),
-        delegate_(delegate) {}
+        dialog_(dialog_delegate) {}
   virtual ~TestDialogClientView() {}
 
   // DialogClientView implementation.
-  virtual DialogDelegate* GetDialogDelegate() const OVERRIDE {
-    return delegate_;
-  }
+  virtual DialogDelegate* GetDialogDelegate() const OVERRIDE { return dialog_; }
 
-  View* GetContentsView() {
-    return contents_view();
-  }
+  View* GetContentsView() { return contents_view(); }
 
   void CreateExtraViews() {
     CreateExtraView();
@@ -35,7 +32,7 @@ class TestDialogClientView : public DialogClientView {
   }
 
  private:
-  DialogDelegate* delegate_;
+  DialogDelegate* dialog_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDialogClientView);
 };
@@ -57,35 +54,27 @@ class DialogClientViewTest : public testing::Test,
   }
 
   // DialogDelegateView implementation.
-  virtual View* GetContentsView() OVERRIDE {
-    return contents_.get();
-  }
-  virtual View* CreateExtraView() OVERRIDE {
-    return extra_view_;
-  }
-  virtual View* CreateFootnoteView() OVERRIDE {
-    return footnote_view_;
-  }
-  virtual int GetDialogButtons() const OVERRIDE {
-    return dialog_buttons_;
-  }
+  virtual View* GetContentsView() OVERRIDE { return contents_.get(); }
+  virtual View* CreateExtraView() OVERRIDE { return extra_view_; }
+  virtual View* CreateFootnoteView() OVERRIDE { return footnote_view_; }
+  virtual int GetDialogButtons() const OVERRIDE { return dialog_buttons_; }
 
  protected:
-  void ResizeAndLayoutClientView() {
+  gfx::Rect GetUpdatedClientBounds() {
     client_view_->SizeToPreferredSize();
     client_view_->Layout();
+    return client_view_->bounds();
   }
 
-  // Maes sure that the content view is sized correctly. Width must be at least
+  // Makes sure that the content view is sized correctly. Width must be at least
   // the requested amount, but height should always match exactly.
   void CheckContentsIsSetToPreferredSize() {
-    ResizeAndLayoutClientView();
-    gfx::Size preferred_size = contents_->GetPreferredSize();
+    const gfx::Rect client_bounds = GetUpdatedClientBounds();
+    const gfx::Size preferred_size = contents_->GetPreferredSize();
     EXPECT_EQ(preferred_size.height(), contents_->bounds().height());
     EXPECT_LE(preferred_size.width(), contents_->bounds().width());
-    EXPECT_EQ(contents_->bounds().x(), client_view()->bounds().x());
-    EXPECT_EQ(contents_->bounds().y(), client_view()->bounds().y());
-    EXPECT_EQ(contents_->bounds().right(), client_view()->bounds().right());
+    EXPECT_EQ(contents_->bounds().origin(), client_bounds.origin());
+    EXPECT_EQ(contents_->bounds().right(), client_bounds.right());
   }
 
   // Sets the buttons to show in the dialog and refreshes the dialog.
@@ -123,13 +112,51 @@ class DialogClientViewTest : public testing::Test,
   DISALLOW_COPY_AND_ASSIGN(DialogClientViewTest);
 };
 
-TEST_F(DialogClientViewTest, ButtonStateCanChange) {
-  ResizeAndLayoutClientView();
-  int no_button_height = client_view()->bounds().height();
+TEST_F(DialogClientViewTest, UpdateButtons) {
+  // This dialog should start with no buttons.
+  EXPECT_EQ(GetDialogButtons(), ui::DIALOG_BUTTON_NONE);
+  EXPECT_EQ(NULL, client_view()->ok_button());
+  EXPECT_EQ(NULL, client_view()->cancel_button());
+  const int height_without_buttons = GetUpdatedClientBounds().height();
+
+  // Update to use both buttons.
+  SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
+  EXPECT_TRUE(client_view()->ok_button()->is_default());
+  EXPECT_FALSE(client_view()->cancel_button()->is_default());
+  const int height_with_buttons = GetUpdatedClientBounds().height();
+  EXPECT_GT(height_with_buttons, height_without_buttons);
+
+  // Remove the dialog buttons.
+  SetDialogButtons(ui::DIALOG_BUTTON_NONE);
+  EXPECT_EQ(NULL, client_view()->ok_button());
+  EXPECT_EQ(NULL, client_view()->cancel_button());
+  EXPECT_EQ(GetUpdatedClientBounds().height(), height_without_buttons);
+
+  // Reset with just an ok button.
+  SetDialogButtons(ui::DIALOG_BUTTON_OK);
+  EXPECT_TRUE(client_view()->ok_button()->is_default());
+  EXPECT_EQ(NULL, client_view()->cancel_button());
+  EXPECT_EQ(GetUpdatedClientBounds().height(), height_with_buttons);
+
+  // Reset with just a cancel button.
+  SetDialogButtons(ui::DIALOG_BUTTON_CANCEL);
+  EXPECT_EQ(NULL, client_view()->ok_button());
+  EXPECT_TRUE(client_view()->cancel_button()->is_default());
+  EXPECT_EQ(GetUpdatedClientBounds().height(), height_with_buttons);
+}
+
+TEST_F(DialogClientViewTest, RemoveAndUpdateButtons) {
+  // Removing buttons from another context should clear the local pointer.
+  SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
+  delete client_view()->ok_button();
+  EXPECT_EQ(NULL, client_view()->ok_button());
+  delete client_view()->cancel_button();
+  EXPECT_EQ(NULL, client_view()->cancel_button());
 
+  // Updating should restore the requested buttons properly.
   SetDialogButtons(ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL);
-  ResizeAndLayoutClientView();
-  EXPECT_GT(client_view()->bounds().height(), no_button_height);
+  EXPECT_TRUE(client_view()->ok_button()->is_default());
+  EXPECT_FALSE(client_view()->cancel_button()->is_default());
 }
 
 // Test that the contents view gets its preferred size in the basic dialog
diff --git a/ui/views/window/dialog_delegate_unittest.cc b/ui/views/window/dialog_delegate_unittest.cc
index 2987919..d7471e7 100644
--- a/ui/views/window/dialog_delegate_unittest.cc
+++ b/ui/views/window/dialog_delegate_unittest.cc
@@ -2,37 +2,205 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/strings/utf_string_conversions.h"
 #include "ui/base/hit_test.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/bubble/bubble_frame_view.h"
+#include "ui/views/controls/button/button_dropdown.h"
+#include "ui/views/controls/button/checkbox.h"
+#include "ui/views/controls/button/label_button.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_client_view.h"
 #include "ui/views/window/dialog_delegate.h"
 
 namespace views {
 
-typedef ViewsTestBase DialogTest;
-
 namespace {
 
-class TestDialog : public DialogDelegateView {
+class TestDialog : public DialogDelegateView, public ButtonListener {
  public:
-  TestDialog() {}
+  TestDialog()
+      : canceled_(false),
+        accepted_(false),
+        closeable_(false),
+        last_pressed_button_(NULL) {}
   virtual ~TestDialog() {}
 
-  // BubbleDelegateView overrides:
+  // DialogDelegateView overrides:
+  virtual bool Cancel() OVERRIDE {
+    canceled_ = true;
+    return closeable_;
+  }
+  virtual bool Accept() OVERRIDE {
+    accepted_ = true;
+    return closeable_;
+  }
+
+  // View override:
   virtual gfx::Size GetPreferredSize() OVERRIDE { return gfx::Size(200, 200); }
 
+  // ButtonListener override:
+  virtual void ButtonPressed(Button* sender, const ui::Event& event) OVERRIDE {
+    last_pressed_button_ = sender;
+  }
+
+  Button* last_pressed_button() const { return last_pressed_button_; }
+
+  void PressEnterAndCheckStates(Button* button) {
+    ui::KeyEvent key_event(ui::ET_KEY_PRESSED, ui::VKEY_RETURN, 0, false);
+    GetFocusManager()->OnKeyEvent(key_event);
+    const DialogClientView* client_view = GetDialogClientView();
+    EXPECT_EQ(canceled_, client_view->cancel_button()->is_default());
+    EXPECT_EQ(accepted_, client_view->ok_button()->is_default());
+    // This view does not listen for ok or cancel clicks, DialogClientView does.
+    CheckAndResetStates(button == client_view->cancel_button(),
+                        button == client_view->ok_button(),
+                        (canceled_ || accepted_ ) ? NULL : button);
+  }
+
+  void CheckAndResetStates(bool canceled, bool accepted, Button* last_pressed) {
+    EXPECT_EQ(canceled, canceled_);
+    canceled_ = false;
+    EXPECT_EQ(accepted, accepted_);
+    accepted_ = false;
+    EXPECT_EQ(last_pressed, last_pressed_button_);
+    last_pressed_button_ = NULL;
+  }
+
+  void TearDown() {
+    closeable_ = true;
+    GetWidget()->Close();
+  }
+
  private:
+  bool canceled_;
+  bool accepted_;
+  // Prevent the dialog from closing, for repeated ok and cancel button clicks.
+  bool closeable_;
+  Button* last_pressed_button_;
+
   DISALLOW_COPY_AND_ASSIGN(TestDialog);
 };
 
+class DialogTest : public ViewsTestBase {
+ public:
+  DialogTest() : dialog_(NULL) {}
+  virtual ~DialogTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    ViewsTestBase::SetUp();
+    dialog_ = new TestDialog();
+    DialogDelegate::CreateDialogWidget(dialog_, GetContext(), NULL)->Show();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    dialog_->TearDown();
+    ViewsTestBase::TearDown();
+  }
+
+  TestDialog* dialog() const { return dialog_; }
+
+ private:
+  TestDialog* dialog_;
+
+  DISALLOW_COPY_AND_ASSIGN(DialogTest);
+};
+
 }  // namespace
 
+TEST_F(DialogTest, DefaultButtons) {
+  DialogClientView* client_view = dialog()->GetDialogClientView();
+  LabelButton* ok_button = client_view->ok_button();
+  LabelButton* cancel_button = client_view->cancel_button();
+
+  // DialogDelegate's default button (ok) should be default (and handle enter).
+  EXPECT_EQ(ui::DIALOG_BUTTON_OK, dialog()->GetDefaultDialogButton());
+  dialog()->PressEnterAndCheckStates(ok_button);
+
+  // Focus another button in the dialog, it should become the default.
+  LabelButton* button_1 = new LabelButton(dialog(), string16());
+  client_view->AddChildView(button_1);
+  client_view->OnWillChangeFocus(ok_button, button_1);
+  EXPECT_TRUE(button_1->is_default());
+  dialog()->PressEnterAndCheckStates(button_1);
+
+  // Focus a Checkbox (not a push button), OK should become the default again.
+  Checkbox* checkbox = new Checkbox(string16());
+  client_view->AddChildView(checkbox);
+  client_view->OnWillChangeFocus(button_1, checkbox);
+  EXPECT_FALSE(button_1->is_default());
+  dialog()->PressEnterAndCheckStates(ok_button);
+
+  // Focus yet another button in the dialog, it should become the default.
+  LabelButton* button_2 = new LabelButton(dialog(), string16());
+  client_view->AddChildView(button_2);
+  client_view->OnWillChangeFocus(checkbox, button_2);
+  EXPECT_FALSE(button_1->is_default());
+  EXPECT_TRUE(button_2->is_default());
+  dialog()->PressEnterAndCheckStates(button_2);
+
+  // Focus nothing, OK should become the default again.
+  client_view->OnWillChangeFocus(button_2, NULL);
+  EXPECT_FALSE(button_1->is_default());
+  EXPECT_FALSE(button_2->is_default());
+  dialog()->PressEnterAndCheckStates(ok_button);
+
+  // A ButtonDropDown will handle events, but ok will be still be default.
+  ButtonDropDown* drop_down = new ButtonDropDown(dialog(), NULL);
+  dialog()->AddChildView(drop_down);
+  drop_down->SetBoundsRect(gfx::Rect(0, 0, 100, 100));
+  const gfx::Point point(1, 1);
+  client_view->OnWillChangeFocus(NULL, drop_down);
+  drop_down->OnMousePressed(ui::MouseEvent(
+      ui::ET_MOUSE_PRESSED, point, point, ui::EF_LEFT_MOUSE_BUTTON));
+  drop_down->OnMouseReleased(ui::MouseEvent(
+      ui::ET_MOUSE_RELEASED, point, point, ui::EF_LEFT_MOUSE_BUTTON));
+  dialog()->CheckAndResetStates(false, false, drop_down);
+  EXPECT_FALSE(button_1->is_default());
+  EXPECT_FALSE(button_2->is_default());
+  dialog()->PressEnterAndCheckStates(ok_button);
+
+  // Focus the cancel button, it should become the default.
+  client_view->OnWillChangeFocus(drop_down, cancel_button);
+  EXPECT_FALSE(button_1->is_default());
+  EXPECT_FALSE(button_2->is_default());
+  dialog()->PressEnterAndCheckStates(cancel_button);
+}
+
+TEST_F(DialogTest, AcceptAndCancel) {
+  DialogClientView* client_view = dialog()->GetDialogClientView();
+  LabelButton* ok_button = client_view->ok_button();
+  LabelButton* cancel_button = client_view->cancel_button();
+
+  // Check that return/escape accelerators accept/cancel dialogs.
+  const ui::KeyEvent return_key(ui::ET_KEY_PRESSED, ui::VKEY_RETURN, 0, false);
+  dialog()->GetFocusManager()->OnKeyEvent(return_key);
+  dialog()->CheckAndResetStates(false, true, NULL);
+  const ui::KeyEvent escape_key(ui::ET_KEY_PRESSED, ui::VKEY_ESCAPE, 0, false);
+  dialog()->GetFocusManager()->OnKeyEvent(escape_key);
+  dialog()->CheckAndResetStates(true, false, NULL);
+
+  // Check ok and cancel button behavior on a directed return key events.
+  ok_button->OnKeyPressed(return_key);
+  dialog()->CheckAndResetStates(false, true, NULL);
+  cancel_button->OnKeyPressed(return_key);
+  dialog()->CheckAndResetStates(true, false, NULL);
+
+  // Check that return accelerators cancel dialogs if cancel is focused.
+  cancel_button->RequestFocus();
+  dialog()->GetFocusManager()->OnKeyEvent(return_key);
+  dialog()->CheckAndResetStates(true, false, NULL);
+}
+
+TEST_F(DialogTest, RemoveDefaultButton) {
+  // Removing buttons from the dialog here should not cause a crash on close.
+  delete dialog()->GetDialogClientView()->ok_button();
+  delete dialog()->GetDialogClientView()->cancel_button();
+}
+
 TEST_F(DialogTest, HitTest) {
-  TestDialog* dialog = new TestDialog();
-  DialogDelegate::CreateDialogWidget(dialog, NULL, GetContext());
-  const NonClientView* view = dialog->GetWidget()->non_client_view();
+  const NonClientView* view = dialog()->GetWidget()->non_client_view();
 
   if (DialogDelegate::UseNewStyle()) {
     // Ensure that the new style's BubbleFrameView hit-tests as expected.