There is a subtle race condition with the notification code when an image is first loaded that can lead to OnStartDecode being called twice in a row without getting OnStop* in between. This is causing an abort in the onload blocking code in nsImageLoadingContent.cpp: http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsImageLoadingContent.cpp#197 This doesn't normally show up, but we do enough decode requests in bug 512260 that it starts to become in issue there. The fix for this bug depends on joe's work in bug 572520, so we need to wait for that. Here's what happens: A consumer wants an image. To this end, it calls LoadImage: http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#1491 We get a cache hit, and thus validate the entry: http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#1555 We decide that, norly, we really want to validate, and call ValidateRequestWithNewChannel: http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#1356 Next, we create a proxy, which (ahah!) adds the proxy to the imgRequest without sending any notifications, since we send those later during cache entry validation. http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#1200 http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgRequestProxy.cpp#131 We then proceed to instantiate an asynchronous imgCacheValidator, and return. http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#1213 Next, before the cache validator gets itself going, we have some activity on the underlying image (say, someone calls RequestDecode() in bug 512260), which causes it to start decoding and send OnStartDecode(), but nothing farther than that. The imgCacheValidator gets itself going, and calls NotifyProxyListener() http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/imgLoader.cpp#2021 BAM! 2 OnStartDecodes in a row! Joe thinks that the solution here is to use his "defer" machinery in bug 572520, but that's not ready yet. So we wait.

This blocks bug 512260, which blocks a blocker (bug 563088). Nominating.

Who owns this? Need an owner ASAP.

That's me! Forgot to take the bug - sorry about that.

Added a patch to call defer/undefer in the right places. I spoke with Joe a bit about this on gchat, and he was ok with it. Flagging for review.

Comment on attachment 462196 [details] [diff] [review] Initial stab at fixing the race condition Oh hey, that actually worked! Sorry for the noise. (In case anyone's interested, I just figured out how to push from git to bugzilla).

pushed to try as 0b9b622cde81

Comment on attachment 462155 [details] [diff] [review] patch v1 I'd prefer the comments say something along the lines of "We will send notifications from imgRequestValidator::OnStartRequest; in the mean time, we must defer notifications because we are added to the imgRequest's proxy list, and we can get extra notifications resulting from methods such as RequestDecode()." Just a little bit more explicit. Also, is it possible for you to write a test along the lines of the ones I did for bug 572520, to make sure that we don't get two OnStartRequests in a row? I presume we can't just add NS_ABORT_IF_FALSEs to imgStatusTracker because of other latent issues along this line.

Updated patch, addressing joe's comment concerns. I _think_ this looks good on try, but it's hard to tell because everything's so orange. I'll push it when things clear up.

This blocks beta 5, the feature freeze.

pushed: http://hg.mozilla.org/mozilla-central/rev/58b623ff605c Resolving fixed.

There's some tree weirdness going on right now (tinderbox is being crashy). I'm going to leave this open to make sure we verify that it stuck.

This seems to have stuck. Re-resolving fixed.