New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 836897



Sign in to add a comment
link

Issue 912574: [BlinkGenPropertyTrees] virtual/threaded/animations/responsive/transform-responsive-neutral-keyframe.html crashes

Reported by pdr@chromium.org, Dec 6 Project Member

Issue description

virtual/threaded/animations/responsive/transform-responsive-neutral-keyframe.html crashes consistently with --additional-driver-flag=--enable-blink-features=BlinkGenPropertyTrees:
STDERR: [FATAL:property_tree.cc(1877)] Check failed: !check_node_existence || ((!state.currently_running[property] && !state.potentially_animating[property]) || needs_rebuild). Attempting to animate non existent transform node
cc::PropertyTrees::ElementIsAnimatingChanged(cc::MutatorHost const*, cc::ElementId, cc::ElementListType, cc::PropertyAnimationState const&, cc::PropertyAnimationState const&, bool) + 1121
cc::LayerTreeHost::ElementIsAnimatingChanged(cc::ElementId, cc::ElementListType, cc::PropertyAnimationState const&, cc::PropertyAnimationState const&) + 323
cc::ElementAnimations::UpdateClientAnimationState() + 1309
cc::KeyframeEffect::RemoveKeyframeModel(int) + 450
cc::Animation::RemoveKeyframeModelForKeyframeEffect(int, unsigned long) + 237
cc::SingleKeyframeEffectAnimation::RemoveKeyframeModel(int) + 60
blink::CompositorAnimation::RemoveKeyframeModel(int) + 48
blink::CompositorAnimations::CancelAnimationOnCompositor(blink::Element const&, blink::CompositorAnimation*, int) + 113
blink::KeyframeEffect::CancelAnimationOnCompositor(blink::CompositorAnimation*) + 261
blink::Animation::RestartAnimationOnCompositor() + 89
blink::KeyframeEffect::ClearEffects() + 144
blink::KeyframeEffect::UpdateChildrenAndEffects() const + 341
blink::AnimationEffect::UpdateInheritedTime(double, blink::TimingUpdateReason) const + 2997
blink::Animation::Update(blink::TimingUpdateReason) + 418
blink::DocumentTimeline::ServiceAnimations(blink::TimingUpdateReason) + 678
blink::(anonymous namespace)::UpdateAnimationTiming(blink::Document&, blink::TimingUpdateReason) + 35
blink::DocumentAnimations::UpdateAnimationTimingForAnimationFrame(blink::Document&) + 26
blink::PageAnimator::ServiceScriptedAnimations(base::TimeTicks) + 717

This test also crashes flakily with the same stack without BGPT:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=animations%2Fresponsive%2Ftransform-responsive-neutral-keyframe.html
 

Comment 1 by flackr@chromium.org, Dec 6

Owner: flackr@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by flackr@chromium.org, Dec 6

Labels: Hotlist-Experimental

Comment 3 by pdr@chromium.org, Dec 6

Cc: smcgruer@chromium.org majidvp@chromium.org
Stephen or Majid, Rob will likely have his hands full with 909055, would you be able to look into this?

Comment 4 by smcgruer@chromium.org, Dec 7

Owner: smcgruer@chromium.org
I can take a look at this, probably not until Monday however

Comment 5 by smcgruer@chromium.org, Dec 7

Some logs from a failing case:

