This means that instead of recurring into DoRebuildAllStyleData, we'll
call StartRebuildAllStyleData in the middle of processing the restyle
queue (which is fine). StartRebuildAllStyleData will move the old rule
tree out of the way and immediately do a full-tree restyle, before
returning to any queue processing that might be left (the full-tree
restyle should have consumed all remaining restyle hints, but might have
posted some new ones for handling reframes that require reframing
ancestors). And, more importantly, the EndReconstruct() call to get rid
of the old rule tree won't happen until after we're done processing the
containing RestyleTracker's queue of restyles, which reduces the risk of
having dangling old style contexts and makes it easier (in bug 1110277)
to have a ReframingStyleContexts with the right lifetime.
This changes what was probably a silly design choice when I wrote the
code for 'rem'-basis handling; we shouldn't try continuing through the
rest of RestyleElement() here, but instead repost the hint to the
rebuild-all process.
This switches RebuildAllStyleData() to the normal
ProcessPendingRestyles() manner of restyle processing. This means a
rebuild-all going through this codepath (the main rebuild-all codepath)
only sets up for non-animation restyle processing once rather than doing
it twice (and potentially having reframes posted in
DoRebuildAllStyleData() that don't get processed until
ProcessPendingRestyles(), which causes a variant of bug 1110277 with
transitions on reframed elements failing to start because it doesn't
match the lifetime of the ReframingStyleContexts).
In the new way of doing a rebuild-all, StartRebuildAllStyleData might be
called directly from ProcessPendingRestyles rather than from
RebuildAllStyleData (which null-checks the root frame) or from within
processing restyles (which can only happen when there's a root frame).
This means it needs its own null-check of the root frame.
Here we call StartRebuildAllStyleData from BeginProcessingRestyles (much
like patch 9 and EndProcessingRestyles). But we will later also call it
from the code that handles a root element font size change when we have
'rem' units. That's because it's fine to *start* the rebuild process in
the middle of processing the queue of pending restyles. (We have to end
after the whole process is done, though, in order to avoid wanting to
destroy the old rule tree while we still have style contexts referencing
it.)
We only call StartRebuildAllStyleData in this case when we're processing
our primary restyle queue (mPendingRestyles), not the animation restyles
(to be removed in bug 960465) or the animation-only restyles, since a
rebuild-all should be processed (in terms of animation phases, or in
terms of having an animation-only update before it) like a normal
restyle. (This isn't true for the 'rem' unit restyle, which could
happen during any sort of update.)
This moves the code that finishes the rebuild-all process into
EndProcessingRestyles(), which is part of the main restyling codepath.
Patch 7 ensures that we'll always get to EndProcessingRestyles in this
case, when we're going through the normal ProcessPendingRestyles()
codepath rather than the special DoRebuildAllStyleData() codepath (which
will be removed later in this patch series).
This fixes one of the omissions in the rebuild-all codepaths (where it
incorrectly differs from the regular ProcessPendingRestyles codepath).
Note that the explicit FlushOverflowChangedTracker() is no longer needed
because that's part of EndProcessingRestyles.
(This will all get refactored more substantially in the following
patches.)
This is needed for patch 9 (once patch 9 is used via the
ProcessPendingRestyles() codepath in patch 13); it ensures that when we
use the new way of rebuilding, we don't bail out early because we think
we have nothing to do.
This adds a member variable that is currently only used within a single
function, but that function will be split apart so that different parts
of it can be called from different places within ProcessPendingRestyles.
This is the variable that says we *need to* rebuild style data. Since
the next patch will introduce a variable that says we're *currently*
rebuilding all style data, renaming this one makes things clearer.
Part of this refactoring involves the ability to start the rebuild-all
process within the processing of restyles. This means we can't pass
parameters directly from RebuildAllStyleData into DoRebuildAllStyleData.
So this continues storing the hints as member variables a little bit
deeper into the process.
(I tried to move in a different direction in this patch queue, and store
these hints in mPendingRestyles, for the root element. But that broke
layout/style/test/test_counter_style.html and
layout/style/test/test_font_loading_api.html, and I didn't want to
figure out why. It would be somewhat better in the long run, since
currently these hints will get processed if we do a rebuild-all on a
RestyleTracker other than mPendingRestyles, which can happen if we have
'rem' units and have a root element font size change in the
animation-only update or in mPendingAnimationRestyles.)
The patches in this series refactor the process of rebuilding all style
data (RestyleManager::RebuildAllStyleData and
RestyleManager::DoRebuildAllStyleData) so that the process of rebuilding
all style data uses the existing restyle processing loops in
ProcessPendingRestyles. (Rebuilding all style data is what we do when
we need to throw away the rule tree because something has invalidated
the cached data in it.) This removes (increasing, especially with bug
960465 coming) code duplicated between the two codepaths, fixes some
omissions from the separate rebuild-all codepath, and (more immediately)
allows fixing lifetime issues of ReframingStyleContexts objects in bug
1110277 so that we can have a single ReframingStyleContexts for all of
the restyle processing in each restyle processing operation. In other
words, the goal is to change the rebuild-all process from a separate
codepath to a few variables that modify the way ProcessPendingRestyles
works (and make it do the extra work).
This is just a small first step in that process, which moves one piece
of code from a chunk of duplicated and to-be-removed code into a chunk
of code that will be preserved.
This means that we avoid an O(N^2) calling pattern of IsVisible when we
call IsVisible for all tabs of a tabbrowser, since in a tabbrowser
(which uses an nsDeckFrame), IsVisibleConsideringAncestors is O(N) in
number of earlier tabs.
The change to GetAfterFrameForContent prevents the reframe that is part
of the chain of events leading to this bug, and thus fixes the bug on
its own. The change to GetBeforeFrameForContent seems desirable for
symmetry.
Note that patch 6 also independently fixes the reported bug.
This probably needs somewhat careful review. We should examine:
(1) what the rules for calling nsLayoutUtils::GetBeforeFrame and
nsLayoutUtils::GetAfterFrame are, and whether both (or neither)
need to be patched.
(2) What the rules are for which frame the GenConProperty() lives on,
and whether we should adjust nsIFrame::GetGenConPseudos() to either
do something more intelligent, or assert about callers.
(We should probably clean up some of these things in a followup bug.)
Since the symptom of this bug is (once patch 4 is in the tree) only
causing extra reframes, it can only be tested using the new API (from
bug 1115691) for observing reframes. I confirmed that the test for this
bug fails without the patch and passes with the patch (as noted by the
removal of its todo annotation).
This patch fixes the assertion on layout/generic/crashtests/600100.xhtml,
though I haven't investigated why.
This patch is not needed to fix the bug, but it seems like it's probably
desirable. It's not needed for this bug because
MaybeReframeForBeforePseudo and MaybeReframeForAfterPseudo are already
called (by ElementRestyler::RestyleChildren) on only the first and last
continuation or ib-split sibling with the same style. So this patch
should only actually change anything for cases like a block-in-inline
split whose initial inline part is inside of a ::first-line (where
different parts of the block-in-inline split chain have different style).
Since the symptom of this bug is (once patch 6 is in the tree) only
causing extra reframes, it can only be tested using the new API (from
bug 1115691) for observing reframes. I confirmed that the test for this
bug fails without the patch and passes with the patch (as noted by the
removal of its todo annotation).
This patch switches on the new, "actually start the player in the next refresh
driver tick" behavior. It updates PendingPlayerTracker, adding
a StartPendingPlayersOnNextTick method which calls the appropriate method on
AnimationPlayer. The existing StartPendingPlayers is renamed to
StartPendingPlayersNow and is used for testing only.
Furthermore, since we now expect AnimationPlayer::StartOnNextTick to be
functional, AnimationPlayer::DoPlay is updated to use it when there is no
document available. This should make playing an animation player always
asynchronous, that is, always transition to the pending state temporarily
(unless we are already playing).
In this patch series we adjust the behavior of animation starting so that the
animation does not actually start until the following refresh driver tick. This
requires some tweaks to tests to ensure they continue to pass.
L. David Baron
Daniel Holbert
Ethan Lin
Mats Palmgren
Kartikaya Gupta
Gabor Krizsanits
Shih-Chiang Chien
Jeremy Chen
Andrea Marchesini
Xidorn Quan
Robert O'Callahan
Jonathan Kew
Ryan VanderMeulen
Christoph Kerschbaumer
Nigel Babu
Tom Schuster
Seth Fowler
Masatoshi Kimura
Robert Longson
Ms2ger
Geoff Lankow
Wes Kocher
Chris Peterson
Neil Deakin
William Chen
Gregor Wagner
Carsten "Tomcat" Book
Fernando Jimenez Moreno
Makoto Kato
Boris Zbarsky
pchang
Ehsan Akhgari
Simon Montagu
Michael Pruett
Botond Ballo
Morris Tseng
JerryShih
Frédéric Wang
Brian Birtles
Benoit Girard
Tom Tromey
Phil Ringnalda
Andrew McCreight