1024898 - Allow most nsBaseChannel subclasses to be retargeted to a different thread

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

A few are driven by IPC so it doesn't make sense, but most just work.

Comment 1

[Steve Workman [:sworkman] (INACTIVE)]

Comment on attachment 8439749 [details] [diff] [review]
Patch

Review of attachment 8439749 [details] [diff] [review]:
-----------------------------------------------------------------

Cool. Are you thinking of using it for something in particular?

I think r=me, but I'd like Jason to weigh in too.

Comment 2

[Jason Duell]

Comment on attachment 8439749 [details] [diff] [review]
Patch

Review of attachment 8439749 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/src/nsBaseChannel.cpp
@@ +815,5 @@
>    return NS_OK;
>  }
> +
> +NS_IMETHODIMP
> +nsBaseChannel::RetargetDeliveryTo(nsIEventTarget* aEventTarget)

It'd be nice to have some enforcement that this is only called during OnStartRequest.  But I can live w/o it.  We're not doing such checks in HTTP either, FWIW.

::: netwerk/protocol/ftp/FTPChannelChild.cpp
@@ +43,5 @@
>    NS_ADDREF(gFtpHandler);
>    SetURI(uri);
>    mEventQ = new ChannelEventQueue(static_cast<nsIFTPChannel*>(this));
> +
> +  DisallowThreadRetargeting();

Could be worth a comment

   // We could support retargeting but as long as we're getting IPDL on 
   // the main thread it's not a win.

::: netwerk/protocol/rtsp/RtspChannelParent.cpp
@@ +17,5 @@
>  RtspChannelParent::RtspChannelParent(nsIURI *aUri)
>    : mIPCClosed(false)
>  {
>    nsBaseChannel::SetURI(aUri);
> +  DisallowThreadRetargeting();

Any reason RtspParent needs to call Disallow on the "real" necko channel, but FtpParent doesn't?   I would assume that the Parent classes are never going to try to retarget, so I'm not sure why we need to have disallow call.  Not that it does any harm...

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Steve Workman [:sworkman] from comment #1)
> Cool. Are you thinking of using it for something in particular?

data URIs in the HTML parser were the main goal (since those trip the warning I added in bug 1024388).

(In reply to Jason Duell (:jduell) from comment #2)
> Any reason RtspParent needs to call Disallow on the "real" necko channel,
> but FtpParent doesn't?   I would assume that the Parent classes are never
> going to try to retarget, so I'm not sure why we need to have disallow call.
> Not that it does any harm...

No, not really.  The reason I did it in RtspParent is because it doesn't run any of the channel machinery at all.

https://hg.mozilla.org/integration/mozilla-inbound/rev/16ff50091d32

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/16ff50091d32