DevTools listening on ws://127.0.0.1:33705/devtools/browser/ecda9351-93ec-49b4-8aa5-fbfd0c72636e
[1:1:1207/112347.976433:INFO:paint_property_tree_builder.cc(598)] "LayoutView #document": CompositingReasonsForTransform: compositing_reasons was 0, is now that plus 0
[1:1:1207/112347.976699:INFO:paint_property_tree_builder.cc(598)] "LayoutBlockFlow HTML": CompositingReasonsForTransform: compositing_reasons was 0, is now that plus 0
[1:1:1207/112347.977105:INFO:layer_tree_host.cc(1598)] SetActiveRegisteredElementIds: (50), (18), (27), (16), 
[1:1:1207/112348.277237:INFO:paint_property_tree_builder.cc(598)] "LayoutView #document": CompositingReasonsForTransform: compositing_reasons was 0, is now that plus 0
[1:1:1207/112348.277619:INFO:paint_property_tree_builder.cc(598)] "LayoutBlockFlow HTML": CompositingReasonsForTransform: compositing_reasons was 0, is now that plus 0
[1:1:1207/112348.277795:INFO:paint_property_tree_builder.cc(598)] "LayoutBlockFlow (positioned) DIV id='target'": CompositingReasonsForTransform: compositing_reasons was 0, is now that plus 192
[1:1:1207/112348.278489:INFO:layer_tree_host.cc(1598)] SetActiveRegisteredElementIds: (149), (18), (150), (114), (148), (27), (16), 
[1:1:1207/112348.278859:INFO:property_tree.cc(1876)] No transform node for (148), checking. state.currently_running[property]: 1, state.potentially_animating[property]: 1, needs_rebuild: 1
[1:8:1207/112348.279741:INFO:property_tree.cc(1876)] No transform node for (148), checking. state.currently_running[property]: 1, state.potentially_animating[property]: 1, needs_rebuild: 1
[1:1:1207/112348.280783:INFO:layer_tree_host.cc(1598)] SetActiveRegisteredElementIds: (149), (18), (150), (114), (148), (27), (16), 
[1:8:1207/112348.280957:INFO:property_tree.cc(1876)] No transform node for (148), checking. state.currently_running[property]: 1, state.potentially_animating[property]: 1, needs_rebuild: 1
[1:1:1207/112348.367014:INFO:property_tree.cc(1876)] No transform node for (148), checking. state.currently_running[property]: 0, state.potentially_animating[property]: 1, needs_rebuild: 0
[1:1:1207/112348.367187:FATAL:property_tree.cc(1884)] Check failed: !check_node_existence || ((!state.currently_running[property] && !state.potentially_animating[property]) || needs_rebuild). Attempting to animate non existent transform node

I'm still trying to understand how this all works, but it seems strange to me that we have (148) as an active registered element id but no transform node for it. The test has a 1e8s transform animation, it should always be there?

Comment 6 by smcgruer@chromium.org, Dec 7

Status: Started (was: Assigned)

Comment 7 by flackr@chromium.org, Dec 7

It shouldn't be trying to animate transform on node 148; 148 is (9<<4 + 4) which is the kPrimaryEffect namespace (i.e. used for opacity)

Comment 8 by smcgruer@chromium.org, Dec 7

Rob came and helped me understand this; looks like he just overlooked PropertyTrees::ElementIsAnimatingChanged during his patch. It is being called with the KeyframeEffect element id rather than the KeyframeModel one, which you can't lookup all the different animation types on.

In this case the KeyframeEffect is associated with the kPrimaryEffect element id, which I suspect is because the animation starts with opacity, but I'm not sure yet.

Comment 9 by flackr@chromium.org, Dec 7

All animations arbitrarily use kPrimaryEffect just to detect when the element has begun to be composited for animations. But the problem is still what you mentioned, ElementIsAnimatingChanged should probably be looking up the element id from the keyframe model for each property.

Comment 10 by smcgruer@chromium.org, Dec 10

To keep this bug up-to-date; I'm currently trying to understand what has broken. Rob's change definitely broke this code for transform animations (the element_id will never be correct), but yet most of animations (tests, real life) has continued to chug along nicely as far as we can tell.

Playing with some local test cases, we always fail to find a TransformNode, since the element_id is wrong, but we usually pass the DCHECK because either check_node_existence is false (which means we are on the impl thread) or needs_rebuild is true (seems to be true almost every time the function is called).

So if we're quietly skipping the path that previously found a TransformNode, what are we skipping? For each animated property type, the code mostly seems to:

1. Set the 'is_currently_animating' and 'has_potential_animation' fields on TransformNode (either to true or false),

2. Set the 'has_only_translation_animations' field on TransformNode

3. Call set_needs_update on the TransformTree

4. Return 'true' for the TransformNode path.

Note that all of the above is correctly set when a tree is built (it takes a different codepath elsewhere which properly asks the ElementAnimations rather than just using the element id), it would fail when animations change without rebuilding the tree. 

I've not grokked the implications of #1-#4 yet, but working on that.

Note that all of this is orthogonal to getting a fix; the fix is clear (just use the transform namespace element id for checking transform, the effect namespace element id for checking effects, etc), but testing the fix requires knowing what is actually going wrong other than 'a DCHECK sometimes fires' (because the code is always failing, the DCHECK only fires in a subset of those cases).

Comment 11 by flackr@chromium.org, Dec 11

