Overlay scrollbar did not fade out after the first appear |
||
Issue descriptionChrome Version: git: da4b99838d45 OS: Linux What steps will reproduce the problem? (0) Enable overlay-scrollbars (1) open a page has scrollbar What is the expected result? Scrollbar fade out after a while. What happens instead? Scrollbar does not fade out until a scroll update.
,
May 3 2017
This issue is caused by Aura Overlay Scrollbar is created with opacity>0. We used to call LayerTreeImpl::UpdateScrollbars to fade out when initialization. ``` #2 0x7f23d0dfe1ea cc::LayerTreeImpl::UpdateScrollbars() #3 0x7f23d0dfd999 cc::LayerTreeImpl::DidUpdateScrollState() #4 0x7f23d0e07231 cc::LayerTreeImpl::RegisterScrollLayer() ``` But we remove ScrollbarAnimationController::DidScrollUpdate call in UpdateScrollbars at 697a467f819ce09da5209a3df13b8b92f33e35a4. Then Overlay Scrollbar can not fade out until we have real scroll.
,
May 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b14926c905396fbeb0a77bfc6cce569c8a35fa5f commit b14926c905396fbeb0a77bfc6cce569c8a35fa5f Author: chaopeng <chaopeng@chromium.org> Date: Thu May 04 19:36:19 2017 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 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/2858973002 Cr-Commit-Position: refs/heads/master@{#469425} [modify] https://crrev.com/b14926c905396fbeb0a77bfc6cce569c8a35fa5f/cc/trees/layer_tree_host_impl_unittest.cc [modify] https://crrev.com/b14926c905396fbeb0a77bfc6cce569c8a35fa5f/cc/trees/layer_tree_impl.cc
,
May 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9320fa40f77c26a054be97c9e2c7543e277c3081 commit 9320fa40f77c26a054be97c9e2c7543e277c3081 Author: chaopeng <chaopeng@chromium.org> Date: Fri May 05 20:07:03 2017 Revert of Fade out overlay scrollbar after page load (patchset #1 id:1 of https://codereview.chromium.org/2858973002/ ) Reason for revert: Revert for regression Details: https://bugs.chromium.org/p/chromium/issues/detail?id=718863 Original issue's description: > 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 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/2858973002 > Cr-Commit-Position: refs/heads/master@{#469425} > Committed: https://chromium.googlesource.com/chromium/src/+/b14926c905396fbeb0a77bfc6cce569c8a35fa5f TBR=weiliangc@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 717222 Review-Url: https://codereview.chromium.org/2863153002 Cr-Commit-Position: refs/heads/master@{#469763} [modify] https://crrev.com/9320fa40f77c26a054be97c9e2c7543e277c3081/cc/trees/layer_tree_host_impl_unittest.cc [modify] https://crrev.com/9320fa40f77c26a054be97c9e2c7543e277c3081/cc/trees/layer_tree_impl.cc
,
May 9 2017
The patch skobes@ landed had that as an intended change. We can't rely on RegisterScrollLayer/Scrollbars since that might happen before the scroll bounds are pushed. Instead, we should be relying on the main thread telling the compositor to show the scrollbars. In this case, why isn't the call to ShowOverlayScrollbars in FrameView::ViewportSizeChanged enough?
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4180d24ef895be388449653c9fa14b105cd49a45 commit 4180d24ef895be388449653c9fa14b105cd49a45 Author: chaopeng <chaopeng@chromium.org> Date: Wed May 10 20:38:57 2017 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} [modify] https://crrev.com/4180d24ef895be388449653c9fa14b105cd49a45/cc/trees/layer_tree_host_impl_unittest.cc [modify] https://crrev.com/4180d24ef895be388449653c9fa14b105cd49a45/cc/trees/layer_tree_impl.cc
,
May 11 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by chaopeng@chromium.org
, May 2 2017