New issue
Advanced search Search tips

Issue 676456 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 649560

Blocking:
issue 666986



Sign in to add a comment

CompositorAnimations::canStartAnimationOnCompositor() can be called during style update producing a lifecycle violation.

Project Member Reported by wkorman@chromium.org, Dec 21 2016

Issue description

Filing tracking bug for discussion/consideration of a FIXME found in animation logic. This is a known issue that appears to date to ~2014, see comment in 

http://crrev.com/189883003

The issue will persist with SPv2, excepting that rather than querying stale compositing state via PaintLayer direct compositing reasons, we'll be querying stale paint property tree state via transform/effect paint property node direct compositing reasons.

On consideration we currently believe that the SPv2 situation is roughly analogous to SPv1 and that it is possible a cancelled animation could stay on the compositor longer than necessary, but is otherwise not harmful.

Why are we calling canStartAnimationOnCompositor() from cancelAnimationOnCompositor()? I would think we could use compositor state booleans on the Element instead of relying on the can-start logic to essentially reverse engineer whether we did-start. Perhaps the compositor state booleans are potentially stale, or complicated by the potential existence of multiple animations for a single Element?

If we can't use the compositor state bools then as the comment states we'll need to postpone the animation cancellation until at or after the appropriate lifecycle stage. Which means we'd either need to accrue a list of the animations to be cancelled somewhere and call that when appropriate, or post a message to process them later (which I'm told is done in other similar circumstances via some existing infrastructure).
 
Summary: CompositorAnimations::canStartAnimationOnCompositor() can be called during style update producing a lifecycle violation. (was: cancelAnimationOnCompositor can be called during style update, producing a lifecycle violation.)

Comment 2 by suzyh@chromium.org, Jan 11 2017

Cc: alancutter@chromium.org
Labels: Update-Quarterly
Blink>Animation has a policy of only having one component on a bug except in limited circumstances (see https://sites.google.com/a/chromium.org/dev/blink/animation-team). What is the concrete work that needs to be done here, and is that work first in the animation team or the paint team?

Alan/Alexey: Can you comment on the particular questions raised in the initial report?
Components: -Blink>Paint
Labels: Hotlist-CodeHealth
I think this is a similar code health problem as issue 492887. Some of our KeyframeEffectReadyOnly code is written with execution outside of style recalc in mind (e.g. via Javascript) and does problematic things when executed inside style recalc.

Fixing this will not involve paint code, it is a sanity bug in animations.

Comment 4 by loyso@chromium.org, Jan 11 2017

DisableCompositingQueryAsserts hacks need to reworked.
Blockedon: 649560
Cc: wkorman@chromium.org
Owner: ----
Status: Available (was: Assigned)
Unassigning as I am not actively working on this.
Project Member

Comment 7 by sheriffbot@chromium.org, May 14 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: -loyso@chromium.org -dstockwell@chromium.org -alancutter@chromium.org -wkorman@chromium.org flackr@chromium.org petermayo@chromium.org
Status: Available (was: Untriaged)
Still relevant, still lower priority, thanks for the nudge sheriffbot.
May change based on slimming paint v1.875

Sign in to add a comment