CSS-animation based UMA is not reporting correctly |
|||||
Issue descriptionIn this previous CL: https://chromium-review.googlesource.com/c/chromium/src/+/636305 We added a DCHECK in animation_host.cc to make sure that the main thread animation is >= main thread compositable animation. However, it appears that we could hit this DCHECK in the real world.
,
Dec 8 2017
,
Jan 11 2018
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac51db7592b74c8d627e09b80e0e8af2d347ba36 commit ac51db7592b74c8d627e09b80e0e8af2d347ba36 Author: Xida Chen <xidachen@chromium.org> Date: Tue Feb 06 22:40:03 2018 Fix main thread compositable animation UMA I had this CL that implements animation specific UMA: https://chromium-review.googlesource.com/c/chromium/src/+/636305 In particular, we record frame rate when there is main thread / compositor / main thread compositable animations in this frame. However, the main thread compositable animations is not calculated correctly in the above CL. The result of that is a crash which is tracked in crbug.com/781305 . In order to solve the crash, we temporary disabled the main thread compositable animation UMA in here: https://chromium-review.googlesource.com/c/chromium/src/+/753405 Now it is time to re-enable that UMA. This is how we check whether an animation is main thread compositable or not: we first check whether the effects can start on compositor or not. For example, a transform animation will be able to start on compositor. Then we check whether the target element can start on compositor or not. If the target element is not paint into its own backing, we know that this is due to a running experiment. The problem with the above logic is SVG element. For example, if we have a transform animation on an SVG element, the effect can be started on compositor but the SVG element cannot start on compositor. In order to tell whether this is a main thread compositable, we have to exclude the SVG element. This CL fixed the problem. It re-enables a unit test and added another test for SVG element. Bug: 781305 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I030f2e7d2061589ccb0a62f12744b1097855f2f7 Reviewed-on: https://chromium-review.googlesource.com/895409 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#534820} [modify] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/cc/animation/animation_host.cc [modify] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp [modify] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/third_party/WebKit/Source/core/animation/CompositorAnimations.h [modify] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp [modify] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/third_party/WebKit/Source/core/animation/DocumentTimeline.cpp [add] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/third_party/WebKit/Source/core/animation/test_data/transform-animation-on-svg.html [modify] https://crrev.com/ac51db7592b74c8d627e09b80e0e8af2d347ba36/third_party/WebKit/Source/core/paint/compositing/CompositingReasonFinder.cpp
,
Feb 7 2018
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0e80e68c0a5670217965b3aeedb4783dbf0587d commit c0e80e68c0a5670217965b3aeedb4783dbf0587d Author: Xida Chen <xidachen@chromium.org> Date: Fri Feb 16 18:16:34 2018 Simplify logic of computing main thread compositable animations A main thread compositable animation means that this animation should be composited such as an opacity or 2d transform, but it is not due to certain check such as RuntimeEnabledFeatures::TurnOff2DAndOpacityCompositorAnimationsEnabled which is designed to be used for an experiment described in crbug.com/754471. Currently our logic is: 1. Check if the effect (such as opacity) can start on compositor or not 2. If 1 is true, and the target element is not paint into its own backing + the target element is not a SVG, then it is a main thread compositable animation. This logic is over complicated. We should really just check whether the run time flag is enabled or not. So this CL simplifies it. We already have unit test in CompositorAnimationsTest to ensure that this change is fine. Bug: 781305 Change-Id: I8f9a1d82fa60eea79018c6cf777a1b45bd565022 Reviewed-on: https://chromium-review.googlesource.com/919316 Commit-Queue: Xida Chen <xidachen@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#537351} [modify] https://crrev.com/c0e80e68c0a5670217965b3aeedb4783dbf0587d/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp [modify] https://crrev.com/c0e80e68c0a5670217965b3aeedb4783dbf0587d/third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
,
Mar 5 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Nov 4 2017