Last Comment Bug 734681 - Shutdown the decoder in SourceDataComplete() if decoding is already finished
: Shutdown the decoder in SourceDataComplete() if decoding is already finished
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla14
Assigned To: Robert Lickenbrock [:rclick]
: Milan Sreckovic [:milan]
Depends on:
Blocks: 721917
  Show dependency treegraph
Reported: 2012-03-10 12:36 PST by Robert Lickenbrock [:rclick]
Modified: 2012-03-27 05:10 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (2.82 KB, patch)
2012-03-10 13:41 PST, Robert Lickenbrock [:rclick]
joe: review+
Details | Diff | Splinter Review
patch for check-in [r=joe] (2.89 KB, patch)
2012-03-26 05:19 PDT, Robert Lickenbrock [:rclick]
no flags Details | Diff | Splinter Review

Description User image Robert Lickenbrock [:rclick] 2012-03-10 12:36:53 PST
For the BMP decoder we don't shutdown the decoder until we've decoded all the data, and we don't know that until SourceDataComplete() is called. If the decode worker decodes all the data before SourceDataComplete(), we won't do an UNTIL_SIZE decode and the decoder is shutdown asynchronously.

This is a problem because the RasterImage doesn't notice some errors until the decoder is shutdown, which means that we'll sometimes fire onload when we otherwise would have fired onerror. I believe that this discrepancy is the cause of the reftest failures in bug 721917.

Patch coming up shortly.
Comment 1 User image Robert Lickenbrock [:rclick] 2012-03-10 13:41:06 PST
Created attachment 604684 [details] [diff] [review]

This changes UNTIL_SIZE decodes so that instead of returning early if the size is already known, we only skip the decoding loop. That way we'll check if decoding is finished and shutdown the decoder.
Comment 2 User image Joe Drew (not getting mail) 2012-03-10 20:11:06 PST
Don't forget to ask for review :)
Comment 3 User image Robert Lickenbrock [:rclick] 2012-03-14 01:59:52 PDT
Comment on attachment 604684 [details] [diff] [review]

Right, I forgot the review flag. I'm out of town for the week, so no need to rush on the review. I'll address any review comments and do some try runs when I return.
Comment 4 User image Robert Lickenbrock [:rclick] 2012-03-22 15:37:56 PDT
Pushed to try:
Comment 5 User image Mozilla RelEng Bot 2012-03-23 02:03:55 PDT
Try run for f1667d464be7 is complete.
Detailed breakdown of the results available here:
Results (out of 224 total builds):
    exception: 3
    success: 188
    warnings: 17
    failure: 16
Builds (or logs if builds failed) available at:
Comment 6 User image Robert Lickenbrock [:rclick] 2012-03-26 05:19:57 PDT
Created attachment 609289 [details] [diff] [review]
patch for check-in [r=joe]
Comment 7 User image Justin Lebar (not reading bugmail) 2012-03-26 09:06:44 PDT
Comment 8 User image Marco Bonardo [::mak] 2012-03-27 05:10:37 PDT

Note You need to log in before you can comment on or make changes to this bug.