It's both required by the spec and by our own internal invariants that an image's size should be available when we fire the image's load event. Unfortunately, inspecting the code reveals that VectorImage fires LOAD_COMPLETE even if mIsFullyLoaded is false, violating this invariant. I'm somewhat amazed this hasn't caused any problems until now. It's possible that it *has* been causing subtle problems - for example, intermittent oranges involving SVG-as-image - that we haven't tracked down to this root cause yet. A fix for this would look more or less like the code in bug 1163856 that delays the load event for RasterImage. In fact, bug 1163856 blocks this bug, because we need to stop requiring in tests that the image load event be delivered at the same time as the underlying Necko OnStopRequest callback, and we make that change in bug 1163856.

OK, it turns out that this can't depend on bug 1163856, because we need this fixed to land bug 1174923, which bug 1163856 depends on. That should be fine, though. I think we don't actually need to update any tests, because the tests I mentioned in comment 0 that check that the image load event is delivered at the same time as the underlying Necko OnStopRequest callback, IIRC, only check that property for raster images. If that turns out to be false, we can move the test modifications from bug 1163856 in the patch in this bug.

Here's the patch. This ensures that when VectorImage delivers LOAD_COMPLETE, it's fully loaded, and observers can call GetWidth() and GetHeight(). The approach is modeled very closely after the one used in RasterImage in bug 1163856 (which was already r+'d by Timothy).

Comment on attachment 8624010 [details] [diff] [review] Make VectorImage wait to deliver LOAD_COMPLETE until its size is available >+++ b/image/VectorImage.h >+ /// Progress resulting from the Necko load of the image. We actually notify >+ /// this in OnSVGDocumentLoaded or OnSVGDocumentError. >+ Maybe<Progress> mLoadProgress; Could you add a comment indicating that this is cleared to Nothing() once loading is complete? (That makes the usage clearer -- e.g. how we may skip setting this entirely in OnImageDataComplete.) Thanks for fixing this! r=me

Thanks for the quick review! I've updated the comment to explain the lifecycle of mLoadProgress in more detail, as requested.

Comment on attachment 8624397 [details] [diff] [review] Make VectorImage wait to deliver LOAD_COMPLETE until its size is available >+++ b/image/VectorImage.h >+ // Stored resulting from the Necko load of the image, which we save in >+ // OnImageDataComplete if the underlying SVG document isn't loaded. If we save >+ // this, we actually notify this progress (and clear this value) in >+ // OnSVGDocumentLoaded or OnSVGDocumentError. >+ Maybe<Progress> mLoadProgress; The first two words look like a typo -- perhaps you meant "Stored result"?

(In reply to Daniel Holbert [:dholbert] from comment #6) > The first two words look like a typo -- perhaps you meant "Stored result"? I did, thanks! I'll fix it before pushing.