Much in the spirit of bug 1442207.
They're not only unneeded, and cheap to get, but also we call them
inconsistently with the light DOM and flattened tree parent (like ContentRemoved
for display: contents), so they're really confusing, and kind of a footgun.
MozReview-Commit-ID: 9u3Kp8Kpp5i
This patch basically does:
* remove StyleSetHandle and its corresponding files
* revisit #includes of related header files and change correspondingly
* change nsIPresShell::mStyleSet to be UniquePtr<ServoStyleSet>
* change the creating path of ServoStyleSet to pass UniquePtr
* change other mentions of StyleSetHandle to ServoStyleSet*
* remove AsServo() calls on ServoStyleSet
Some unfortunate bits:
* some methods of (Servo)StyleSet only accepts ServoStyleSheet while
many places call into the methods with StyleSheet, so there are many
->AsServo() added to sheets
MozReview-Commit-ID: K4zYnuhOurA
-Wmissing-prototypes is a new optional warning available in clang ToT. It warns about global functions that have no previous function declaration (e.g. from an #included header file). These functions can probably be made static (allowing the compiler to better optimize them) or they may be unused.
Confusingly, clang's -Wmissing-prototypes is equivalent to gcc's -Wmissing-declarations, not gcc's -Wmissing-prototypes. A function prototype is a function declaration that specifies the function's argument types. C++ requires that all function declarations specify their argument types, but C does not. As such, gcc's -Wmissing-prototypes is a C-only warning about C functions that have no previous function *prototypes* (with argument types), even if a previous function *declaration* (without argument types) was seen.
MozReview-Commit-ID: FGKVLzeQ2oK
Deletion at the end of a text-node ends up translated to an empty append. It's
harmless though.
Reviewers: xidorn
Bug #: 1442506
Differential Revision: https://phabricator.services.mozilla.com/D667
MozReview-Commit-ID: DqheOYVWx8o
This unfortunately doesn't fix my test-case (because we're replacing the text
content all the time and all that), but it's still worth it, since it fixes the
case we care about (the parser appending).
We could also optimize pure insertions (since in that case we can still figure
out what the old text was), but it's probably annoying and not worth the churn.
In any case, we cannot optimize anything that resembles any kind of removal,
because from there we don't know the old text in any way (and the text nodes
like to reuse string buffers and such).
We could do two other optimizations to replace / extend this one, in that order:
* Pass the buffer and length to CharacterDataWillChange, and use that to get
the exact old text and the new one in RestyleManager. That would make the
optimization exact.
* Pass some sort of Maybe<bool> mWasWhitespace down the CharacterDataChangeInfo
which is computed like:
HasFlag(NS_CACHED_TEXT_IS_ONLY_WHITESPACE)
? Some(NS_TEXT_IS_ONLY_WHITESPACE)
: Nothing()
It's not clear to me it's going to be completely worth the churn, so I haven't
done those yet, if we see code in the wild which resembles my testcase, we can
think of doing it.
MozReview-Commit-ID: 2rTWaZti8rv
We used to do it this way effectively until I fixed it in bug 1400936.
Per the list of fuzz bugs that bug has in the "Depends on" field, some of those
without a super-clear fix, and others that aren't listed in there, and all the
complexity we had to deal with while receiving restyle requests mid-unbind, etc,
I think this is the right call.
This clears data on RestyleManager::ContentRemoved for non-anonymous nodes, and
on UnbindFromTree for subtrees rooted at anonymous nodes.
This will hopefully yield enforceable invariants.
MozReview-Commit-ID: IMwX5Uh1apv
We used to do it this way effectively until I fixed it in bug 1400936.
Per the list of fuzz bugs that bug has in the "Depends on" field, some of those
without a super-clear fix, and others that aren't listed in there, and all the
complexity we had to deal with while receiving restyle requests mid-unbind, etc,
I think this is the right call.
This clears data on RestyleManager::ContentRemoved for non-anonymous nodes, and
on UnbindFromTree for subtrees rooted at anonymous nodes.
This will hopefully yield enforceable invariants.
MozReview-Commit-ID: IMwX5Uh1apv
This patch does basically throttle animations on visibility:hidden element
and unthrottle it once the animating element became visible or a child of the
animating element became visible. But still there are some cases that we don't
throttle such animations perfectly. For example;
div.style.visibility = 'hidden'; // the 'div' has no children at this moment
div.animate(..);
// The animation is throttled
div.appendChild(visibleChild);
// The animation isn't throttled
visibleChild.style.visibility = 'hidden';
// Now the animation should be throttled again, but actually it's not.
To throttle this case properly, when the |visibleChild|'s visibility changed
to hidden, we would need to do either
1) Check all siblings of the |visibleChild| have no visible children
or
2) The parent element stores visible children count somewhere and decrease it
and check whether the count is zero
To achieve 1) we need to walk up ancestors and their siblings, actually it's
inefficient.
2) is somewhat similar to what we already do for animating images but it's hard
to reuse it for CSS animations since it does not take into account that
descendants' visibilities.
Another example that this patch does not optimize is the the case where
animating element has children whose visibility is inherited and the element
itself initially visible something like this;
let child = document.createElement('div'); // child visibility is 'inherit'
div.appendChild(child);
div.animate(..); // the 'div' is visible
// The animation isn't throttled since the animating element is visible
div.style.visiblily = 'hidden';
// Now the animation should be throttled, but it's not since this patch does
// not descend down all descendants to check they are invisible or not when the
// animating element visibility changed to hidden.
This patch adds a test case for this case introduced with todo_is().
Another test case added in this patch fails if we don't use
nsPlaceholderFrame::GetRealFrameFor() in HasNoVisibleDescendants().
MozReview-Commit-ID: BJwzQvP9Yc4
This new change hint doesn't influence layout so that it can be regarded
as nsChangeHint_Hints_CanIgnoreIfNotVisible. Note that if visibility changed
from collapse or to collapse, we set NS_STYLE_HINT_REFLOW separetely.
MozReview-Commit-ID: AirDWeBYVKG
This patch does basically throttle animations on visibility:hidden element
and unthrottle it once the animating element became visible or a child of the
animating element became visible. But still there are some cases that we don't
throttle such animations perfectly. For example;
div.style.visibility = 'hidden'; // the 'div' has no children at this moment
div.animate(..);
// The animation is throttled
div.appendChild(visibleChild);
// The animation isn't throttled
visibleChild.style.visibility = 'hidden';
// Now the animation should be throttled again, but actually it's not.
To throttle this case properly, when the |visibleChild|'s visibility changed
to hidden, we would need to do either
1) Check all siblings of the |visibleChild| have no visible children
or
2) The parent element stores visible children count somewhere and decrease it
and check whether the count is zero
To achieve 1) we need to walk up ancestors and their siblings, actually it's
inefficient.
2) is somewhat similar to what we already do for animating images but it's hard
to reuse it for CSS animations since it does not take into account that
descendants' visibilities.
Another example that this patch does not optimize is the the case where
animating element has children whose visibility is inherited and the element
itself initially visible something like this;
let child = document.createElement('div'); // child visibility is 'inherit'
div.appendChild(child);
div.animate(..); // the 'div' is visible
// The animation isn't throttled since the animating element is visible
div.style.visiblily = 'hidden';
// Now the animation should be throttled, but it's not since this patch does
// not descend down all descendants to check they are invisible or not when the
// animating element visibility changed to hidden.
This patch adds a test case for this case introduced with todo_is().
Another test case added in this patch fails if we don't use
nsPlaceholderFrame::GetRealFrameFor() in HasNoVisibleDescendants().
MozReview-Commit-ID: BJwzQvP9Yc4
This new change hint doesn't influence layout so that it can be regarded
as nsChangeHint_Hints_CanIgnoreIfNotVisible. Note that if visibility changed
from collapse or to collapse, we set NS_STYLE_HINT_REFLOW separetely.
MozReview-Commit-ID: AirDWeBYVKG
(Path is actually r=froydnj.)
Bug 1400459 devirtualized nsIAtom so that it is no longer a subclass of
nsISupports. This means that nsAtom is now a better name for it than nsIAtom.
MozReview-Commit-ID: 91U22X2NydP
We do return RepaintFrame change hint from nsStyleEffects::CalcDifference for
the case where we optimize 0.99 over opacity values. We also need RepaintFrame
change hint for the opacity optimization in SVG, but we can't return the change
hint from CalcDifference() since we have no way to know whether we are
calculating change hint for the optimized frame, so we fix it up in
ProcessRestyledFrames() instead.
MozReview-Commit-ID: 5GUaN9ecfoC
We currently use the aFlags argument of ContentRemoved for two purposes:
(1) To determine when a frame is being removed due to its element being removed
from the DOM, so we reframe its now-possibly-no-longer-suppressed
whitespace siblings as needed.
In other cases, our ContentRemoved call will be followed by a
ContentInserted call, which will end up doing AddTextItemIfNeeded() to
generate the relevant textframes if they're necessary.
Since we only need to tell apart the "DOM removal" and "anything else"
cases, we don't need to thread the aFlags argument through all the ways
ContentRemoved can call itself (on an ancestor).
All those cases should just be treated as "not DOM removal". In particular,
even if the original call _was_ for a DOM removal, if we convert it to an
ancestor reframe, which will call ContentInserted on the ancestor as well,
we don't need to do anything with text siblings of the ancestor.
(2) To save the frame tree state from DestroyFramesFor, but the frame tree
state is unconditionally captured on RecreateFramesForContent, so we only
need to care about it in the original ContentRemoved call.
Because of that, we can move that to the callsite, patch incoming for that.
MozReview-Commit-ID: Gy5IhUysBlz
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
There's only one case of sync frame construction from ContentRemoved now, and
it's not on the element being removed, but on the whitespace siblings if needed,
and _only_ when they don't support lazy frame construction.
Basically, this switches all the RecreateFramesForContent calls to use
`aAsyncInsert` (which I changed to an enum class for readability), except when
we're already reframing.
Also, it switches ReframeTextIfNeeded to opt-in into lazy frame construction,
since it's used only when aFlags == CONTENT_REMOVED.
This allows to simplify the DestroyFramesFor API (which I'm happy to rename to
something more meaningful, since now it's something like
DestroyFramesForAndRecreateThemAsync), and do some other consistency cleanups.
A bunch of the ContentRemoved callsites were pretty random at passing
aAsyncInsert, and that was some kind of a mess. This patch ensures consistency,
and makes it impossible to do O(n^2) work when removing DOM nodes, which is
nice.
The underlying reason for this is explained in the description of bug 1377848,
and basically allows us to remove a bunch of Servo hacks on the longer term (a
few of them are going away already, yay!).
MozReview-Commit-ID: 2DrUTxGV8RX
CSS animations / transitions and element.animate animations of opacity will
still be treated as animations.
JS-implemented opacity animations will no longer be detected as animations.
I hope that's fine. The current heuristic makes us detect opacity 'animations'
in lots of cases where there isn't an actual animation, and the resulting
layerization changes from those detected animations cause expensive repaints.
MozReview-Commit-ID: KJlc6c8OWSP
