1160423 - Deliver the results of a size decode without using a runnable

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

It happens quite frequently that we uselessly decode the headers of an image twice. This happens because we will finish a size decode for the image off-main-thread, but before we can notify the main thread that that happened using a runnable, Necko delivers OnStopRequest to the main thread. Since we are required to send the load event from the OnStopRequest callback, and we cannot send the load event without knowing the image's size, we end up doing a second synchronous size decode in the OnImageDataComplete.

Especially after bug 1160422, an image's size decode should virtually always have finished before OnStopRequest gets delivered. We should just deliver the size decode results by updating RasterImage's members directly, under a mutex. We can use wait / notify to ensure that we always get the values, even if the main thread gets ahead.

Comment 1

[Seth Fowler [:seth] [:s2h]]

(It's possible that we should bound the amount of time we wait, and then revert to a synchronous size decode. But I think we should try the simple thing first.)

Comment 2

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 1) - Add locking for RasterImage::SetSize()

This part adds the necessary locking for RasterImage::SetSize(). Since
RasterImage::mSize and RasterImage::mOrientation are now protected by a lock, we
need to take the lock everywhere we read or write them.

That's unfortunately a large number of places. I'm not wild about that; it may
be necessary, but I'm going to experiment with an alternative approach before
requesting review on this.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 2) - Deliver the results of a size decode without using a runnable

Here's part 2, which makes us actually deliver the size to RasterImage from
off-main-thread. These changes enable some additional refactoring, but I'll save
that for a followup.

(And I want to explore that alternative approach I mentioned above first.)

Comment 4

[Seth Fowler [:seth] [:s2h]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97347486bc9c

Comment 5

[Seth Fowler [:seth] [:s2h]]

I think we're going to end up going with bug 1163856, so I'm resolving this for now. If we change our minds about bug 1163856, we can always reopen this one.