Overlay scrollbars are painted onload.

Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range. Now, when it is called during LayerImpl::PushPropertiesTo, the temporarily invalid scrollbar->Opacity() doesn't set the opacity to zero.

The tests fails before applying this patch, and passes on this patch.

BUG=635629
TEST=ScrollbarAnimationControllerThinningTest.AppearOnResize
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/38991f06ebb579440a3bf3f3fbb8792a60b610d4
Cr-Commit-Position: refs/heads/master@{#421910}

[sahel, 2016-09-15 22:26:08]

Description was changed from

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range.
Now, when it is called during LayerImpl::PushPropertiesTo, the
temprorarily invalid scrollbar->Opacity() doesn't set the opacity to zero.

BUG=635629
==========

to

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range.
Now, when it is called during LayerImpl::PushPropertiesTo, the
temprorarily invalid scrollbar->Opacity() doesn't set the opacity to zero.

BUG=635629
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sahel, 2016-09-15 22:27:32]

Description was changed from

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range.
Now, when it is called during LayerImpl::PushPropertiesTo, the
temprorarily invalid scrollbar->Opacity() doesn't set the opacity to zero.

BUG=635629
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range.
Now, when it is called during LayerImpl::PushPropertiesTo, the
temporarily invalid scrollbar->Opacity() doesn't set the opacity to zero.

BUG=635629
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sahel, 2016-09-15 22:28:06]

The CQ bit was checked by sahel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-15 22:28:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sahel, 2016-09-15 22:29:03]

sahel@chromium.org changed reviewers:
+ bokan@chromium.org

[commit-bot: I haz the power, 2016-09-15 23:11:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-15 23:11:06]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sahel, 2016-09-16 14:42:23]

Patchset #1 (id:1) has been deleted

[sahel, 2016-09-16 14:42:40]

The CQ bit was checked by sahel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 14:43:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 15:26:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 15:26:22]

Dry run: This issue passed the CQ dry run.

[sahel, 2016-09-16 15:28:00]

sahel@chromium.org changed reviewers:
+ reveman@chromium.org

[sahel, 2016-09-16 15:28:00]

reveman@chromium.org: Please review changes in

[reveman, 2016-09-16 16:53:12]

reveman@chromium.org changed reviewers:
+ davemoore@chromium.org

[reveman, 2016-09-16 16:53:13]

+davemoore

[bokan, 2016-09-19 15:41:17]

Looks fine to me. Needs a test though, please look at add one to the unittests.

I don't think davemoore@ works on Chromium anymore...

https://codereview.chromium.org/2345823003/diff/20001/cc/input/scrollbar_anim...
File cc/input/scrollbar_animation_controller_thinning.cc (right):

https://codereview.chromium.org/2345823003/diff/20001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning.cc:129: float max_value) {
I don't see a need for these to be params. Just inline the kIdleOpacity and 1
below.

[sahel, 2016-09-19 16:07:21]

https://codereview.chromium.org/2345823003/diff/20001/cc/input/scrollbar_anim...
File cc/input/scrollbar_animation_controller_thinning.cc (right):

https://codereview.chromium.org/2345823003/diff/20001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning.cc:129: float max_value) {
On 2016/09/19 15:41:17, bokan wrote:
> I don't see a need for these to be params. Just inline the kIdleOpacity and 1
> below.

The function is called for adjusting scale of both opacity and thickness. That's
why I didn't inline the ranges.

[bokan, 2016-09-19 16:19:27]

ok, code is fine, just needs a test.

https://codereview.chromium.org/2345823003/diff/20001/cc/input/scrollbar_anim...
File cc/input/scrollbar_animation_controller_thinning.cc (right):

https://codereview.chromium.org/2345823003/diff/20001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning.cc:129: float max_value) {
On 2016/09/19 16:07:21, sahel wrote:
> On 2016/09/19 15:41:17, bokan wrote:
> > I don't see a need for these to be params. Just inline the kIdleOpacity and
1
> > below.
> 
> The function is called for adjusting scale of both opacity and thickness.
That's
> why I didn't inline the ranges.

Ah, sorry, I missed that. In that case this is fine.

[sahel, 2016-09-20 19:12:29]

The CQ bit was checked by sahel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 19:12:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 19:57:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 19:57:11]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sahel, 2016-09-21 14:54:20]

Description was changed from

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range.
Now, when it is called during LayerImpl::PushPropertiesTo, the
temporarily invalid scrollbar->Opacity() doesn't set the opacity to zero.

BUG=635629
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range. Now,
when it is called during LayerImpl::PushPropertiesTo, the temporarily invalid
scrollbar->Opacity() doesn't set the opacity to zero.

The tests fails before applying this patch, and passes on this patch.

BUG=635629
TEST=ScrollbarAnimationControllerThinningTest.AppearOnResize
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sahel, 2016-09-21 14:55:50]

sahel@chromium.org changed reviewers:
- davemoore@chromium.org

