Closed Bug 1196065 Opened 4 years ago Closed 4 years ago
Add sanity tests for image decoders
In bug 1193119, and probably more bugs following that one, there is some major refactoring happening in the GIF decoder code. I think it's important that we have some sanity tests for the image decoders, to help verify that this refactoring is correct. We can gradually add to these tests, but for now I'm mostly interested in verifying that the decoders generally work correctly in these two cases: 1. When the data is delivered as a single chunk. 2. When the data is delivered as a series of single-byte chunks. These two scenarios test that the 'hold' mechanism (which the decoders use to deal with the situation where not all the data is available) works in the most extreme scenarios. Hopefully this ensures that it works in general.
Here's the patch. This is almost entirely test-only changes, but there are two minor non-test changes I made to facilitate these tests: 1. I got rid of a warning in SourceBuffer.cpp that these tests trigger. I'd rather not spam that warning thousands of times in everyone's logs. 2. I made it possible to pass an alternate IResumable to Decoder::Decode(), to make the multichunk tests possible. (Since there's no DecodePool running during those tests, the default implementation of IResumable that Decoder uses wouldn't work.) Note that I just noticed that there's some commented out code in the test file. I'll fix that before landing this, so please ignore that during your review.
In this version I removed the commented out code and added more verbose comments.
Attachment #8649659 - Flags: review?(tnikkel)
Comment on attachment 8649659 [details] [diff] [review] Add sanity tests for image decoders FYI, you can also use .sjs files and mochitest to introduce arbitrary delays when sending images.
Attachment #8649659 - Flags: review?(tnikkel) → review+
(In reply to Timothy Nikkel (:tn) from comment #4) > Comment on attachment 8649659 [details] [diff] [review] > Add sanity tests for image decoders > > FYI, you can also use .sjs files and mochitest to introduce arbitrary delays > when sending images. Thanks for the review! You're right; we could've introduced something like this much earlier.
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/066ad55e75a9 because of static build failure: /builds/slave/m-in-m64-st-an-d-0000000000000/build/src/image/test/gtest/TestDecoders.cpp:120:3: error: 'AddRef' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] /builds/slave/m-in-m64-st-an-d-0000000000000/build/src/image/test/gtest/TestDecoders.cpp:120:3: error: 'Release' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] make: *** [Unified_cpp_image_test_gtest0.o] Error 1 make: *** [image/test/gtest/target] Error 2 make: *** Waiting for unfinished jobs.... make: *** [compile] Error 2 make: *** [default] Error 2 make: *** [realbuild] Error 2 make: *** [build] Error 2 Return code: 2 'mach build' did not run successfully. Please check log for errors. Running post_fatal callback... Exiting -1
It looks like the NS_INLINE_DECL_REFCOUNTING for NoResume needed to become NS_INLINE_DECL_REFCOUNTING(NoResume, override).
Attachment #8649659 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.