I'm not sure I follow. Is it that you're looking for a consistent repro which gets to the root of the issue?

It seems to me that this may happen when an animation finishes on the main thread due to an animation clock which is ahead of the compositor clock (you can force this by polling the animation clock before the raf can be handled). This results in LayerTreeHost::AnimateLayers ticking the animation past the end but no animation events being generated because from the LayerTreeHost perspective it has not finished yet.

Comment 12 by smcgruer@chromium.org, Dec 11

I am looking for the consequences of this breakage. Apparently we have no test that would fail if you just make this function into `return false` (otherwise your CL would not have landed), so what the code is for is a bit of an unknown*.

Why do you think this would cause animation events to not be generated? I am not seeing a clear link between the various fields that might be updated in ElementIsAnimatingChanged and generating animation events, but there are very likely layers of indirection here.

* https://chromium.googlesource.com/chromium/src/+/eb83ceebd189762e06fb281ef9b11cd98b51aa43 implies there are tests for this, but none of them failed when Rob landed his patch

Comment 13 by flackr@chromium.org, Dec 11

I don't see any reason to suspect I actually broke anything other than the check, I just didn't update the check for node existence to ensure that if we think we are animating something the node does in fact exist.

As for animation events, I skipped over a few details:

AnimationEvents are generated by ticking the mutator on the LayerTreeHost, which since https://chromium-review.googlesource.com/c/chromium/src/+/1256364 was not landed, is ticked with the frame time instead of the animation clock time.

This means that if the animation clock is ahead an animation can finish but tickanimations would not generate an animationevent for it.

If an animation event had been generated LayerTreeHost::AnimateLayers would set property_trees_.needs_rebuild = true.

Comment 14 by flackr@chromium.org, Dec 11

Sorry, I spoke too soon, this does modify the animating bits of the property node, I wonder if it fails to update something if we have a main thread update which does no tree changes but does start an animation.

Comment 15 by smcgruer@chromium.org, Dec 11

Note: The tests succeed because they never call set_element_id on the KeyframeModels (because they had no reason to before), so they use the cc::Animation element_id which is then correct for looking up TransformNodes in the test.

Aka, synthetic LayerTreeHost tests once again run into the problem of not accurately capturing reality. (Or arguably put another way - we need more regression tests and less unittests).

Comment 16 by pdr@chromium.org, Dec 13

Labels: -Pri-3 Pri-1
This crash is occurring on youtube.com. Due to the high crash rate, I'm temporarily turning off the finch trial and demoting BlinkGenPropertyTrees to status=test instead of status=experimental.

Comment 17 by pdr@chromium.org, Dec 13

Cc: wangxianzhu@chromium.org

Comment 18 by smcgruer@chromium.org, Dec 14

We have a naive fix for this in progress. It doesn't really uncover the underlying reason behind the code, but should hopefully restore it back to the same behavior it had before flackr's patch. I will be working on it today and hopefully landing it if I am productive enough.

Comment 19 by smcgruer@chromium.org, Dec 19

Unfortunately the fix is taking longer than expected, particularly in trying to figure out how to test it at a useful level. I do not expect to land anything before the winter holidays.

Comment 20 by bugdroid1@chromium.org, Jan 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0eab4a3e36612417196e6766d7a770f675169c4

commit b0eab4a3e36612417196e6766d7a770f675169c4
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Wed Jan 09 17:27:20 2019

[BlinkGenPropertyTrees] Fix ElementIsAnimatingChanged logic

As of https://crrev.com/2a64f7b5, there is no longer a single ElementId
for an entire 'animating target' in cc. However ElementIsAnimatingChanged
still assumes that it can lookup the different types of nodes
(TransformNode, EffectNode) using a single ElementId. This caused both
incorrect logic (code that should have been executed was not as the node
could not be found) as well as DCHECK failures (again from the incorrect
branch).

This fix is essentially a hack, and it is important to note that we don't
have a clear idea of what ElementIsAnimatingChanged is required for (i.e.
what the consequences of it not working -outside of DCHECKs - are). The
goal of this CL is just to restore the function to how it was working
before https://crrev.com/2a64f7b5.

Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel

