Comment 149 for bug 25830

(In reply to comment #122)
> fine with me to make aRequest a precondition. aData is allowed to be null though
> (see the interface docs).

Why, though? What's the use case for a null aData?

> > Could we please put this in a different file? Especially if we expect to allow
> > embeddors to implement nsIDispatchable?
> I dunno, do we? ;) sure, fine with me. in uriloader/base or exthandler? I guess
> base is better (I envision possibly passing this to nsIURIContentListener
> objects as well, sometime in the future)

Ah, to nsIURIContentListener2? That would make sense....

If we do plan to just keep this an exthandler-specific hack, it may be better to just expose
that interface on aContext (basically have the object implementing the nsIDispatchable
interface be the context, and have it forward to the original context as needed). Then the
interface could be in a .h file altogether or something, with all documentation in that file.

> that's the only object currently which gets an nsIDispatchable? the code would
> have to be expanded if we ever pass it to other handlers, sure. I am not sure
> that I want to give up this precondition, it seems important to let the current
> handler rely on this.

It's a good precondition, but again only one that makes sense if we plan to have multiple
handlers that get an nsIDispatchable. If we do, then a lot of these comments make more
sense...

> > Maybe it should? Would that actually break anything in this case?
> I'd have to check. it probably should get onStopRequest... maybe with a special
> status code? (NS_BINDING_RETARGETED?) I suspect onstopr currently doesn't deal
> with that very well.

Special status code and dealing sounds good to me.

> I don't see how that follows... if doContent does call it, we re-enter uriloader
> while it's not done with DoContent yet. that seems scary.

Oh, I agree that it's bad.....

> I can rephrase it as "Must be called in or after onStartRequest", if you prefer that.

I think that's reasonable. Put an assert in the URILoader code to enforce this, maybe
(with a debug-only boolean member set across the DoContent call or something)?

> hmm, guess we never store the uricontentlistener we chose? if we do,
> DispatchContent could just compare the new one to the current one and fail if
> they are equal (after CanHandleContent/IsPreferred returned true)

It's almost worse, actually. We want to prevent oscillatory bouncing between two
handlers that both want to fob the load off as the type the other one handles... So
extending this to allowing all handlers to redispatch would have to be done pretty
carefully.