1189598 - Finished CSS transition still added to pending restyles

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Ting-Yu Chou [:ting] (away)]

Attachment: min_repro.zip

I attached a test page (min_repro.zip), which can reproduce the issue by following steps:

  1. Open index.html, #unseen's background color is changed from yellow to black
  2. Drag the white rectangle up and down
  3. Every time changing translateY of #rect, #unseen is added to pending restyle and processed

nsTransitionManager::ConsiderStartingTransition() calls CommonAnimationManager::GetAnimations() which adds #unseen to its |mElementCollections|. But it is not removed from the collections after the transition is finished. Later when user drags the rectangle and RestyleManager::ProcessPendingRestyles() comes in, CommonAnimationManager::AddStyleUpdatesTo() will find #unseen from the collections and add it to pending restyle.

Note this could be a cause of bug 1183361 and bug 1189004.

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

AddStyleUpdatesTo should probably do:

  if (!collection->mNeedsRefreshes) {
    continue;
  }

before calling EnsureStyleRuleFor.

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think this should help:

https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/91858e7cbc8e/animation-needs-refreshes

Comment 3

[Ting-Yu Chou [:ting] (away)]

Just curious, why in this case not removing the collection after transition is finished?

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to Ting-Yu Chou [:ting] from comment #3)
> Just curious, why in this case not removing the collection after transition
> is finished?

See https://hg.mozilla.org/mozilla-central/rev/b2ee72589c18 (commit message and first code comment, mainly).

Comment 5

[Ting-Yu Chou [:ting] (away)]

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #2)
> I think this should help:
> 
> https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/
> 91858e7cbc8e/animation-needs-refreshes

Now AddStyleUpdatesTo() does not add #unseen to pending restyle. But I can still see RestyleManager::PostRestyleEvent() is called with #unseen, which is from [1].

[1] https://dxr.mozilla.org/mozilla-central/source/layout/style/nsTransitionManager.cpp#980

Comment 6

[Brian Birtles (:birtles)]

(In reply to Ting-Yu Chou [:ting] from comment #5)
> Now AddStyleUpdatesTo() does not add #unseen to pending restyle. But I can
> still see RestyleManager::PostRestyleEvent() is called with #unseen, which
> is from [1].
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/layout/style/
> nsTransitionManager.cpp#980

I have plans to fix that after bug 1188251 (the current patches have a FIXME describing that optimization).

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think this is a regression from bug 960465 (which retained finished transitions), although I suppose it could be more recent.