Hide hover effect on hidden ink drop host views.

ui/views/controls/button/custom_button.cc

Index: ui/views/controls/button/custom_button.cc
diff --git a/ui/views/controls/button/custom_button.cc b/ui/views/controls/button/custom_button.cc
index 62f048173c68b2de35b20602c71f909c42275b17..d3a8fa62341aa01924ae055844b7e9049779bb37 100644
--- a/ui/views/controls/button/custom_button.cc
+++ b/ui/views/controls/button/custom_button.cc
@@ -130,13 +130,13 @@ void CustomButton::OnEnabledChanged() {
   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 {
@@ -349,6 +349,7 @@ void CustomButton::GetAccessibleState(ui::AXViewState* state) {
 }
 
 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);