This requires a change to how we process test manifests in the build system:
now, whenever we see a support file mentioned in a manifest, we require that
file isn't already in that test's support files, but if we see a support file
that was already seen in some other test, the entry is ignored, but it is not
an error. As a result of this change, several duplicate support-files entries
needed to be removed.
MozReview-Commit-ID: G0juyxzcaB8
--HG--
rename : testing/mozbase/manifestparser/tests/test_default_skipif.py => testing/mozbase/manifestparser/tests/test_default_overrides.py
If the image load is from the same document that cached the image we are required to use the cached version. Otherwise we should be free to ignore the cached version.
If the sync decoding the LookupFrame does encounters an error it will set mError on the RasterImage, which LookupFrame callers check before calling LookupFrame. But they've called LookupFrame before the error was encountered, so we check if the frame has had Abort called on it to determine if we should return it at all.
We only does this if one of the sync decode flags was passed in because IsAborted needs to get the imgFrame's monitor, so we don't want to block consumers that haven't asked for decoding.
This means that in RasterImage::LookupFrame when we are asked to do a sync decode (if needed) we use WaitUntilComplete to wait until the frame is finished decoding. But we would actually return after the next progressive pass notified the monitor to wake up. Thus, we would draw a not-fully-decoded image even though the sync decode flag was passed.
The change in FrameAnimator means that we won't draw the next frame in an animated image until all progressive passes of that image are complete. This seems like what we want anyways.
There is one real use of IsImageComplete left, in imgFrame::Draw, where we need to know if the decoded image data covers the whole image frame. (There are a couple of uses of IsImageComplete in asserts.)
The PNG decoder posts the size almost immediately, and later posts transparency (even for non-animated images).
It would be nice to still assert what this assert is intending (that transparency of non-animated images is posted during the metadata decode) but we don't have any easy way of telling when a metadata finishes here.
This is left over from the pre-SurfacePipe code that interacted directly with the Downscaler. It was calculating the size of the surface for the Downscaler to use, and then the Downscaler would handle putting transparent pixels inside that surface (and outside the framerect).
--HG--
extra : rebase_source : aad384fa8589f291254f0a18537a5d6674487182
The testcase has an svg-as-image inside an svg-as-image. At shutdown the viewer of the inner svg-as-image is destroyed (via the shutdown observer) first. Then the outer svg-as-image destroys its viewer which tries to unregister all image requests from the refresh driver. So it unregisters the inner svg-as-image, which calls GetAnimated.
When storing ms, 32 bit ints can hold 2^32/1000/60/60/24 ~= 49 days. It's quite conceivable that someone would leave a tab in the background for 50 days.
GetSingleLoopTime returns -1 on exceptional cases but we used an unsigned int to hold the return value in AdvanceFrame. So the |loopTime > 0| check would succeed. Fortunately the |delay.ToMilliseconds() > loopTime| check would fail because loopTime was MAX_UNIT32, so we didn't do anything incorrect.
http://hg.mozilla.org/mozilla-central/rev/263980931d1b (bug 890743) changed GetSingleLoopTime from returning 0 (and uint32_t) to -1 (and int32_t) on exceptional cases. But the caller of GetSingleLoopTime wasn't updated.
Before the previous patch we would (wrongly) loop through the decoded frames even though we didn't have all of the frames of the animation. This had the beneficial side effect of advancing mCurrentAnimationFrameTime to aTime (the current time). With the previous patch we stop at the last decoded frame and don't advance mCurrentAnimationFrameTime, so it can lag behind. The problem with this is that when we have finished decoding we will then try to catch mCurrentAnimationFrameTime up, and this will jump us to a random point in the animation. So we need to advance mCurrentAnimationFrameTime ourselves.
If we were blocked on network/decoding then displaying the last available decoded frame is the correct frame to be displaying. So we are up to date. So we advance mCurrentAnimationFrameTime to the current time.
mImage->GetNumFrames() is the current number of decoded frames (that the RasterImage knows about), so it only represents the last frame of the animation if we are done decoding.
If we are not fully decoded, and we are on the last decoded frame, just stay on the last decoded frame. When more frames get decoded (or we determine that we are the last frame of the animation) we will advance.
One might expect that if |nextFrameIndex == mImage->GetNumFrames()| then |GetRawFrame(nextFrameIndex)| would return a null surface. But that is not the case because the decoding thread can insert frames into the surface cache that the RasterImage hasn't acknowledged yet (because it has to do so on the main thread, which we are currently running on).
This is why moving animated images to the surface cache is likely the cause of this bug.
This introduces an issue that is explained in, and fixed by the next patch.
http://hg.mozilla.org/mozilla-central/rev/411f18fdffeb (bug 1186796) had a mistake in it.
It changed ImageSurfaceCache::LookupBestMatch to use a for loop instead of using a callback to iterate each entry of the hashtable. The callback was called with the surface key of its entry, and it used the name |aSurfaceKey| for that key. ImageSurfaceCache::LookupBestMatch uses the name |aSurfaceKey| for the key we are looking for. So when the code from the callback was moved into the for loop in ImageSurfaceCache::LookupBestMatch the meaning of |aSurfaceKey| changed, but the code was not updated.
In (non-animated) PNGs the image data is contained in IDAT chunks. In APNGs there are IDAT chunks, which contain the default image, and fDAT chunks, which contain frames of the animation. The default image is sometimes part of the animation (as the first frame), and sometimes not (displayed only by non-APNG aware viewers).
The default image must have the same size as in the PNG header chunk. But the fDAT images can be any (smaller) size. So the first frame of a PNG is allowed to be smaller than the whole image size so long as we are in an APNG and the first frame is from an fDAT chunk, not an IDAT chunk.
We post transparency if we encounter this case because we don't draw into those pixels on at least the first frame.
Be warned. Do not attemp to change the .js "test" source code in ./js
They are meant to check
- the outdated 0666 octal constant is still parsed correctly,
- the outdated 0666 octal constant raises syntax error flag
in strict mode, etc.
So leave them alone.
Also add crashtest with a corrupt animated PNG image that causes a leak to be reported (thanks to these MOZ_COUNT calls). This leak is fixed by the patch on separate bug 1237709.
This patch introduces TerminalState and changes LexerTransition::mNextState to
be a Variant<State, TerminalState>. This means that SUCCESS and FAILURE no
longer need to be part of State.
Some things to note:
- This simplifies the handling of Lex()'s return value, which is nice.
- The patch splits Terminate() into TerminateSuccess() and TerminateFailure().
- |const State& aNextState| wouldn't work for the first arg to
LexerTransition's ctor due to errors in Variant construction that I didn't
understand. I had to change it to |State aNextState|.
--HG--
extra : rebase_source : f405a67fdf0f1bb712409eafecb21ac9b59d6db0
This fixes failures for
image/test/reftest/bmp/bmpsuite/b/{badrle.bmp,shortfile.bmp} with the Skia
back-end.
--HG--
extra : rebase_source : 6c5b967cebf43cf5d49d0e532619bdd1c8ccc69e
gfxRect can be implicitly constructed from IntRect, which hides a number of
implicit conversion points, makes Moz2Dification harder, and has some
surprising effects.
This patch removes the implicit constructor and replaces it with an explicit
conversion function:
gfxRect ThebesRect(const IntRect&)
This is the obvious outcome of removing the constructor.
But there is also a second, less obvious outcome: currently we do a number of
IntRect-to-Rect conversions using ToRect(), which (surprisingly) works because
it turns into an implicit IntRect-to-gfxRect conversion (via the implicit
constructor) combined with an explicit gfxRect-to-Rect conversion (via
ToRect()). I.e. we do two conversions, going from a Moz2D type to a Thebes
type and back to a Moz2D type!
So this patch also changes these conversion. It moves this existing function:
Rect ToRect(const IntRect&)
from gfx2DGlue.h -- where it doesn't really belong because it doesn't involve
any Thebes types -- to gfx/2d/Rect.h, templatifying and renaming it as
IntRectToRect() in the process.
The rest of the patch deals with fall-out from these changes. The call sites
change as follows:
- IntRect-to-gfxRect conversions:
- old: implicit
- new: ThebesRect()
- IntRect-to-Rect conversions:
- old: ToRect()
- new: IntRectToRect()
--HG--
extra : rebase_source : e4e4c2ad10b36ecad4d57d1630158f3374e403be
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.