Hide hover effect on hidden ink drop host views.

ui/views/controls/button/custom_button.cc

 // Copyright (c) 2012 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 "ui/views/controls/button/custom_button.h"
 
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/events/event.h"
 #include "ui/events/event_utils.h"
@@ skipping 112 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // CustomButton, View overrides:
 
 void CustomButton::OnEnabledChanged() {
   // TODO(bruthig): Is there any reason we are not calling
   // Button::OnEnabledChanged() here?
   if (enabled() ? (state_ != STATE_DISABLED) : (state_ == STATE_DISABLED))
     return;
 
-  if (enabled())
+  if (enabled()) {
     SetState(ShouldEnterHoveredState() ? STATE_HOVERED : STATE_NORMAL);
-  else
+    if (ink_drop_delegate())
+      ink_drop_delegate()->SetHovered(InDrag() && IsMouseHovered());

[bruthig, 2016/06/03 14:30:50]

Shouldn't this be !InDrag() ?

IMO it would be easier to maintain if you didn't duplicate the "is hovered"
logic here and in ShouldShowInkDropHighlight(). 
i.e.
bool ShouldShowInkDropHighlight() const {
  return IsHovered() || IsFocused();
}

[Evan Stade, 2016/06/03 17:55:59]

fixed
I don't really think that makes sense. How would you write IsHovered?

bool IsHovered() {
  return IsMouseHovered() && !InDrag();
}

Question: why do we care about InDrag when ShouldEnterHoveredState doesn't seem
to?

[bruthig, 2016/06/03 19:42:36]

Yeah.
ShouldEnterHoveredState() probably doesn't need to check the drag state because
ButtonState::HOVERED and ButtonState::PRESSED are mutually exclusive and drag
operations, AFAIK, require a button to be in a PRESSED state. i.e. a hover state
never needs to be cleared/prevented while in a drag.
Whereas, when an InkDrop animates to HIDDEN at the start of a drag operation, we
don't want the hover to fade back in during the drag, which may happen when
InkDropImpl::HoverAfterRippleTimerFired() -> ... ->
InkDropHost::CreateInkDropHover().

[Evan Stade, 2016/06/03 21:53:23]

But then IsHovered is different from IsMouseHovered which is unintuitive.
Actually I think we should be reusing ShouldEnterHoveredState() here (updated).
If the test of whether this works is dragging a bookmark bar button or folder
and seeing if the hover ever fades back in, this seems to work.

[bruthig, 2016/06/03 22:35:25]

I'd be more concerned if it were the same as it would be completely duplicated
code ;)
Ok, so would it be accurate to say ShouldEnterHoveredState() would track the
logical hovered state and ShouldShowInkDropHighlight() would track the visual
hover/focus state?  I could buy in to that.

+  } else {
     SetState(STATE_DISABLED);
-
-  if (ink_drop_delegate())
-    ink_drop_delegate()->SetHovered(ShouldShowInkDropHover());
+  }
 }
 
 const char* CustomButton::GetClassName() const {
   return kViewClassName;
 }
 
 bool CustomButton::OnMousePressed(const ui::MouseEvent& event) {
   if (state_ == STATE_DISABLED)
     return true;
   if (state_ != STATE_PRESSED && ShouldEnterPushedState(event) &&
@@ skipping 192 matching lines @@
       state->AddStateFlag(ui::AX_STATE_DISABLED);
       break;
     case STATE_NORMAL:
     case STATE_COUNT:
       // No additional accessibility state set for this button state.
       break;
   }
 }
 
 void CustomButton::VisibilityChanged(View* starting_from, bool visible) {
+  Button::VisibilityChanged(starting_from, visible);
   if (state_ == STATE_DISABLED)
     return;
   SetState(visible && ShouldEnterHoveredState() ? STATE_HOVERED : STATE_NORMAL);
 }
 
 std::unique_ptr<InkDropHover> CustomButton::CreateInkDropHover() const {
   return ShouldShowInkDropHover() ? Button::CreateInkDropHover() : nullptr;
 }
 
 SkColor CustomButton::GetInkDropBaseColor() const {
@@ skipping 106 matching lines @@
   Button::NotifyClick(event);
 }
 
 void CustomButton::OnClickCanceled(const ui::Event& event) {
   if (ink_drop_delegate())
     ink_drop_delegate()->OnAction(views::InkDropState::HIDDEN);
   Button::OnClickCanceled(event);
 }
 
 }  // namespace views