Resets hover state when ToolbarActionView is dragged

chrome/browser/ui/views/toolbar/toolbar_action_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/toolbar/toolbar_action_view.h"
 
 #include <string>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ skipping 181 matching lines @@
 void ToolbarActionView::set_context_menu_callback_for_testing(
     base::Callback<void(ToolbarActionView*)>* callback) {
   context_menu_callback_for_test = callback;
 }
 
 gfx::Size ToolbarActionView::GetPreferredSize() const {
   return gfx::Size(ToolbarActionsBar::IconWidth(false),
                    ToolbarActionsBar::IconHeight());
 }
 
+void ToolbarActionView::OnMouseCaptureLost() {

[Devlin, 2016/03/01 15:00:59]

Why is this in ToolbarActionView instead of a button class?  It looks like
CustomButton has an implementation of OnMouseCaptureLost that does this for
everything *except* dragged views (because starting a drag results in capture
lost)... and I thought that was intentional.  Don't we want the view to remain
"pressed" when we are dragging it around?

[varkha, 2016/03/01 16:32:31]

Yes, I thought so too but wasn't sure if there is more to it. I think with MD
the hovered or pressed state looks bad during a drag, particularly in the app
menu. estade@ pointed this out and we thought it would be OK to remove this
everywhere. I am open to removing InDrag() check from
CustomButton::OnMouseCaptureLost. I think during a drag we give enough feedback
with the drag widget and insertion point to make the pressed / hovered state of
the drag target redundant.

[Evan Stade, 2016/03/01 21:01:17]

The problem is that MD ink drops (which provide hover and pressed effects) look
substantially different from the pre-MD pressed state. So mixing and matching
these (i.e. using ink drops for hover and press, but reverting to pre-md press
effect for drag) looks bad. That's why I suggested just removing it. I think the
change should be MD-only to be conservative.

Does this mean we don't want any effect at all on the dragged button in the
world of MD? That's a question for Sebastien and something to fix down the line
based on his answer. But I'm pretty sure no effect is better than what we have
now.

As far as moving this to CustomButton, that would make sense and it would make
the bookmark bar buttons consistent with the toolbar action buttons... but not
all custom buttons are updated to used ink drops yet so I'd be careful about
determining if some draggable button somewhere were losing this pressed state
despite not having an inkdrop applied to it yet.

+  MenuButton::OnMouseCaptureLost();
+  if (IsHotTracked())
+    SetState(STATE_NORMAL);
+}
+
 bool ToolbarActionView::OnMousePressed(const ui::MouseEvent& event) {
   // views::MenuButton actions are only triggered by left mouse clicks.
   if (event.IsOnlyLeftMouseButton()) {
     // TODO(bruthig): The ACTION_PENDING triggering logic should be in
     // MenuButton::OnPressed() however there is a bug with the pressed state
     // logic in MenuButton. See http://crbug.com/567252.
     ink_drop_delegate()->OnAction(views::InkDropState::ACTION_PENDING);
   }
   return MenuButton::OnMousePressed(event);
 }
@@ skipping 122 matching lines @@
     if (menu_controller->in_nested_run()) {
       // There is another menu showing. Close the outermost menu (since we are
       // shown in the same menu, we don't want to close the whole thing).
       menu_controller->Cancel(views::MenuController::EXIT_OUTERMOST);
       return true;
     }
   }
 
   return false;
 }