This was observed in an intermittent failure of image/test/mochitest/test_discardAnimatedImage.html. What happened was:
1) Document::MaybePreLoadImage was called for the images in the test.
2) imgRequest::OnDataAvailable is called on at least one of the images. This creates the RasterImage, so any proxy for this imgRequest will now return the image via GetImage(). imgRequest::OnDataAvailable also queues the FinishPreparingForNewPartRunnable back to the main thread to call OnImageAvailable on the progress tracker on the main thread.
3) We get the actual LoadImage calls for the images of the document. We create new proxies for the existing imgRequests. imgRequestProxy::Init calls mBehaviour->SetOwner(aOwner), which sets mOwnerHasImage to true because the progress tracker has an mImage (the one we created above).
4) We get a call to LockImage, this gets forwarded to the RasterImage because mOwnerHasImage is true and we can access the image.
5) The FinishPreparingForNewPartRunnable finally runs on the main thread. The OnImageAvailable notification from the progress tracker ends up in imgRequestProxy::SetHasImage. imgRequestProxy::SetHasImage applies our local count mLockCount to the RasterImage, even though we've already forwarded one of those LockImage calls to the image. LockImage calls are now unbalanced and the image will always remain locked.
The fix is simple. Only apply the Lock/Unlock calls if the FinishPreparingForNewPartRunnable has hit the main thread (ie ignore an image we can access until this happens).
Differential Revision: https://phabricator.services.mozilla.com/D29326
--HG--
extra : moz-landing-system : lando
The img decode API allows a web author to request that an image be
decoded at its intrinsic size and be notified when it has been
completed. This is useful to ensure an image is ready to display before
adding it to the DOM tree -- this will help reduce flickering.
Differential Revision: https://phabricator.services.mozilla.com/D11362
When the underlying image request (imgIRequest) changes for an image, we
need to ensure that we invalidate the cached WebRenderImageData such that
the image container stored therein is updated to be for the correct
image. This gets a little tricky because some display items store both
the current and previous images, and choose to display the latter if the
former is not yet ready. We also don't know what image the image
container belongs to. As such, we now compare the producer ID of the
current frame in the image container, to the expected producer ID of the
current image request. If they don't match, we must regenerate the
display list.
Differential Revision: https://phabricator.services.mozilla.com/D19699
Summary: Really sorry for the size of the patch. It's mostly automatic
s/nsIDocument/Document/ but I had to fix up in a bunch of places manually to
add the right namespacing and such.
Overall it's not a very interesting patch I think.
nsDocument.cpp turns into Document.cpp, nsIDocument.h into Document.h and
nsIDocumentInlines.h into DocumentInlines.h.
I also changed a bunch of nsCOMPtr usage to RefPtr, but not all of it.
While fixing up some of the bits I also removed some unneeded OwnerDoc() null
checks and such, but I didn't do anything riskier than that.
If class A is derived from class B, then an instance of class A can be
converted to B via a static cast, so a slower QI is not needed.
Differential Revision: https://phabricator.services.mozilla.com/D6861
--HG--
extra : moz-landing-system : lando
nsIAssociatedContentSecurity and nsISecurityInfoProvider are unused as of
bug 832834, so this patch removes them.
Differential Revision: https://phabricator.services.mozilla.com/D5693
--HG--
extra : moz-landing-system : lando
The 'x' prefix makes it clearer that these are infallible.
A couple of nsJSID methods are now also infallible.
--HG--
extra : rebase_source : fcce44a00212d6d341afbf3827b31bd4f7355ad5
This patch is an automatic replacement of s/NS_NOTREACHED/MOZ_ASSERT_UNREACHABLE/. Reindenting long lines and whitespace fixups follow in patch 6b.
MozReview-Commit-ID: 5UQVHElSpCr
--HG--
extra : rebase_source : 4c1b2fc32b269342f07639266b64941e2270e9c4
extra : source : 907543f6eae716f23a6de52b1ffb1c82908d158a
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
There are two other means from which a caller can get the current state
which originally ignored validation -- GetImageStatus and
StartDecodingWithResult. These methods are used by layout in some
circumstances to decide whether or not the image is ready to display. As
observed in some web platform tests, in particular
css/css-backgrounds-3/background-size-031.html, we may actually validate
and purge the cache for images under test. The state given by the
aforementioned methods was misleading, because validation changed it.
Now they take into account validation, and do not imply any particular
state while validation is in progress.
IProgressObserver::SetNotificationsDeferred is now used just for
ProgressTracker to track when there is a pending notification for
an observer. It has been renamed to MarkPendingNotify and
ClearPendingNotify to make a clear distinction.
When cache validation is in progress, imgRequestProxy defers its
notifications to its listener until the validation is complete. This is
because the cache may be discarded, and the current state will change.
It attempted to share the same flags with notification deferrals used by
ProgressTracker to indicate that there is a pending notification, but
this has problematic/confusing. Hence this patch creates dedicated flags
for notification deferrals due to cache validation.
The "current URL" in the spec:
https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-currentsrc
maps to imgIRequest.URI, not currentURI.
Rename imgIRequest.currentURI to finalURI to prevent such confusion.
MozReview-Commit-ID: CjBh2V4z8K9
--HG--
extra : rebase_source : 01277d16ef12845e12cc846f9dd4a21ceeca283b
The "current URL" in the spec:
https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-currentsrc
maps to imgIRequest.URI, not currentURI.
Rename imgIRequest.currentURI to finalURI to prevent such confusion.
MozReview-Commit-ID: CjBh2V4z8K9
--HG--
extra : rebase_source : d3047aed22f116ff9a74099b646a84e597388673
An imgRequestProxy may defer notifications when it needs to block on an
imgCacheValidator. It may also be cancelled before the validator has
completed its operation, but before this change, we did not remove the
request from the set of proxies, imgCacheValidator::mProxies. When the
deferral was completed, it would assert to ensure each proxy was still
expecting a deferral before issuing the notifications. Cancelling a
request can actually reset that state, which means we fail the assert.
Failing the assert is actually harmless; in release we suffer no
negative consequences as a result of this sequence of events. Now we
just remove the proxy from the validator set to avoid asserting.
imgRequestProxy::CancelAndForgetObserver was intended to always dispatch
any load group removals due to reentracy conflicts with the callers.
However in bug 1404422 the fact that imgRequest::RemoveProxy can
indirectly trigger a load group removal through completing an
incompleted request.
Historically imgRequestProxy::PerformClone would only add the cloned
request to the (original proxy's) document's load group if the request
was still being validated. Now it adds the cloned request to the given
document's load group before requesting the notifications, unless the
request has already been completed. We ensure that any removals from
the load group occur outside the current execution context.
Legacy listeners may use imgRequestProxy::SyncClone to request
notifications on the image state. Ideally they would not, but they do
not work as expected with the asynchronous notifications all new callers
must use. While in theory this would suggest their code is re-entrant,
not all of it is. In particular we need to be sensitive about when we
remove a request from a load group.
There should be no functional change here, but we rely upon the new
structure in the next patch in the series. This separates out the
notions of removing a request from the load group (which is always
final, and must be executed outside of synchronous calls from the owner
of the imgRequestProxy) and wanting to readd a request to the load group
as a background request (for multipart images).
The most important addition is mForceDispatchLoadGroup which if true
when imgRequestProxy::RemoveFromLoadGroup is called, will dispatch the
removal from the load group instead of executing it inline. This ensures
safety for any callers (e.g. to CancelAndForgetObserver) as above.
imgRequestProxy::SetLoadGroup did not have a predictable effect and
it appears to be unused. It is somewhat complicated to support given
we must be sensitive about what context we execute removing the
request from the original load group.
imgRequestProxy::IsOnEventTarget must return false in order for imgRequestProxy::Dispatch to be called. Typically we check for mListener before any of this but in imgRequest::OnLoadComplete, we have other things to do besides notifying the listener. As such, we want to dispatch even if there is no listener, and that is when the assert can fail. Since IsOnEventTarget can only return false if it has either a tab group *or* a listener, we can change the assert to match.
imgRequestProxy::SyncClone preserves the original behaviour of issuing
synchronous notifications once cloned. Some uses and tests depend on
this behaviour but in an ideal world, it would not be required.
imgRequestProxy::Clone is intended to be the replacement going forward,
which issues asynchronous notifications once cloned.
During painting we do some image decoding, but we want to send the image progress notifications from that decoding async. The CSS image renderer checks if the image is complete before painting it. So if the decoding we did during painting resulted in the images becoming complete there is no way to tell that during the same paint. Thus making that decoding a waste of time.
So we add a limited way of telling if the result of a StartDecoding call has resulting in an image that is ready to paint so we can get that result during the same paint.
I would have prefered to change StartDecoding to just return a bool but that would have made the bool an outparam, which would make every StartDecoding call uglier with extra code. Changing it to a notxpcom function would have fixed that, but I'm not sure if that is safe.
Andrea Marchesini
Coroiu Cristina
Timothy Nikkel
Andrew Osmond
Olli Pettay
Emilio Cobos Álvarez
Sylvestre Ledru
Andrew McCreight
Dana Keeler
Nicholas Nethercote
Chris Peterson
Josh Matthews
Samael Wang
Noemi Erli
Jonathan Watt
Honza Bambas
Ehsan Akhgari
Ehsan Akhgari
Shih-Chiang Chien
Eric Rahm
Sebastian Hengst
Bill McCloskey