Change-Id: I0d0eb0c9add83ff3c1b6d6f30858804aed300aac
Bug:  912574 
Reviewed-on: https://chromium-review.googlesource.com/c/1372173
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621198}
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/animation/animation_host.cc
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/animation/element_animations.cc
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/animation/element_animations.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/test/animation_timelines_test_common.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/layer_tree_host.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/mutator_host_client.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/property_tree.cc
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/property_tree.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/cc/trees/target_property.h
[modify] https://crrev.com/b0eab4a3e36612417196e6766d7a770f675169c4/third_party/blink/web_tests/TestExpectations

Comment 21 by smcgruer@chromium.org, Jan 9

Status: Fixed (was: Started)
This is tentatively fixed, though we aren't very confident in the fix being correct :(. Closing for now, let's hope it can stay closed.

Comment 22 by bugdroid1@chromium.org, Jan 10

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93

commit 89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Thu Jan 10 12:34:18 2019

Revert "[BlinkGenPropertyTrees] Fix ElementIsAnimatingChanged logic"

This reverts commit b0eab4a3e36612417196e6766d7a770f675169c4.

Reason for revert: Investigating flakiness https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vYjBlYWI0YTNlMzY2MTI0MTcxOTZlNjc2NmQ3YTc3MGY2NzUxNjljNAw

Original change's description:
> [BlinkGenPropertyTrees] Fix ElementIsAnimatingChanged logic
> 
> As of https://crrev.com/2a64f7b5, there is no longer a single ElementId
> for an entire 'animating target' in cc. However ElementIsAnimatingChanged
> still assumes that it can lookup the different types of nodes
> (TransformNode, EffectNode) using a single ElementId. This caused both
> incorrect logic (code that should have been executed was not as the node
> could not be found) as well as DCHECK failures (again from the incorrect
> branch).
> 
> This fix is essentially a hack, and it is important to note that we don't
> have a clear idea of what ElementIsAnimatingChanged is required for (i.e.
> what the consequences of it not working -outside of DCHECKs - are). The
> goal of this CL is just to restore the function to how it was working
> before https://crrev.com/2a64f7b5.
> 
> Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> 
> Change-Id: I0d0eb0c9add83ff3c1b6d6f30858804aed300aac
> Bug:  912574 
> Reviewed-on: https://chromium-review.googlesource.com/c/1372173
> Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
> Reviewed-by: Majid Valipour <majidvp@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#621198}

TBR=flackr@chromium.org,majidvp@chromium.org,smcgruer@chromium.org

Change-Id: I5448fe9ff246b984916b07fbf2adfdf90ca0d896
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  912574 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/c/1404158
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621553}
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/animation/animation_host.cc
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/animation/element_animations.cc
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/animation/element_animations.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/test/animation_timelines_test_common.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/layer_tree_host.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/mutator_host_client.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/property_tree.cc
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/property_tree.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/cc/trees/target_property.h
[modify] https://crrev.com/89fea2015ea4cd0e1e9f474aa3f7a03f8b903c93/third_party/blink/web_tests/TestExpectations

Comment 23 by smcgruer@chromium.org, Jan 10

Status: Started (was: Fixed)
CL was reverted due to  issue 920447 , flakiness in virtual/threaded/animations/skew-notsequential-compositor.html

Given that that test is also flaky on BGPT, I wonder if it is an actual testcase for the discussion we had in https://chromium-review.googlesource.com/c/chromium/src/+/1372173/7/cc/animation/element_animations.cc#507 (the one on line 507). Will be investigating today.

Comment 24 by smcgruer@chromium.org, Jan 10

Patching my CL back in, I was able to reproduce via:

python third_party/blink/tools/run_web_tests.py --additional-driver-flag=--disable-blink-features=BlinkGenPropertyTrees virtual/threaded/animations/skew-notsequential-compositor.html

It appears to reproduce instantly, so no need for multiple runs.

Comment 25 by smcgruer@chromium.org, Jan 10

I believe I was on a bit of a wild goose for the last hour, because I just found that even *without* my change the test failed consistently when I passed '--additional-driver-flag=--disable-blink-features=BlinkGenPropertyTrees'. However with no additional driver flag I can't get it to fail (even though it *should* be flaky under BGPT even before my CL...).

This is super weird, but I'm going to revert http://crrev.com/00868f0800150009e4cd5575e2477c566817fc43 locally and hope that I'm in a saner space for the next hour of effort...

Comment 26 by smcgruer@chromium.org, Jan 10

