Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2868553002:
Reland Fade out overlay scrollbar after page load (Closed)
DescriptionReland Fade out overlay scrollbar after page load
This issue is caused by Aura Overlay Scrollbar is created with
opacity>0. We
used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out
when RegisterScrollLayer.
```
#2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars()
#3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState()
#4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer()
```
But we remove ScrollbarAnimationController::DidScrollUpdate call in
UpdateScrollbars at
https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4.
Then Overlay Scrollbar can not fade out until we have real scroll.
In this patch, we apply set_needs_show_scrollbars(true) to the layer in
LayerTreeImpl::RegisterScrollLayer for Aura that will let
LTI::HandleScrollbarShowRequestsFromMain pickup and call
SAC::DidScrollUpdate.
For cc_unittest, remove set_needs_show_scrollbars call in
LayerTreeHostImplTestScrollbarAnimation.* and
LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit.
BUG=717222
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2868553002
Cr-Commit-Position: refs/heads/master@{#470695}
Committed: https://chromium.googlesource.com/chromium/src/+/4180d24ef895be388449653c9fa14b105cd49a45
Patch Set 1 #Patch Set 2 : fix test #
Messages
Total messages: 23 (14 generated)
Description was changed from ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura Overlay Scrollbar that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 ========== to ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura Overlay Scrollbar that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Description was changed from ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura Overlay Scrollbar that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura Overlay Scrollbar that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
chaopeng@chromium.org changed reviewers: + weiliangc@chromium.org
PTAL. Thank you.
LGTM. Is the regression because this is adding more code on the critical path for gesture scroll? Do we need to worry about CrOS (where it would be aura but also possible touch screen with gesture scroll?)
Description was changed from ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura Overlay Scrollbar that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we call SAC::DidScrollUpdate in LayerTreeImpl::RegisterScrollLayer. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Patchset #2 (id:20001) has been deleted
Description was changed from ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we call SAC::DidScrollUpdate in LayerTreeImpl::RegisterScrollLayer. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
On 2017/05/08 19:41:00, weiliangc wrote: > LGTM. > > Is the regression because this is adding more code on the critical path for > gesture scroll? Do we need to worry about CrOS (where it would be aura but also > possible touch screen with gesture scroll?) For more investigation, I confirmed the regression is from fade out animation not gesture scroll.
The CQ bit was checked by chaopeng@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by chaopeng@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from weiliangc@chromium.org Link to the patchset: https://codereview.chromium.org/2868553002/#ps40001 (title: "fix test")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by chaopeng@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494443438446680,
"parent_rev": "2893fe08decf1fca698b0096e5e530eba397baa5", "commit_rev":
"4180d24ef895be388449653c9fa14b105cd49a45"}
Message was sent while issue was closed.
Description was changed from ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Reland Fade out overlay scrollbar after page load This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call SAC::DidScrollUpdate - LTI::UpdateScrollbars to fade out when RegisterScrollLayer. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at https://crrev.com/697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll. In this patch, we apply set_needs_show_scrollbars(true) to the layer in LayerTreeImpl::RegisterScrollLayer for Aura that will let LTI::HandleScrollbarShowRequestsFromMain pickup and call SAC::DidScrollUpdate. For cc_unittest, remove set_needs_show_scrollbars call in LayerTreeHostImplTestScrollbarAnimation.* and LayerTreeHostImplTest.ScrollbarVisibilityChangeCausesRedrawAndCommit. BUG=717222 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2868553002 Cr-Commit-Position: refs/heads/master@{#470695} Committed: https://chromium.googlesource.com/chromium/src/+/4180d24ef895be388449653c9fa1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/4180d24ef895be388449653c9fa1... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
