Chrome crashes when animation is reversed, if we set will-change to transform |
|||||
Issue descriptionA debug build of content_shell crashed when (i). "will-change: transform" was set. (ii). animation was reversed, triggered by a mouse-click event. Removing "will-change: transform" would fix the problem. Please see attached the html file. The error message is [1:5:0517/090152.134663:582399987691:FATAL:layer_tree_impl.cc(590)] Check failed: 1u == property_trees()->element_id_to_transform_node_index.count( element_id) (1 vs. 0) #0 0x7f30a748abd7 base::debug::StackTrace::StackTrace() #1 0x7f30a74b053d logging::LogMessage::~LogMessage() #2 0x7f30a5bf567f cc::LayerTreeImpl::SetTransformMutated() #3 0x7f30a5bf0ed9 cc::LayerTreeHostImpl::SetElementTransformMutated() #4 0x7f309fe888a8 cc::ElementAnimations::OnTransformAnimated() #5 0x7f309fe842b9 cc::AnimationPlayer::TickAnimations() #6 0x7f309fe83dde cc::AnimationPlayer::Tick() #7 0x7f309fe80556 cc::AnimationHost::TickAnimations() #8 0x7f30a5beff2a cc::LayerTreeHostImpl::AnimateLayers() #9 0x7f30a5be5f60 cc::LayerTreeHostImpl::AnimateInternal() #10 0x7f30a5be59c5 cc::LayerTreeHostImpl::UpdateSyncTreeAfterCommitOrImplSideInvalidation() #11 0x7f30a5be588d cc::LayerTreeHostImpl::CommitComplete() #12 0x7f30a5c11827 cc::ProxyImpl::ScheduledActionCommit() #13 0x7f30a5bac331 cc::Scheduler::ProcessScheduledActions() #14 0x7f30a5bacabb cc::Scheduler::NotifyReadyToCommit() #15 0x7f30a5c0ee77 cc::ProxyImpl::NotifyReadyToCommitOnImpl() #16 0x7f30a7475f01 _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv #17 0x7f30a748b4f3 base::debug::TaskAnnotator::RunTask() #18 0x7f30a2c95de6 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue() #19 0x7f30a2c9405a blink::scheduler::TaskQueueManager::DoWork() #20 0x7f30a7475f01 _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv #21 0x7f30a748b4f3 base::debug::TaskAnnotator::RunTask() #22 0x7f30a74bcbed base::MessageLoop::RunTask() #23 0x7f30a74bcfdc base::MessageLoop::DeferOrRunPendingTask() #24 0x7f30a74bd3a6 base::MessageLoop::DoWork() #25 0x7f30a74bec09 base::MessagePumpDefault::Run() #26 0x7f30a74bc955 base::MessageLoop::RunHandler() #27 0x7f30a74f1e7c base::RunLoop::Run() #28 0x7f30a752ff4c base::Thread::Run() #29 0x7f30a7530468 base::Thread::ThreadMain() #30 0x7f30a7527afc base::(anonymous namespace)::ThreadFunc() #31 0x7f30a8f7f184 start_thread #32 0x7f30a0904bed clone Please advise if you're the right team to look at this issue, or we should send it to some other team. It is blocking another issue that we intend to work on. Thank you. Jia
,
May 17 2017
Walter could you check this one out? Could be a good hint into a problem.
,
May 18 2017
Able to repro at ToT after clicking a few times. Looking further.
,
May 18 2017
Logging when we start animation normally: [1:1:0518/111015.712177:862930601325:ERROR:property_tree_builder.cc(385)] PTB AddTransformNodeIfNeeded [layer=20, has_potentially_animated_transform=1, has_any_transform_animation=1, has_surface=0, requires_node=1]. When we see DCHECK failure: [1:1:0518/111018.075766:862932964834:ERROR:property_tree_builder.cc(385)] PTB AddTransformNodeIfNeeded [layer=20, has_potentially_animated_transform=0, has_any_transform_animation=0, has_surface=0, requires_node=0]. For some reason, when we flip animation player's playback rate, in some cases we don't see a transform node. Involved layer id is 20 and element id is 24 with specific test case and ToT build. From logging in AnimationPlayer, the normal lifecycle cc-side for an animation player is: DetachElement (lazy removal if there was a previous animation) UnregisterPlayer (lazy removal if there was a previous animation) AttachElement RegisterPlayer AddAnimation This happens first on main thread, and then main thread builds property trees and pushes animation data and trees to impl thread. Impl thread then repeats above animation player sequence, but when ticked it has bad property tree data. Need to understand why property tree builder isn't seeming to see the transform animation despite the player lifecycle inclusive of AddAnimation happening on the main thread ahead of building property trees.
,
May 18 2017
The layer does not have an element id set on it when we rebuild property trees despite AddAnimation call prior. So we're seeing something like: - CLM sets element id of animated layer to X - PTB builds property trees, starts and runs animation successfully referencing element id X - CLM sets element id of animated layer to 0 (clearing it). This does not always happen, but it always happens when we see the DCHECK. - user clicks to start animation again - previous animation is removed, player detached, unregistered - new animation is attached to element, player registered, animation added - PTB builds property trees, doesn't see element id for animation as CLM hasn't reset element id of animated layer back to X for some reason - property tree and animation data is pushed to impl thread, animation starts ticking to animate transform - CLM UpdateElementIdAndCompositorMutableProperties runs now for some reason and belatedly sets element id to X. Too little, too late. - LayerTreeImpl::SetTransformMutated is called and fails DCHECK
,
May 18 2017
Great news! This bug will totes be fixed by patch in work by chrishtr@, as we will shift to always setting compositor element id on GraphicsLayers. See http://crrev.com/2890953002/ Transferring bug back to him. I manually hacked CLM and confirmed fixes this test case. We could consider adding the test case on this bug to our LayoutTests repertoire.
,
May 19 2017
Thanks for such a detailed walkthrough of what was happening!
,
May 19 2017
Thanks for fixing this! My test runs showed it's all ok now.
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/18fc5e473594ff6bb062183835a60257def9a627 commit 18fc5e473594ff6bb062183835a60257def9a627 Author: chrishtr <chrishtr@chromium.org> Date: Fri May 19 04:20:59 2017 [SPv1] Always set a CompositorElementId on main graphics layers; use PaintLayer id. This means we will always have an element id in cc property trees, which means we can rely on it as a key for node lookup instead of layer id. BUG= 718564 , 723099 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2890953002 Cr-Commit-Position: refs/heads/master@{#473085} [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/animation/Animation.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/animation/AnimationTest.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/page/scrolling/ScrollState.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/paint/PaintLayer.h [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/platform/graphics/CompositorElementId.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/platform/graphics/CompositorElementId.h [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/platform/graphics/CompositorElementIdTest.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.h [modify] https://crrev.com/18fc5e473594ff6bb062183835a60257def9a627/third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp
,
May 19 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by jiameng@chromium.org
, May 16 2017