MacViews: Handle Space and Return keys correctly for Buttons.

ui/views/style/platform_style.h

 // Copyright 2015 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.
 
 #ifndef UI_VIEWS_STYLE_PLATFORM_STYLE_H_
 #define UI_VIEWS_STYLE_PLATFORM_STYLE_H_
 
 #include <memory>
 
 #include "base/macros.h"
@@ skipping 28 matching lines @@
 
   // Whether the default button for a dialog can be the Cancel button.
   static const bool kDialogDefaultButtonCanBeCancel;
 
   // Whether right clicking on text, selects the word under cursor.
   static const bool kSelectWordOnRightClick;
 
   // The menu button's action to show the menu.
   static const CustomButton::NotifyAction kMenuNotifyActivationAction;
 
+  // Action to perform on a button when the Space key is pressed.
+  static const CustomButton::ButtonAction kButtonActionOnSpace;
+
+  // Whether Return key clicks the focused control.
+  static const bool kClickControlOnReturn;

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

I read "Control" and thought "Control+Return" - i.e. a key combination.. But do
we need two constants? They seem to imply each other. e.g. can we have

// Whether buttons react only to the Space key (when pressed). Otherwise buttons
// react to Return when pressed, and Space when released.
static const bool kButtonsClickOnSpaceOnly = is_mac;

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

Is kReturnClicksFocusedControl better? 
 
Two constants seem better to me. We are describing two separate behaviors here- 
whether to click a button on space-press/space-release, and whether we should
click a control on pressing the Return key (This will end up being used for
other views as well, for eg. see https://codereview.chromium.org/2609943004/). 

[tapted, 2017/01/05 00:34:41]

Hm. I see your point. I was thinking of it in terms of a platform having one
"primary" key for activating, which always activates on key press (mac=space,
win=return). Then, if the primary key isn't space, space activates on release.
This is more "one-dimensional".

There's a downside to having them separate -- it implies there exists a platform
configuration that doesn't change them as a pair. But I guess you can say that
about lots of things here. This is OK, but I think we can improve the
documentation (see later comment)

+
   // Whether TreeViews get a focus ring on the entire TreeView when focused.
   static const bool kTreeViewHasFocusRing;
 
   // Whether selecting a row in a TreeView selects the entire row or only the
   // label for that row.
   static const bool kTreeViewSelectionPaintsEntireRow;
 
   // Whether ripples should be used for visual feedback on control activation.
   static const bool kUseRipples;
 
@@ skipping 29 matching lines @@
   // the failed edit if platform-appropriate.
   static void OnTextfieldEditFailed();
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(PlatformStyle);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_STYLE_PLATFORM_STYLE_H_