I'm using MakeRefPtr nearly everywhere, except when the old code was
immediately returning foo.forget() - in those spots, I'm using MakeAndAddRef
for brevity (since that function does the forget() internally).
Differential Revision: https://phabricator.services.mozilla.com/D299013
This is mostly just a convenience shorthand to avoid running the same
boilerplate/error-checking code in the implementation of the 'width' and
'height' attributes, for the benefit of callers that need both sizes. But,
beyond that convenience: this also lets us elegantly express the case where an
image simply lacks an intrinsic width and/or height (not due to any error, but
simply because it's e.g. an SVG that has no preferred size in either or both of
its axes). This is a state we'll need to be able to detect and give special
treatment to in a forthcoming patch for bug 1935269.
Differential Revision: https://phabricator.services.mozilla.com/D248526
This patch doesn't change behavior; it just refactors out some commont logic
into a helper (so that we can add another call to this same helper in a later
patch in this series).
Differential Revision: https://phabricator.services.mozilla.com/D248525
This patch doesn't change behavior; it's just a rename.
Without this, it's easy to get confused and assume that img.intrinsicSize would
just be a shorthand for img.width and img.height -- but it's not. (It has
different units from the 'width' and 'height' attributes, and in some cases --
e.g. the VectorImage implementation -- it queries for its answer in a different
way from how 'width' and 'height' do.
(I'm doing this partly because I'll soon be adding a new attribute that *does*
just represent img.width and img.height, and that new attribute will also be
named something like "intrinsicSize"; and I want to minimize confusion between
that new attribute and this existing one.)
Differential Revision: https://phabricator.services.mozilla.com/D248492
An SVG image document might declare a negative value for `width` and/or
`height` on the root <svg> node, but we clamp that to be nonnegative for actual
layout purposes, and we should similarly clamp it in the imagelib size getters.
Otherwise we potentially expose bogus values via the web-exposed naturalWidth
and naturalHeight APIs.
Differential Revision: https://phabricator.services.mozilla.com/D248149
Based on intermittent crashes in automation, it seems like during shutdown, two
things may happen independently:
(A) we gather a memory report that asks our SVG images for their width/height.
(B) we tear down the internal SVG document for our SVG images.
If they happen in that order (A before B), then everything's fine; but if B
happens *before* A, then the width/height queries in A will find an SVG
document with a null pointer for the root element. So we need to handle that
case gracefully.
Differential Revision: https://phabricator.services.mozilla.com/D231825
AspectRatio already has a way of representing a null / invalid ratio,
and there are some cases (broken sizes or invalid SVGOuterSVGFrame
ratio) where we might get Some(AspectRatio()).
It's not a meaningfully different state, and IMO we shouldn't treat it
differently.
Differential Revision: https://phabricator.services.mozilla.com/D224212
AspectRatio already has a way of representing a null / invalid ratio,
and there are some cases (broken sizes or invalid SVGOuterSVGFrame
ratio) where we might get Some(AspectRatio()).
It's not a meaningfully different state, and IMO we shouldn't treat it
differently.
Differential Revision: https://phabricator.services.mozilla.com/D224212
When not using blob recordings for vector images we use simple surface providers. When we get an invalidation we will call SurfaceCache::InvalidateImage which will set a dirty bit on all blob recordings and remove all other surfaces from the surface cache. And if it finds a blob recording we will call ResumeHonoringInvalidations. This is not enough to invalidate in the case of a non-animated vector image using the webrender GetImageProvider path. Even though the surface isn't in the surface cache anymore it is still cached on the frame and when we tell it about the invalidation it will just has the image provider to update its key. That will call this code
https://searchfox.org/mozilla-central/rev/cdddec7fd690700efa4d6b48532cf70155e0386b/image/DecodedSurfaceProvider.cpp#222
which will just share the same surface as before the invalidation happened again. That will let us handle one invalidation but it still won't fix the bug, we need to call ResumeHonoringInvalidations so that we handle further invalidations.
When not using the image provider path, we don't save an image provider on the frame, that alone would avoid this problem. But we also call ResumeHonoringInvalidations for every successful call of VectorImage::Draw.
* * *
imported patch asvfref
Differential Revision: https://phabricator.services.mozilla.com/D174450
This code was added before we decided to do fission, when we wanted to separate out different sites that were in the same process so we could prioritize them better. We are not going down that path so we can simplify this code. There should be no change in functionality with this patch, just simpler code.
Differential Revision: https://phabricator.services.mozilla.com/D201705
Instead of starting transitions and animations as a result of a paint,
use the refresh driver tick to do this.
This sets the transition-ready time to the current time during the next
refresh driver tick that it was started on (see mSawTickWhilePending).
This is similar to what's described in the bugs comments, and seems to
work nicely in practice.
We could easily change that (current time) by a paint-based time if
needed (when available), which would be more similar to what we were
doing. But I'd rather do the simple thing for now, and land this shortly
after the soft freeze is over so that we have time to watch out for
regressions.
There's one regression on a test that birtles wrote (using an XHR doc
and switching the timeline to a rendered doc's timeline).
We use the timeline's document rather than the target document to
determine whether to trigger animations now. That's one of the cases
where we'd keep vsync perma-running without this patch, and Chrome also
fails that test. Maybe the test should be removed / the spec should be
tweaked to allow this behavior?
This causes some progression in some CSS transitions tests too, and I
added an extra test for the vsync behavior.
Over-all this is much simpler to reason about and I think we should try
to do this.
Differential Revision: https://phabricator.services.mozilla.com/D193583
- Use LengthPercentage as the return type for GetIntrinsicHeight and
GetIntrinsicWidth. This allows the caller to handle percent values.
No callers have additional logic to handle percent values as of now,
but this may change in the future, for compat; see last paragraph of
bug 700533 comment 69.
- Refactor intrinsic size function definitions.
Differential Revision: https://phabricator.services.mozilla.com/D176974
All its members are optional, so we can just use it as a plain struct
rather than Maybe<> all around, which simplifies the code and prevents
silly bugs like bug 1779592.
Mostly automatic via:
rg -l 'SVGImageContext' . | xargs sed -i 's/Maybe<SVGImageContext>/SVGImageContext/g'
With trivial build fixes.
Not intended to change behavior.
Differential Revision: https://phabricator.services.mozilla.com/D151846
We'll probably want to do something more accurate in the future with a
custom clang static analysis pass which validates that XPIDL interfaces
have the expected vtable and struct layout, however doing so would be
more involved than the string matching done in this patch.
In addition to checking for extra virtual methods, we'll likely also
want to check for data members on interfaces, and reject them unless the
class is marked as `[builtinclass]` in addition to some other attribute
which we'll need to add to prevent them from being implemented in Rust
(as c++ data members will not be reflected by the rust macro).
There were 2 instances of a comment which contained the word 'virtual'
within a CDATA block. These comments were moved out of the CDATA block
to avoid triggering the error.
Differential Revision: https://phabricator.services.mozilla.com/D151068
We're checking animation consumers, not locked status. We don't care
about animation consumer count of non-animated images.
Depends on D137765
Differential Revision: https://phabricator.services.mozilla.com/D137766
If we are shutting down, the document for a VectorImage may be cleared.
If a refresh tick raced with the shutdown, we might try to deref the
null document.
Differential Revision: https://phabricator.services.mozilla.com/D132953
Valentin Gosu