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
For multipart images we create a MultipartImage which contains each part. Each part in turn is a VectorImage or RasterImage. The MultipartImage and each part image all have their own ProgressTracker. The ProgressTracker for the MultipartImage observes the notifications of each part image via the IProgressObserver interface. This interfaces notably has no way to notify about an image error. So when a part image has an error it never gets propagated to the MultipartImage's ProgressTracker. This confuses our assertions about consistency of progress notifications. In this case we expect that when we get the load complete notification then we either have the size of the image or we encountered an error. So if the first part of a multipart image is broken and we are unable to get a size from it we will trigger this assertion.
There are two ways to fix this. One would be to propagate errors to the MultipartImage's ProgressTracker. This would put the ProgressTracker for the MultipartImage permanently into error state and prevent showing the images from the remaining parts if one part image had an error.
So in this patch I create a way to tell a ProgressTracker that is is for a multipart image, and use that to relax the assertions. As far as I can tell our code should be able to handle "ignoring" an error in a bad part image.
Addtionaly there is a way that an error flag can get propagated to the MultipartImage's tracker: in MultipartImage::FinishTransition we get the progress directly from the part image and notify for it. This seems like an oversight as the comment at
https://dxr.mozilla.org/mozilla-central/rev/bfa85d23df57c8a1db17c99b267667becc1c4afd/image/imgRequest.cpp#989
indicates that we don't want one bad part to prevent later parts from displaying. So we add the error flag to the ones we filter out when we propagate progress.
This function is an infallible alternative to nsIURI::GetSpec(). It's useful
when it's appropriate to handle a GetSpec() failure with a failure string, e.g.
for log/warning/error messages. It allows code like this:
nsAutoCString spec;
uri->GetSpec(spec);
printf("uri: %s", spec.get());
to be changed to this:
printf("uri: %s", uri->GetSpecOrDefault().get());
This introduces a slight behavioural change. Previously, if GetSpec() failed,
an empty string would be used here. Now, "[nsIURI::GetSpec failed]" will be
produced instead. In most cases this failure string will make for a clearer
log/warning/error message than the empty string.
* * *
Bug 1297961 (part 1b) - More GetSpecOrDefault() additions. r=hurley.
I will fold this into part 1 before landing.
--HG--
extra : rebase_source : ddc19a5624354ac098be019ca13cc24b99b80ddc
Slightly less than half (93 / 210) of the NS_METHOD instances in the codebase
are because of the use of NS_CALLBACK in
nsI{Input,Output,UnicharInput},Stream.idl. The use of __stdcall on Win32 isn't
important for these callbacks because they are only used as arguments to
[noscript] methods.
This patch converts them to vanilla |nsresult| functions. It increases the size
of xul.dll by about ~600 bytes, which is about 0.001%.
--HG--
extra : rebase_source : c15d85298e0975fd030cd8f8f8e54501f453959b
This patch makes most Run() declarations in subclasses of nsIRunnable have the
same form: |NS_IMETHOD Run() override|.
As a result of these changes, I had to add |override| to a couple of other
functions to satisfy clang's -Winconsistent-missing-override warning.
--HG--
extra : rebase_source : 815d0018b0b13329bb5698c410f500dddcc3ee12
This ID will be null for non-controlled documents and also for image cache
entries for which a document is not available, and it will be the numerical
value of the document pointer for controlled documents.
This effectively makes sure that a controlled document doesn't share its
image cache entries with anything else.
mTouchedTime is not appropriate for this as it is updated when an image
load re-uses the same imgRequest, especially as it has one second
granularity. A timestamp that is updated every time the backing file is
re-read should work better.
A millisecond granularity timestamp would be preferable, and would be
achievable on most or all supported platforms. But some older
filesystems have timestamp granularity of a second or worse, notably
ext3 and FAT32 (and even ext4 filesytems created with inode_size < 256
bytes, e.g. with 'mke2fs -t small' - see
https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout#Inode_Timestamps
for details.)
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
--HG--
rename : mfbt/nsRefPtr.h => mfbt/RefPtr.h
Andrew Osmond
Emilio Cobos Álvarez
Chris Peterson
Jonathan Kingston
Samael Wang
Noemi Erli
Kris Maglione
Nicholas Nethercote
Sebastian Hengst
Honza Bambas
Bill McCloskey
Shih-Chiang Chien
Eric Rahm
Wei-Cheng Pan
Tom Tromey
Sylvestre Ledru
Timothy Nikkel
Thomas Nguyen
Kyle Huey
sajitk
Nathan Froyd
Ehsan Akhgari
Seth Fowler
Phil Ringnalda
Birunthan Mohanathas
Oliver Henshaw