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
--HG--
extra : rebase_source : 459e8efeb171adad089d94272e143e8c244bd279
extra : source : 65ba2f174fcf7dba4e59c00ee8908b1bd0820a48
Everyone calls them with the shell of the current composed document, and this
allows the multi-presShell stuff to just be in UpdateCurrentStyleSources /
DoGetStyleContextNoFlush.
The only reason we need to use OwnerDoc()->GetShell() instead of the composed
doc in GetStyleContext / GetStyleContextNoFlush is Element::GetBindingURL, which
does expect to get the binding URL for stuff outside of the composed doc (and
changing that gave me a useless browser).
That's technically a behavior change on the cases that used to pass nullptr, but
I think all callers are fine with that. I could also just add a special function
for that particular case, it may be worth it.
MozReview-Commit-ID: 2XlnkgdgDCK
It would be convenient to get nsPresContext from nsIDocument.
MozReview-Commit-ID: Ei6V3UE8XGr
--HG--
extra : rebase_source : 8d2a917eb62cf341e4e1810451fd01c01dbc3bad
There are a few different reasons why tests needed updating (not an exhaustive list):
- Tests assume that successive operations take place at different times.
- Tests assume that an operation took a minimum amount of time.
- Tests hardcodes a specific delay.
In most cases we hardcode the preference off. In some cases this is the best approach,
in others, we would like to improve. The bug for tracking those improvements is Bug 1429648
An improvement that is present in some tests is to hardcode a specific precision reduction
that is acceptable based on the confides of the test. (Obviously this needs to be a fix for
the test framework and not a requirement on the feature being tested.)
In a few places, the test itself can be fixed, for example to no longer require the end
time of an operation to be strictly greater than the start time, and allows it to be equal
to it.
MozReview-Commit-ID: J59c7xQtZZJ
--HG--
extra : rebase_source : df8a03e76eaf9cdc9524dbb3eb9035af237e534b
This matches the behavior of Chrome at least, and should reduce compatibility
risk when we remove accessKey support in the next patch in this series by
ensuring that specifications such as begin="3s; accessKey(s)" will continue to
run the animation.
MozReview-Commit-ID: E6sh48yrYSm
--HG--
extra : rebase_source : 1ad85b55acfd226c660386ff579811ed2e70f260
We have ServoCSSParser class, and I think it's better to move those
Servo FFI into this class to avoid including ServoBindings.h everywhere.
MozReview-Commit-ID: 6orXtddp9ZU
--HG--
extra : rebase_source : 6da4158c4fec606aaee49fddee3192f94d6c85a3
This assertion was originally added in bug 1353208 because in that bug we
changed the type of nsSMILCompositor::mCachedBaseValue from
nsAutoPtr<nsSMILValue> to just nsSMILValue. When using nsAutoPtr,
mCachedBaseValue had two null states: one where the pointer is null, and one
where the pointed-to nsSMILValue is null. Coalescing these two states simplifies
the code but there is one case where the difference is significant as described
in the commit message for that changeset (mozilla-central changeset
ad7060dae117):
"There's a subtle difference in behavior with regards to the first sample.
Previously we would compare the (initially) null mCachedBaseValue pointer with
the passed-in nsSMILValue and set mForceCompositing to true. With this patch,
however, we will only set mForceCompositing to true if the passed-in
mCachedBaseValue is not null."
That is, if the base value we get back is a null nsSMILValue, previously we
would set mForceCompositing to true unconditionally, but with the changes in bug
1353208 we would only set that to true if the passed-in nsSMILValue was not
null.
We believed that would never matter since the passed-in nsSMILValue would never
be null if we called GetBaseValue. Quoting from that same commit message:
"... if we do call GetBaseValue the result should not be a null nsSMILValue
(except in some OOM cases where we don't really care if we miss a sample).
This patch adds an assertion to check that GetBaseValue does, in fact, return
a non-null value. (I checked the code and this appears to be the case. Even in
error cases we typically return an empty nsSMILValue of a non-null type. For
example, the early return in nsSMILCSSProperty::GetBaseValue() does this.)"
We added an assertion to validate that assumption but the crashtest included in
this patch demonstrates a case where it does not hold (specifically, when
nsStyleUtil::CSPAllowsInlineStyle returns false, nsCSSProperty::GetBaseValue
will return a null nsSMILValue).
That would seem to suggest that there is at least one case where we might fail
to set mForceIsCompositing to true and hence fail to update style on this first
sample (and presumably thereonwards too since future comparisons of
mCachedBaseValue will compare equal). However, for the case of an initial sample
mForceCompositing should already be set to true since set we update
mForceCompositing in nsSMILCompositor::GetFirstFuncToAffectSandwich() and will
make it true if *anything* in the animation function has changed and at this
point, the initial sample, *everything* will have changed. Hence, I believe
dropping this assertion is acceptable.
I have confirmed that in the crashtest in this patch, during the first sample
mForceCompositing is set to true.
I would create a reftest to test the behavior on the first sample but, at least
for the specific case where inline style is disabled due to CSP, not updating
style *is* the expected behavior so there will be no difference in behavior
regardless of whether or not the mForceCompositing flag is set.
MozReview-Commit-ID: Li0pZEH2PNl
--HG--
extra : rebase_source : a1c12a019b8481600afa4295447dc1e6fb281b22
In some circumstances it is possible to sample a timed element in the active
state with a time that precedes is current interval.
One possible sequence of steps leading to this situation is as follows:
1. A timed element (e.g. <set>, <animate>) with a non-zero begin time is the
child of <svg> element A (its "time container") but has yet to be sampled.
2. In order to resolve its initial interval, the timed element registers a
startup milestone with its time container at time 0.
3. However, before the sample is performed where the timed element's initial
current interval is resolved, <svg> element A is detached from the document
tree.
4. The timed element is then attached to a different <svg> element B that has
a current time greater than the begin time of the timed element and less than
that of <svg> element A.
5. Since the timed element is still in its startup state it registers its
startup milestone again, this time with its new time container, i.e. <svg>
element B.
6. A tick occurs or the document has its style flushed such that a sample is
performed.
This includes running the milestone sample which causes the timed element to
resolve its initial current interval. Furthermore the subsequent regular
sample of the timed element causes it to transition into its active state
because the current time of <svg> element B is greater than the begin time of
the timed element.
7. <svg> element A is re-attached to the document.
8. When we go to run the next sample, we iterate through all time containers
associated with the document's animation controller which includes both <svg>
element A, and <svg> element B.
9. <svg> element A renders up its 0 milestone from step (2) since it has yet to
run it. It converts this to parent time, i.e. the time space of the animation
controller, which will be zero or less depending on the current time of <svg>
element A when it was re-attached.
10. Since the milestone from <svg> element A will be the earliest milestone
time, it will be used as the next milestone sample time.
11. The timed element is then sampled using this time, but first it is converted
to a time in the time space of the timed element's time container, which is
now <svg> element B.
As a result of this conversion, the sample time may end up being *before*
the beginning of the timed element's current interval. Since timed elements
never expect the time to go backwards an assertion fails when it detects
that it is active, but is being sampled before its current interval.
For this particular case, ignoring the "early" sample seems to be the most
appropriate action.
More generally, however, we can anticipate other cases similar to this where
milestones are registered that cause the sample time to temporarily go
backwards. A quick audit of nsSMILTimedElement::DoSampleAt suggests that, with
the code changes from this patch, that is probably ok.
As an alternative we could, perhaps, try to drop and re-create all milestones
when time containers are re-attached to the document tree but that would add
more complexity and would not necessarily cover other similar cases of this
situation.
I have verified that the crashtest included in this changeset fails without the
code changes also in this changeset.
MozReview-Commit-ID: KKGYRayNkpo
--HG--
extra : rebase_source : 832d4b357a2a2fe07abf9eab3a6046599aff3ef5
Using a copy constructor here means that reallocating hashtables
containing nsSMILCompositor does a bunch of unnecessary work; we can use
a move constructor to avoid a lot of that unnecessary work.
Defining Singleton() in the declaration of nsSMILNullType implicitly
sticks an "inline" on the function, which is not what we want: inlining
it spreads around a lot of static initialization code. Providing an
out-of-line definition is much better in terms of code size.
(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
--HG--
rename : xpcom/ds/nsIAtom.h => xpcom/ds/nsAtom.h
extra : rebase_source : ac3e904a21b8b48e74534fff964f1623ee937c67
I have verified that this fails without the code changes from the previous patch
in this series, and passes with them.
MozReview-Commit-ID: 1respvNVQaC
--HG--
extra : rebase_source : 82007792f23465edb1d286b721edeea850e2aaa3
In some rare cases we can end up with both arguments passed to
nsSMILCSSValueType::SandwichAdd being "empty", that is, having the type
nsSMILCSSValueType but a null pointer.
This can happen, for example, when we have a two by-animations and linear
interpolation causing us to pass the "empty" from-value up as the underlying
value from the first by-animation to the second by-animation (which it will try
to add with its empty from-value).
In this case the result of adding "empty" to "empty" should just be "empty"
which we achieve by just doing an early return (since the fallback behavior for
failed addition is to just use the second argument as the result; note that if
we return true though the result would also be the same).
We don't currently have a test case that produces this for interpolate but it
makes sense that trying to interpolate "empty" to "empty" should likewise fail
and just produce "empty". In this case failing will make us fall back to
discrete animation and just use the "empty" values as-is. Indicating failure,
however, has the additional effect of making us use the special keyTimes
behavior defined for discrete animation.
MozReview-Commit-ID: IZ5qg0Mk5Uy
--HG--
extra : rebase_source : 402e881cd8639885568e0a32fb49aa4dfa7cbbde
Normally when we interpolate values for CSS properties we convert empty values
(such as the empty "from" value created for a by-animation) to a suitable zero
value. However, when we use discrete interpolation that step never occurs and in
some rare cases that means we end up setting the empty value on the target
attribute directly which will have an incorrect result (e.g. setting "" for the
height property will have no effect, whereas setting "0px" will).
This patch makes us perform the empty value to zero value conversion when
performing discrete animation.
Unfortunately it is not possible to test this behavior with shorthands since
there are currently no shorthand properties that are additive.
MozReview-Commit-ID: JFT1tis1RAR
--HG--
extra : rebase_source : cc444b6d4d5571c8ab231d88c4d349ea0713baaa
This patch merges nsAtom into nsIAtom. For the moment, both names can be used
interchangeably due to a typedef. The patch also devirtualizes nsIAtom, by
making it not inherit from nsISupports, removing NS_DECL_NSIATOM, and dropping
the use of NS_IMETHOD_. It also removes nsIAtom's IIDs.
These changes trigger knock-on changes throughout the codebase, changing the
types of lots of things as follows.
- nsCOMPtr<nsIAtom> --> RefPtr<nsIAtom>
- nsCOMArray<nsIAtom> --> nsTArray<RefPtr<nsIAtom>>
- Count() --> Length()
- ObjectAt() --> ElementAt()
- AppendObject() --> AppendElement()
- RemoveObjectAt() --> RemoveElementAt()
- ns*Hashtable<nsISupportsHashKey, ...> -->
ns*Hashtable<nsRefPtrHashKey<nsIAtom>, ...>
- nsInterfaceHashtable<T, nsIAtom> --> nsRefPtrHashtable<T, nsIAtom>
- This requires adding a Get() method to nsRefPtrHashtable that it lacks but
nsInterfaceHashtable has.
- nsCOMPtr<nsIMutableArray> --> nsTArray<RefPtr<nsIAtom>>
- nsArrayBase::Create() --> nsTArray()
- GetLength() --> Length()
- do_QueryElementAt() --> operator[]
The patch also has some changes to Rust code that manipulates nsIAtom.
MozReview-Commit-ID: DykOl8aEnUJ
--HG--
extra : rebase_source : 254404e318e94b4c93ec8d4081ff0f0fda8aa7d1
