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
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.
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
This patch adds three telemetry scalars to track how WebP is used. All
of these scalars are updated when we do the MIME type confirmation for
an imgRequest when the first data comes in. We know at this point we
decided to load the given content, so there should be minimal false
positives for data the browser loaded but never displayed.
The first two scalars are merely whether or not WebP was observed. One
is for probes, which are tiny WebP images suggested by the Google WebP
FAQ to probe for different aspects of WebP support (lossy, animated,
etc). We want to count this separately as actual WebP content that the
website wishes us to display. Probes will give a measure of how many
users visit websites that probe for WebP support, and content will give
a measure of how many websites don't care and just give us WebP images
regardless.
The third scalar is intended to give a relative measure of how many WebP
images we are being served relative to all other image types. We expect
the ratio to be small, but it would be good to confirm this from the
data.
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
imgLoader::ValidateEntry would aggressively determine an entry has
expired, even when the request hasn't yet begun. This is because the
expiration time for the entry was not set unless it was for a channel
which supports caching. Now we set the expiration time for all
channels, and if it doesn't support caching, it just expires at the
current time when imgRequest::OnStartRequest is called. Additionally,
imgLoader::ValidateEntry will not consider the expiration time in the
entry until it is non-zero.
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
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.
The imgLoader code consistently uses the term 'loadingPrincipal' for the
principal that is called the triggeringPrincipal everywhere else it's used.
This is confusing, and since we need to make changes to how those values are
determined, it should be fixed beforehand.
MozReview-Commit-ID: 8CTHwayzcaD
--HG--
extra : rebase_source : d4405b0ecfe1c8dfb9bfdf61fe6ed6cfb180ba83
Currently we only permit requests from HTTP channels to be retargeted to
the image IO thread. It was implemented this way originally in bug
867755 but it does not appear there was a specific reason for that.
The only kink in this is some browser chrome mochitests listen on debug
build only events to ensure certain chrome images are loaded and/or
drawn. As such, this patch ensures that those observer notifications
continue to be served, requiring a dispatch from the image IO thread to
the main thread.
Another issue to note is that SVGs must be processed on the main thread;
the underlying SVG document can only be accessed from it. We enforce
this by checking the content type. The possibility already exists that
an HTTP response could contain the wrong content type, and in that case,
we fail to decode the image, as there is no content sniffing support for
SVG. Thus there should be no additional risk taken by using the image IO
thread from other non-HTTP channels (if they don't specify the SVG
content type, it is not rendered today, and if they do, it will remain
on the main thread as it is today).
We also ignore data URIs. The specification requires that we process
these images sychronously. See bug 1325080 for details.
All the SizeOf{In,Ex}cludingThis() functions take a MallocSizeOf function
which measures memory blocks. This patch introduces a new type, SizeOfState,
which includes a MallocSizeOf function *and* a table of already-measured
pointers, called SeenPtrs. This gives us a general mechanism to measure
graph-like data structures, by recording which nodes have already been
measured. (This approach is used in a number of existing reporters, but not in
a uniform fashion.)
The patch also converts the window memory reporting to use SizeOfState in a lot
of places, all the way through to the measurement of Elements. This is a
precursor for bug 1383977 which will measure Stylo elements, which involve
Arcs.
The patch also converts the existing mAlreadyMeasuredOrphanTrees table in the
OrphanReporter to use the new mechanism.
--HG--
extra : rebase_source : 2c23285f8b6c3b667560a9d14014efc4633aed51
