We should cleanup the zero time timer call in OnStopRequest of ImageConsumer to just be a simple while loop till the stream has more data.

G: I'm not following you. More info/detail? -P

If there is more data in that method, it fires off a timer to wait 0 amount of time and then call a method. That method ends up calling OnStopRequest again. Why not just avoid the recursion and loop in a while statement in OnStopRequest() to fetch the additional data?

Gagan: Bruce: Try as I may, I can't get a test case that hits the breakpoint (~line 487 in gfx/src/nsImageNetContextAsync.cpp). I have replaced the code with what makes sense to me. Would you two give this a sanity check? And do you know how I can force the path through this code to test it? (ie, a test case) thnx, P

Does this patch lose the status/return code? When does mStream get released with the new code?

> Does this patch lose the status/return code? Yep. and you are right. It shouldn't. > When does mStream get released with the new code? On error? in the code I will be putting in a few minutes. ;) On ns_ok, I assume when ImageConsumer is destroyed. Gagan, could you comment? I'm not yet comfortable with this part of the code (the interface with necko). -P

Actually ... by calling this->OnStopRequest(), you're just calling the same method over (recursing). You should be able to just return the result of that and get on with life. If you free the mStream after calling that, it has already probably freed it itself just before returning the error. It'd be nice if the code could be restructured to avoid needing recursion at all, but I don't get what is going on well enough for that yet.

sorry about the re-post of the patch. Just a bugzilla hiccup. I'm unclear what should be happening here, too. Especially since I never see the code executed. KeepPumpingStream() did call OnStopRequest() which is why I made the recursive call. No changes until I get more enlightenment from Gagan and the necko crowd. -P

The only thing using mTimer on ImageConsumer was this code, so we can get rid of that member variable while we're here. Other than that, I guess this looks okay to me. I don't think we need the NS_IF_RELEASE(mStream); but it won't crash the browser or anything.

Moving to m18

All pnunn bugs reassigned to Pav, who is taking over the imglib.

code no longer used. marking fixed.

Verified