Call setNeedsRedraw when scrollbar opacity changes.

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index 15d72e6e3034b85f3020bf918d14201df3e99ac4..6bedee3bc7097685c11ff53f27a32d2f22eef1ab 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -2294,6 +2294,7 @@ LayerImpl* LayerTreeHostImpl::ViewportMainScrollLayer() {
 }
 
 void LayerTreeHostImpl::DidChangeScrollbarVisibility() {
+  SetNeedsRedraw();
   client_->SetNeedsCommitOnImplThread();

[weiliangc, 2017/03/07 22:53:34]

For setNeedsRedraw to work correctly we need to make sure damage is set.
Maybe it's possible to utilize how opacity animation works on effect tree?


Also I think maybe requesting a commit here is overkill?

[bokan, 2017/03/07 23:19:49]

I'm not sure what you mean about utilising animations on the opacity tree, could
you elaborate?

Is it possible the damage is already being set, since just adding this makes the
scrollbar show up now? Or are we defaulting to damaging everything by not
setting a region?
The commit is needed because we propagate the visibility back to Blink since all
event handling (i.e. clicking/dragging the scrollbars) is handled on the main
thread and so it needs to know when the scrollbars fade in/out on the
compositor.

[weiliangc, 2017/03/13 20:11:09]

I was wondering why when the opacity is changed and not the visibility, we would
be able to continue to animate.

Seems like we make call to
https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host_impl.cc?l=3650 in
other animation cases. This is called when animation is run but not on Show(),
and is probably needed whenever opacity of scrollbar changes.

My suggestion would be try to reduce the ScrollbarAnimationControllerClient
interface by calling directly of SetNeedsCommit when visibility change, and
always call SetNeedsRedraw on when opacity changes.
The damage for this is probably set when opacity on property tree is updated. 
 
Thanks for the explanation. Could you add this as comment?

[bokan, 2017/03/13 21:56:23]

I think code changed so your link is pointing to a different line than you
intended. Did you mean that we should call
LayerTreeHostImpl::SetNeedsRedrawForScrollbarAnimation()? I've moved that call
from RunAnimationFrame into ApplyOpacityToScrollbars so that any change in
opacity causes a redraw.

I don't think we can reduce the interface of ScrollbarAnimationControllerClient
since we can't directly call SetNeedsCommit (we don't have a direct pointer to
LTHI) but DidChangeScrollbarVisibility now only has the SetNeedsCommit call
Got it, thanks.
Done.

 }