Ahem. Today I learnt about what runtime_enabled_flag statuses actually mean. Both 'test' and 'experimental' mean the feature is enabled in LayoutTests (err, WebTests). So reverting http://crrev.com/00868f0800150009e4cd5575e2477c566817fc43 is irrelevant, and I've been debugging all wrong. (Thanks to pdr@ for help sorting that out).

Having reset my understanding, I can now take another go at investigating the flakiness.

Comment 27 by smcgruer@chromium.org, Jan 10

Ok, so I'm getting an understanding on the failure now (but not the reason!). This test was rebaselined when BGPT was first promoted to experimental (http://crrev.com/0f14f7a53659537ba1e20b4d9a79453d4f317467). The rebaselined version looks close to the original, and was put down to subpixel AA differences (which it may still be) - see http://crbug.com/905861.

With my CL, for whatever reason, occasionally causes BGPT to flake back to the original behavior, e.g.:

$ diff out/Release/layout-test-results/virtual/threaded/animations/skew-notsequential-compositor-actual.png out/Release/layout-test-results/virtual/threaded/animations/skew-notsequential-compositor-expected.png 
Binary files out/Release/layout-test-results/virtual/threaded/animations/skew-notsequential-compositor-actual.png and out/Release/layout-test-results/virtual/threaded/animations/skew-notsequential-compositor-expected.png differ

$ diff out/Release/layout-test-results/virtual/threaded/animations/skew-notsequential-compositor-actual.png ~/Desktop/old-skew-notsequential-compositor-expected.png
[no diff]

I am not sure why the CL causes this change however. As expected, before the CL the logs show that the TransformNode is never found in ElementIsAnimatingChanged. After the CL, it is always found - in both the flaky and non-flaky cases. So there isn't an obvious difference there.
before_cl_log.txt
2.6 KB View Download
after_cl_not_flaky_case_log.txt
3.2 KB View Download
after_cl_flaky_case_log.txt
3.2 KB View Download

Comment 28 by smcgruer@chromium.org, Jan 10

pdr suggested dumping the LayerImpls. This appears to be a good idea; the ones before the CL and after the CL in the not-flaky case are very similar, but the flaky case is much different. Hopefully we can learn something from analyzing these.
after_cl_flaky_case_layer_impl_dump_log.txt
78.5 KB View Download
after_cl_not_flaky_case_layer_impl_dump_log.txt
104 KB View Download
before_cl_layer_impl_dump_log.txt
103 KB View Download

Comment 29 by smcgruer@chromium.org, Jan 11

After some cleanup (removing non-renderer log lines), sadly the logs are actually quite similar, *except* that the flaky case only has one instance of 'After finishing commit on impl' being logged whilst the before-cl and non-flaky cases each have two. The final one of these occur *after* the last call to ElementIsAnimatingChanged, whereas in the flaky case the call to ElementIsAnimatingChanged is the last thing logged.

Comment 30 by smcgruer@chromium.org, Jan 11

 Issue 912337  has been merged into this issue.

Comment 31 by bugdroid1@chromium.org, Jan 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a511d649f39d3f59cecf4731803a2179ea0c322

commit 2a511d649f39d3f59cecf4731803a2179ea0c322
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Fri Jan 11 20:30:06 2019

Reland "[BlinkGenPropertyTrees] Fix ElementIsAnimatingChanged logic"

This relands the commit b0eab4a3e36612417196e6766d7a770f675169c4 from
https://chromium-review.googlesource.com/c/chromium/src/+/1372173

The only change is to mark virtual/threaded/animations/skew-notsequential-compositor.html
as flaky; it has been decided that it is worth landing the CL anyway
and investigating the flakiness afterwards.

TBR=majidvp@chromium.org,pdr@chromium.org

Bug:  912574 , 921105
Change-Id: I93dfc5db4056e3567f99254f28b1072a66c4f532
Reviewed-on: https://chromium-review.googlesource.com/c/1407385
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622123}
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/animation/animation_host.cc
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/animation/element_animations.cc
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/animation/element_animations.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/test/animation_timelines_test_common.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/layer_tree_host.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/mutator_host_client.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/property_tree.cc
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/property_tree.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/cc/trees/target_property.h
[modify] https://crrev.com/2a511d649f39d3f59cecf4731803a2179ea0c322/third_party/blink/web_tests/TestExpectations

Comment 32 by smcgruer@chromium.org, Jan 11

Status: Fixed (was: Started)
The flakiness is being followed up in issue 921105. Hopefully this issue is fixed.

Sign in to add a comment