I've been wanting to remove this code for a while. I think this code is
problematic for three reasons:
(1) It's in the middle of code where it doesn't belong, and which ought
to be handling purely-style-system things. (This is blocking me
from reusing that code elsewhere, e.g., in bug 977991 and
bug 960465, both of which could use it in some form.)
(2) It defeats the optimization from bug 790505 whenever we do a
miniflush (in other words, whenever we have any style change,
whether or not it's related)
(3) It means the conditions for when we decide to ship a new set of
animation data to a layer doesn't cover all the cases the layer
needs it. In particular, we only run this miniflush code when we
have a currently running animation or transition that's running on
the compositor thread. On the other hand, the UpdateTransformLayer
style change handling in DoApplyRenderingChangeToTree depends on
whether the frame currently has a transform layer, which can
continue to be true for a bit after the animation stops. So if we
need to send animations to the layer because of a transform style
change that happens soon after an animation completes, our style
change handling will find the existing layer and call its
SetBaseTransformForNextTransaction method but never do anything
that triggers layer construction. The style throttling code, in
turn, will never stop doing main thread updates because the
animation generation on the layer is out-of-date, and these main
thread updates will keep the layer active, but they'll never show
up because the stale animation data overrides the new transform
that we've been setting. (At least, I think that's what was
happening; it makes sense to me and matches the behavior I was
observing. I didn't verify which main thread updates and which
layer updates were actually happening, though.)
This shows up, for example, in the animation in attachment 8384813
just halting at a corner if I'm careful not to disturb it. (I'm
testing on Linux, with both accelerated layers and OMT animations
explicitly enabled.)
I think there are probably some other things that can be removed as
followups to removing this code, because I think we made some boundary
conditions intentionally incorrect so that problem (3) above wouldn't be
as bad as it otherwise would have been.
This moves restyling management out of nsCSSFrameConstructor (thus
reducing its size), and keeps the restyling code closer together.
This is the first of two big chunks of code moved in this patch series.
A later patch in this series will move related code from nsFrameManager
into the same destination file.
The fixes to the miniflush code
(nsTransitionManager::UpdateThrottledStyle and UpdateAllThrottledStyles)
fix the case where we constructed totally incorrect style contexts for
outer table frames (which have special style contexts inheriting from
the table frame) during the miniflush, leading to inconsistent style
data and other bad things, when we should have been touching the style
on the table frame instead.
The fixes to the other OMTA codepaths lead to layer tests being
performed on the same frame that the styles will be applied to, and
probably fix real bugs (which would occur when animating opacity or
transform on a table).
The fixes to the miniflush code
(nsTransitionManager::UpdateThrottledStyle and UpdateAllThrottledStyles)
fix the case where we constructed totally incorrect style contexts for
outer table frames (which have special style contexts inheriting from
the table frame) during the miniflush, leading to inconsistent style
data and other bad things, when we should have been touching the style
on the table frame instead.
The fixes to the other OMTA codepaths lead to layer tests being
performed on the same frame that the styles will be applied to, and
probably fix real bugs (which would occur when animating opacity or
transform on a table).
