Check failed: scrollbar_ids.vertical == Layer::INVALID_ID (24 vs. -1)Existing scrollbar should have been unregistered. |
||||
Issue description
Chrome Version: ToT {#484301}
OS: Android
What steps will reproduce the problem?
(1) Add the flag --enable-blink-features=SetRootScroller, build Clank with DCHECKs on
(2) Visit https://bokand.github.io/rootscroller3.html
[6255:6359:0706/105506.972768:41685375626:FATAL:layer_tree_impl.cc(1676)] Check failed: scrollbar_ids.vertical == Layer::INVALID_ID (24 vs. -1)Existing scrollbar should have been unregistered.
#0 0x7f83e5b5668d base::debug::StackTrace::StackTrace()
#1 0x7f83e5b54ccc base::debug::StackTrace::StackTrace()
#2 0x7f83e5be4e4a logging::LogMessage::~LogMessage()
#3 0x7f83e577b692 cc::LayerTreeImpl::RegisterScrollbar()
#4 0x7f83e547394b cc::ScrollbarLayerImplBase::SetScrollElementId()
#5 0x7f83e54738a0 cc::ScrollbarLayerImplBase::PushPropertiesTo()
#6 0x7f83e54568e7 cc::PaintedOverlayScrollbarLayerImpl::PushPropertiesTo()
#7 0x7f83e580963b cc::PushLayerPropertiesInternal<>()
#8 0x7f83e5809316 cc::TreeSynchronizer::PushLayerProperties()
#9 0x7f83e57433b0 cc::LayerTreeHostImpl::ActivateSyncTree()
#10 0x7f83e57e9c74 cc::ProxyImpl::ScheduledActionActivateSyncTree()
#11 0x7f83e56314f7 cc::Scheduler::ProcessScheduledActions()
#12 0x7f83e5631ce8 cc::Scheduler::NotifyReadyToActivate()
#13 0x7f83e57e571a cc::ProxyImpl::NotifyReadyToActivate()
#14 0x7f83e573b14d cc::LayerTreeHostImpl::NotifyReadyToActivate()
#15 0x7f83e56bb521 cc::TileManager::CheckAndIssueSignals()
#16 0x7f83e550d6bd _ZN4base8internal13FunctorTraitsIMN2cc18LayerTreeFrameSinkEFvvEvE6InvokeIPS3_JEEEvS5_OT_DpOT0_
#17 0x7f83e550d604 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN2cc18LayerTreeFrameSinkEFvvEJPS5_EEEvOT_DpOT0_
#18 0x7f83e56d6595 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc11TileManagerEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#19 0x7f83e56d64dc _ZN4base8internal7InvokerINS0_9BindStateIMN2cc11TileManagerEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#20 0x7f83eb0402dd _ZNKR4base8CallbackIFvvELNS_8internal8CopyModeE1ELNS2_10RepeatModeE1EE3RunEv
#21 0x7f83eb072673 cc::UniqueNotifier::Notify()
#22 0x7f83eb04107f _ZN4base8internal13FunctorTraitsIMN2cc21DelayedUniqueNotifierEFvvEvE6InvokeINS_7WeakPtrIS3_EEJEEEvS5_OT_DpOT0_
#23 0x7f83eb07285a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMN2cc14UniqueNotifierEFvvENS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#24 0x7f83eb0727f0 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc14UniqueNotifierEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIS6_NSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#25 0x7f83eb072749 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc14UniqueNotifierEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#26 0x7f83e5b01fa1 _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv
#27 0x7f83e5b5b137 base::debug::TaskAnnotator::RunTask()
#28 0x7f83dbe80fd4 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#29 0x7f83dbe7bf8a blink::scheduler::TaskQueueManager::DoWork()
#30 0x7f83dbe88e37 _ZN4base8internal13FunctorTraitsIMN5blink9scheduler16TaskQueueManagerEFvbEvE6InvokeIRKNS_7WeakPtrIS4_EEJRKbEEEvS6_OT_DpOT0_
#31 0x7f83dbe88d95 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN5blink9scheduler16TaskQueueManagerEFvbERKNS_7WeakPtrIS6_EEJRKbEEEvOT_OT0_DpOT1_
#32 0x7f83dbe88d0d _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvbEJNS_7WeakPtrIS5_EEbEEEFvvEE7RunImplIRKS7_RKNSt3__15tupleIJS9_bEEEJLm0ELm1EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#33 0x7f83dbe88c1c _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvbEJNS_7WeakPtrIS5_EEbEEEFvvEE3RunEPNS0_13BindStateBaseE
#34 0x7f83e5b01fa1 _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv
#35 0x7f83e5b5b137 base::debug::TaskAnnotator::RunTask()
#36 0x7f83e5c13bed base::MessageLoop::RunTask()
#37 0x7f83e5c13e77 base::MessageLoop::DeferOrRunPendingTask()
#38 0x7f83e5c14bfa base::MessageLoop::DoWork()
#39 0x7f83e5c1b458 base::MessagePumpDefault::Run()
#40 0x7f83e5c13514 base::MessageLoop::Run()
#41 0x7f83e5cc706d base::RunLoop::Run()
#42 0x7f83e5d92084 base::Thread::Run()
#43 0x7f83e5d92ca2 base::Thread::ThreadMain()
#44 0x7f83e5d73ed1 base::(anonymous namespace)::ThreadFunc()
#45 0x7f83eac6d184 start_thread
#46 0x7f83d81d8ffd clone
Note, I can repro this on linux using content_shell with the following flags:
./out/ChromeDebug/content_shell --single-process --enable-viewport --enable-features=OverlayScrollbar --enable-blink-features=SetRootScroller --enable-threaded-compositing --enable-prefer-compositing-to-lcd-text --enable-fixed-position-compositing ~/bokand.github.io/rootscroller3.html
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8448024fe348b22b8c1e45f6b8b6437cc7853361 commit 8448024fe348b22b8c1e45f6b8b6437cc7853361 Author: David Bokan <bokan@chromium.org> Date: Wed Jul 12 20:59:48 2017 Unregister cc scrollbar when registering new one We previously DCHECKed that we could never register a new scrollbar against a scroll layer that already has a scrollbar. i.e. We expected that the old scrollbar would be unregistered prior to registering the new one. RootScroller can break this assumption on Android. On Android, the global rootScroller (i.e. OuterViewport) is prevented from creating its own scrollbars, instead inheriting the VisualViewport-created scrollbars. In other words, the VisualViewport creates a pair of scrollbars and they get re-assigned to whatever the current outer viewport is. This means that when we change the global rootScroller (for context see core/page/scrolling/README.md), the previous rootScroller now creates its own scrollbars and the VisualViewport scrollbars get registered to the new rootScroller. This then creates a race between moving the VisualViewport scrollbar registration and new scrollbar registration on the old rootScroller. Long-term, the outer viewport shouldn't own the scrollbars but for now it doesn't hurt to just unregister the scrollbar if we notice the scroller already has scrollbars registered. Bug: 739738 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: I99e8a95881e7dad19c0318a661da80f441b97b19 Reviewed-on: https://chromium-review.googlesource.com/562018 Reviewed-by: Weiliang Chen <weiliangc@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#486086} [modify] https://crrev.com/8448024fe348b22b8c1e45f6b8b6437cc7853361/cc/trees/layer_tree_impl.cc
,
Jul 12 2017
,
Jul 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39a4e54d06758898966637d4c5e1a0ec701306ef commit 39a4e54d06758898966637d4c5e1a0ec701306ef Author: David Bokan <bokan@chromium.org> Date: Mon Jul 17 15:28:10 2017 Revert "Unregister cc scrollbar when registering new one" This reverts commit 8448024fe348b22b8c1e45f6b8b6437cc7853361. Reason for revert: This likely introduced a container-overflow: crbug.com/742520 Original change's description: > Unregister cc scrollbar when registering new one > > We previously DCHECKed that we could never register a new scrollbar > against a scroll layer that already has a scrollbar. i.e. We expected > that the old scrollbar would be unregistered prior to registering the > new one. > > RootScroller can break this assumption on Android. On Android, the > global rootScroller (i.e. OuterViewport) is prevented from creating > its own scrollbars, instead inheriting the VisualViewport-created > scrollbars. In other words, the VisualViewport creates a pair of > scrollbars and they get re-assigned to whatever the current outer > viewport is. > > This means that when we change the global rootScroller (for context see > core/page/scrolling/README.md), the previous rootScroller now creates > its own scrollbars and the VisualViewport scrollbars get registered to > the new rootScroller. This then creates a race between moving the > VisualViewport scrollbar registration and new scrollbar registration on > the old rootScroller. > > Long-term, the outer viewport shouldn't own the scrollbars but for now > it doesn't hurt to just unregister the scrollbar if we notice the > scroller already has scrollbars registered. > > Bug: 739738 > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I99e8a95881e7dad19c0318a661da80f441b97b19 > Reviewed-on: https://chromium-review.googlesource.com/562018 > Reviewed-by: Weiliang Chen <weiliangc@chromium.org> > Commit-Queue: David Bokan <bokan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#486086} TBR=bokan@chromium.org,weiliangc@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 739738 , 742520, 742678 Change-Id: I1134e17dcb04a3a2cc50f07ecbc0fc79cc74f344 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/574547 Commit-Queue: David Bokan <bokan@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#487082} [modify] https://crrev.com/39a4e54d06758898966637d4c5e1a0ec701306ef/cc/trees/layer_tree_impl.cc
,
Jul 19 2017
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec9c793c15edaaf317b00c9bcf32c5472d698475 commit ec9c793c15edaaf317b00c9bcf32c5472d698475 Author: David Bokan <bokan@chromium.org> Date: Wed Jul 19 23:16:31 2017 Reland "Unregister cc scrollbar when registering new one" This was previously landed in: https://chromium-review.googlesource.com/c/562018/ but was reverted in: https://chromium-review.googlesource.com/c/574547/ due to a a Clusterfuzz bug. Turns out that calling UnregisterScrollbar can erase the references we're holding in scrollbar_ids and scrollbar_layer_id so we have to refresh those. Bug: 739738 Change-Id: Ic80018212b1370be6a33a39add19da6848b59fb6 Reviewed-on: https://chromium-review.googlesource.com/578428 Reviewed-by: Weiliang Chen <weiliangc@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#488030} [modify] https://crrev.com/ec9c793c15edaaf317b00c9bcf32c5472d698475/cc/trees/layer_tree_impl.cc
,
Jul 20 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by bokan@chromium.org
, Jul 6 2017