Closed Bug 1377544 Opened 3 years ago Closed 3 years ago

Label runnables in ExtensionProtocolHandler

Categories

(Core :: Networking, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: kershaw, Assigned: kershaw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-next])

Attachments

(1 file, 1 obsolete file)

Priority: -- → P2
Attached patch Label ExtensionProtocolHandler (obsolete) — Splinter Review
Summary:
 - Remove AbstractThread according to bug 1372736
 - Let nsContentUtils::GetEventTargetByLoadInfo return nsISerialEventTarget
 - Label runnables in ExtensionStreamGetter

Thanks.
Assignee: nobody → kechang
Status: NEW → ASSIGNED
Attachment #8887401 - Flags: review?(honzab.moz)
Attachment #8887401 - Flags: review?(honzab.moz) → review+
Rebase and carry r+.
Attachment #8889749 - Flags: review+
Attachment #8887401 - Attachment is obsolete: true
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c61346d6f44
Labeling in ExtensionProtocolHandler, r=mayhemer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1c61346d6f44
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
I was under the impression that labeled runnables were only to be used in content. The nsIRunnables used in ExtensionProtocolHandler are only for the parent process and we shouldn't ever need them in content because the runnables are for opening files and content sandboxing prevents content processes from doing that.

For bug 1376496, I'm removing the nsILoadInfo that is sent from child to parent in the ExtensionProtocolHandler remoting. Could you recommend how to implement ExtensionStreamGetter::SetupEventTarget() without a loadinfo?
Flags: needinfo?(kechang)
See Also: → 1376496
(In reply to Haik Aftandilian [:haik] from comment #6)
> I was under the impression that labeled runnables were only to be used in
> content. The nsIRunnables used in ExtensionProtocolHandler are only for the
> parent process and we shouldn't ever need them in content because the
> runnables are for opening files and content sandboxing prevents content
> processes from doing that.
> 

Yes, you are right. Labeled runnables are only used in content process.

But the abstract main thread used in ExtensionStreamGetter::GetAsync() is in content process. We need to also label it.
Also, when using nsIInputStreamPump in content process, we also have to assign a labeled event target to it.

> For bug 1376496, I'm removing the nsILoadInfo that is sent from child to
> parent in the ExtensionProtocolHandler remoting. Could you recommend how to
> implement ExtensionStreamGetter::SetupEventTarget() without a loadinfo?

It seems that the loadinfo is passed in ExtensionProtocolHandler::SubstituteRemoteFileChannel().
I think we can use nsContentUtils::GetEventTargetByLoadInfo() to get a labeled event target and try to pass it to ExtensionStreamGetter.
Flags: needinfo?(kechang)
You need to log in before you can comment on or make changes to this bug.