I don't know of any observable bug that this fixes, but the code without
this fix seems incorrect; the "removed sentinel" concept generally
requires that callers enumerating transitions check that they're not
enumerating the sentinel.
This ensures that HasAnimationOfProperty switches from returning true to
false in the first refresh cycle after the end of the animation rather
than the second.
I originally wrote this in
https://bugzilla.mozilla.org/show_bug.cgi?id=876626#c13 but it turned
out not to be related to that bug.
While debugging bug 858937 I noticed that the transition manager was
calling nsIFrame::SetStyleContextWithoutNotification rather than
nsIFrame::SetStyleContext. SetStyleContextWithoutNotification should
only be used for things that aren't really style changes, but are
instead changes we make during frame construction before things are
really initialized. Anything that's really a dynamic style change, as
these are, should use SetStyleContext.
I realize I said the opposite in bug 780692 comment 186, and bz said the
same in bug 780692 comment 204, which is why this is the state that it
is.
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).
Note that this patch has a little bit of a belt-and-braces aspect to it.
In each file, either one of the changes should be sufficient, but one of
them prevents us from doing unneeded work and the other one ensures that
we never apply style resulting from transitions and animations even if
somehow we do that work.
Also note that the tests don't actually test anything usefully, since
the reftest harness doesn't currently make the pres context non-dynamic.
(Thus they're marked as failing.) I'm not sure what I should do about
that, though I'm considering just deleting the tests entirely.
Except for the changes in:
layout/generic/nsIFrame.h (part)
layout/style/nsComputedDOMStyle.h (all)
layout/style/nsRuleNode.cpp (part)
layout/style/nsStyleContext.cpp (part)
layout/style/nsStyleContext.h (part)
(see patch 3b in the bug), this patch was written with the sed script:
s/\<GetStyle\(Font\|Color\|List\|Text\|Visibility\|Quotes\|UserInterface\|TableBorder\|SVG\|Background\|Position\|TextReset\|Display\|Content\|UIReset\|Table\|Margin\|Padding\|Border\|Outline\|XUL\|SVGReset\|Column\)\>/Style\1/g
This makes it conform to our convention that getters returning pointers
that can never be null do not begin with "Get".
nsStyleContext's rule node is never null because we require a rule node
in order to construct a style context.
The CalcStyleDifference call is absolutely necessary even if we didn't
need to process the change list, because it causes the new style
context to have cached structs that we might need for a later
comparison. This is important because, as an optimization, we only
compare structs that have been retrieved. This optimization requires
that when we replace a style context, we fetch all the structs on the
new style context that had been fetched on the old style context (which
is normally necessary anyway in order to do comparison so we can process
the changes appropriately).
However, actually processing the change list is also necessary to fix
the bug; it's the actual change from the miniflush that matters here.
Based on dholbert's debugging information, I think it's mostly likely
because we were failing to process the UpdateOverflow hint.
