Attached patch brings the logic in line with nsHttpChannel::AsyncOpen, and removes obsolete TODOs (dwitte: we don't need nsHttpChannel's cookie logic here, right?). Also adds channel to loadgroup and calls OnModifyRequest(). I believe it fixes what would otherwise be a leaked AddRef if we wind up being canceled. Does not appear to make the testXUL demo any wobblier :)

Comment on attachment 438314 [details] [diff] [review] Rework HttpChannelChild::AsyncOpen >+ // notify "http-on-modify-request" observers >+ gHttpHandler->OnModifyRequest(this); This is the part we need the cookie bits for (and still do)... need to add the Cookie header before we send out OMR, so observers can... modify it. ;) But you're not making anything worse, and it can wait 'til we have a story for how child/parent-side OMR observers are supposed to interoperate. Might want to leave the TODO in there, though, albeit with a better explanation. >+ // FIXME: bug 536317: We may have been cancelled already, either by >+ // on-modify-request listeners or by load group observers; in that case, >+ // don't create IPDL connection. See nsHttpChannel::AsyncOpen(): I think >+ // we'll need something like >+ // >+ // if (mCanceled) { >+ // LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled)); >+ // AsyncAbort(rv); >+ // return NS_OK; >+ // } Hmm. But if we haven't created any IPDL connection yet, what do we need to abort? (I assume your AsyncAbort() here is an IPDL call, but maybe not?) > mState = HCC_OPENED; Does this want to move up to the 'mIsOpened' et al setter expression, or stay here? r=dwitte

Also, the mCaps TODO still applies, right?

http://hg.mozilla.org/projects/electrolysis/rev/2f30bbaa0912 > I assume your AsyncAbort() here is an IPDL call AsyncAbort is not an IPDL function--it's boilerplate nsHttpChannel logic for calling OnStart/Stop when a channel is cancelled. > > mState = HCC_OPENED; > Does this want to move up to the 'mIsOpened' et al setter expression, or stay here? I want to keep the mState = HCC_OPENED close to the SendAsyncOpen. Eventually it will be replaced by IPDL's forthcoming state transition variable. > need to add the Cookie header before we send out OMR Added a bug (and comment) for the cookies logic. > Also, the mCaps TODO still applies, right? From my read of the code, nsHttpChannel should be getting an identical mCaps from the chrome process's nsHttpHandler::NewProxiedChannel, so we shouldn't need to do anything with the childs. I left it in just so it can share the same Init() code.