MacViews: Handle Space and Return keys correctly for Buttons.

ui/views/window/dialog_delegate_unittest.cc

 // Copyright 2013 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 <stddef.h>
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/base/hit_test.h"
 #include "ui/events/event_processor.h"
@@ skipping 124 matching lines @@
  private:
   TestDialog* dialog_;
 
   DISALLOW_COPY_AND_ASSIGN(DialogTest);
 };
 
 }  // namespace
 
 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/close dialogs.
   EXPECT_EQ(dialog()->input(), dialog()->GetFocusManager()->GetFocusedView());
   const ui::KeyEvent return_event(
       ui::ET_KEY_PRESSED, ui::VKEY_RETURN, ui::EF_NONE);
   SimulateKeyEvent(return_event);
   dialog()->CheckAndResetStates(false, true, false);
   const ui::KeyEvent escape_event(
       ui::ET_KEY_PRESSED, ui::VKEY_ESCAPE, ui::EF_NONE);
   SimulateKeyEvent(escape_event);
   dialog()->CheckAndResetStates(false, false, true);
 
-  // Check ok and cancel button behavior on a directed return key events.
-  ok_button->OnKeyPressed(return_event);
-  dialog()->CheckAndResetStates(false, true, false);
-  cancel_button->OnKeyPressed(return_event);
-  dialog()->CheckAndResetStates(true, false, false);
-
-  // Check that return accelerators cancel dialogs if cancel is focused.
+  // Check that return accelerators cancel dialogs if cancel is focused, except
+  // on Mac where return should perform the default action.
+  LabelButton* cancel_button = client_view->cancel_button();

[tapted, 2017/01/04 04:54:31]

can we keep the old tests as well? (on non-mac at least) - did they start
failing (why?)

[karandeepb, 2017/01/04 06:14:45]

Done. 
I didn't think these tests were appropriate here. These test button behavior on
OnKeyPressed for a return event, which is better tested at the CustomButton
level.

   cancel_button->RequestFocus();
   EXPECT_EQ(cancel_button, dialog()->GetFocusManager()->GetFocusedView());
   SimulateKeyEvent(return_event);
+#if defined(OS_MACOSX)
+  dialog()->CheckAndResetStates(false, true, false);
+#else
   dialog()->CheckAndResetStates(true, false, false);
+#endif
 
   // Check that escape can be overridden.
   dialog()->set_should_handle_escape(true);
   SimulateKeyEvent(escape_event);
   dialog()->CheckAndResetStates(false, false, false);
 }
 
 TEST_F(DialogTest, RemoveDefaultButton) {
   // Removing buttons from the dialog here should not cause a crash on close.
   delete dialog()->GetDialogClientView()->ok_button();
@@ skipping 73 matching lines @@
   dialog2->TearDown();
 }
 
 // Tests default focus is assigned correctly when showing a new dialog.
 TEST_F(DialogTest, InitialFocus) {
   EXPECT_TRUE(dialog()->input()->HasFocus());
   EXPECT_EQ(dialog()->input(), dialog()->GetFocusManager()->GetFocusedView());
 }
 
 }  // namespace views