Second try at fixing location bar icon keyboard activation.

chrome/browser/ui/views/location_bar/bubble_icon_view.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 "chrome/browser/ui/views/location_bar/bubble_icon_view.h"
 
 #include "chrome/browser/command_updater.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/events/event.h"
@@ skipping 80 matching lines @@
   if (!event.IsLeftMouseButton())
     return;
 
   const bool activated = HitTestPoint(event.location());
   ink_drop_delegate_->OnAction(
       activated ? views::InkDropState::ACTIVATED : views::InkDropState::HIDDEN);
   if (activated)
     ExecuteCommand(EXECUTE_SOURCE_MOUSE);
 }
 
-bool BubbleIconView::OnKeyReleased(const ui::KeyEvent& event) {
-  if (event.key_code() != ui::VKEY_SPACE && event.key_code() != ui::VKEY_RETURN)
+bool BubbleIconView::OnKeyPressed(const ui::KeyEvent& event) {
+  if (event.key_code() != ui::VKEY_RETURN && event.key_code() != ui::VKEY_SPACE)
     return false;
 
   ink_drop_delegate_->OnAction(views::InkDropState::ACTIVATED);
+  // As with CustomButton, return activates on key down and space activates on
+  // key up.

[Peter Kasting, 2016/05/04 02:21:00]

Shouldn't we change the ink drop OnAction() call to match when ExecuteCommand()
is called?

[Evan Stade, 2016/05/04 02:45:52]

We want to show the ripple on key down for both space and enter. If you press
enter, it will open the bubble and activate the ripple. If you press space but
don't release, it will show the ripple, then when you release it will show the
bubble. This matches how the pressed state works in pre-md.

That said, I think the toolbar navigation buttons need to behave this way as
well (currently space down has no visible effect on them).

[Peter Kasting, 2016/05/04 02:57:40]

o_O Seems, then, like the ripple is basically a "you pressed a key here" rather
than an indicator that the button was activated... so wouldn't we want to show
it on basically any press/keydown?

I guess I don't really understand the semantics of the ripple...
File a bug?

[Evan Stade, 2016/05/04 03:03:57]

Perhaps "something is happening or will happen soon"
done, crbug.com/608996

+  if (event.key_code() == ui::VKEY_RETURN)
+    ExecuteCommand(EXECUTE_SOURCE_KEYBOARD);
+  return true;
+}
+
+bool BubbleIconView::OnKeyReleased(const ui::KeyEvent& event) {
+  if (event.key_code() != ui::VKEY_SPACE)
+    return false;
+
   ExecuteCommand(EXECUTE_SOURCE_KEYBOARD);
   return true;
 }
 
 void BubbleIconView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   View::ViewHierarchyChanged(details);
   if (details.is_add && GetNativeTheme())
     UpdateIcon();
 }
@@ skipping 77 matching lines @@
   image_->SetImage(
       gfx::CreateVectorIcon(GetVectorIcon(), icon_size, icon_color));
 }
 
 void BubbleIconView::SetActiveInternal(bool active) {
   if (active_ == active)
     return;
   active_ = active;
   UpdateIcon();
 }