[sahel, 2016-09-21 15:00:02]

sahel@chromium.org changed reviewers:
+ aelias@chromium.org
- reveman@chromium.org

[bokan, 2016-09-23 20:15:18]

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
File cc/input/scrollbar_animation_controller_thinning_unittest.cc (right):

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning_unittest.cc:70: 
Nit: spurious new line

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning_unittest.cc:110:
scrollbar_controller_->DidScrollUpdate(false);
DidScrollUpdate gets called when we scroll though, right? Currently, the
scrollbars appear after the user scrolls. Does this test fail without your fix?

[sahel, 2016-09-23 20:25:11]

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
File cc/input/scrollbar_animation_controller_thinning_unittest.cc (right):

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning_unittest.cc:110:
scrollbar_controller_->DidScrollUpdate(false);
On 2016/09/23 20:15:18, bokan wrote:
> DidScrollUpdate gets called when we scroll though, right? Currently, the
> scrollbars appear after the user scrolls. Does this test fail without your
fix?

It get's called when we are scrolling, AND during pushPropertiesTo,
and the latter call causes the bug on load, because it's getting called without
scrolling and during sets the opacity_change_ to be DECREASE.

Yes, it fails without this fix.

[bokan, 2016-09-23 20:26:03]

lgtm

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
File cc/input/scrollbar_animation_controller_thinning_unittest.cc (right):

https://codereview.chromium.org/2345823003/diff/40001/cc/input/scrollbar_anim...
cc/input/scrollbar_animation_controller_thinning_unittest.cc:110:
scrollbar_controller_->DidScrollUpdate(false);
On 2016/09/23 20:25:11, sahel wrote:
> On 2016/09/23 20:15:18, bokan wrote:
> > DidScrollUpdate gets called when we scroll though, right? Currently, the
> > scrollbars appear after the user scrolls. Does this test fail without your
> fix?
> 
> It get's called when we are scrolling, AND during pushPropertiesTo,
> and the latter call causes the bug on load, because it's getting called
without
> scrolling and during sets the opacity_change_ to be DECREASE.
> 
> Yes, it fails without this fix.

Got it. Thanks!

[aelias_OOO_until_Jul13, 2016-09-23 21:57:44]

lgtm

[sahel, 2016-09-24 01:55:56]

The CQ bit was checked by sahel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-24 01:56:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sahel, 2016-09-24 02:02:26]

The CQ bit was checked by sahel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-24 02:02:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-24 02:42:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-24 02:42:35]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sahel, 2016-09-29 18:47:31]

The CQ bit was checked by sahel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 18:47:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 19:45:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-29 19:45:09]

Dry run: This issue passed the CQ dry run.

[sahel, 2016-09-29 19:50:14]

The CQ bit was checked by sahel@chromium.org

[sahel, 2016-09-29 19:50:15]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
aelias@chromium.org
Link to the patchset: https://codereview.chromium.org/2345823003/#ps60001
(title: "nit fixed.")

[commit-bot: I haz the power, 2016-09-29 19:50:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 19:56:37]

Description was changed from

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range. Now,
when it is called during LayerImpl::PushPropertiesTo, the temporarily invalid
scrollbar->Opacity() doesn't set the opacity to zero.

The tests fails before applying this patch, and passes on this patch.

BUG=635629
TEST=ScrollbarAnimationControllerThinningTest.AppearOnResize
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range. Now,
when it is called during LayerImpl::PushPropertiesTo, the temporarily invalid
scrollbar->Opacity() doesn't set the opacity to zero.

The tests fails before applying this patch, and passes on this patch.

BUG=635629
TEST=ScrollbarAnimationControllerThinningTest.AppearOnResize
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-09-29 19:56:38]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-09-29 19:58:39]

Description was changed from

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range. Now,
when it is called during LayerImpl::PushPropertiesTo, the temporarily invalid
scrollbar->Opacity() doesn't set the opacity to zero.

The tests fails before applying this patch, and passes on this patch.

BUG=635629
TEST=ScrollbarAnimationControllerThinningTest.AppearOnResize
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Overlay scrollbars are painted onload.

I fixed the AdjustScale function to keep the values within a valid range. Now,
when it is called during LayerImpl::PushPropertiesTo, the temporarily invalid
scrollbar->Opacity() doesn't set the opacity to zero.

The tests fails before applying this patch, and passes on this patch.

BUG=635629
TEST=ScrollbarAnimationControllerThinningTest.AppearOnResize
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/38991f06ebb579440a3bf3f3fbb8792a60b610d4
Cr-Commit-Position: refs/heads/master@{#421910}
==========

[commit-bot: I haz the power, 2016-09-29 19:58:40]

Patchset 3 (id:??) landed as
https://crrev.com/38991f06ebb579440a3bf3f3fbb8792a60b610d4
Cr-Commit-Position: refs/heads/master@{#421910}