Audit the consumers of Necko to look for Necko APIs used in the child process that aren't available there

NEW
Assigned to

Status

()

P4
normal
3 years ago
3 years ago

People

(Reporter: Ehsan, Assigned: jduell)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(e10s+)

Details

(Reporter)

Description

3 years ago
We need to create a list of Necko APIs that don't work in the child process and audit the code for the usages of those APIs in the child process that may be failing silently without anyone knowing.

Bug 1217571 is one example of what can go wrong here with nsICachingChannel.
Jason, do you want to do this audit?
Flags: needinfo?(jduell.mcbugs)
(Assignee)

Comment 2

3 years ago
So our previous approach here has been to call NECKO_MAYBE_ABORT() when an API we don't support on the child is called.  But that's only for IDLs that we partially support (some functions are OK to call, some aren't--those we abort on in debug builds).

We don't currently have anything like this for entire IDLs that are missing in the child.  Perhaps we should hack QI and abort if the child is asked for nsICachingChannel (or any other IDL that parent channels implement but child ones don't).  Ehsan, what do you think of that idea?
Flags: needinfo?(jduell.mcbugs) → needinfo?(ehsan)
(Reporter)

Comment 3

3 years ago
Aborting in QI if we ask for a non-existing interface sounds like a fantastic idea!

But note that you won't be able to land that patch because of bug 1217571.  But it'd be nice to hack around that and also deal with other non-existing interfaces and see what this finds.
Flags: needinfo?(ehsan)

Updated

3 years ago
Assignee: nobody → jduell.mcbugs
tracking-e10s: ? → m8+
Jason, I think I have patches that pass try for nsICachingChannel; are there other examples of APIs that should be shut out in the child process?
Flags: needinfo?(jduell.mcbugs)
(Assignee)

Comment 5

3 years ago
Honza, can you think of other IDLs we'd want to check here? I'm guessing that across platform there would be quite a few.  NSS-related stuff, socket transport, etc?  (though the socket transport thread continues to linger on the child right now for httpd.js support).
Flags: needinfo?(jduell.mcbugs) → needinfo?(honzab.moz)
Hmm.. as I understand, you want to simply block ANY class that QI to nsICachingChannel (and any other we may list here) to throw?

I think of following ifaces to black-list as well: 

nsINetworkLinkService, nsIPackagedApp* (probably just *Service, but not sure), nsICacheEntry*, nsICacheStorage*, nsIHttpAuth*, nsISSLSocketControl, nsISOCKSSocketInfo.

According NSS, I lost track some time ago.  :keeler is the one to ask these days.  In my times (looong ago) there were few missing interfaces implementations that I have fixed.
Flags: needinfo?(honzab.moz)
(Assignee)

Comment 7

3 years ago
We also don't support nsICookieManager (we do support nsICookieService).
(In reply to Jason Duell [:jduell] (needinfo? me) from comment #7)
> We also don't support nsICookieManager (we do support nsICookieService).

I think there is IPC already under the hood.  I've seen this, but it seems (at the first sight) non-problematic to me.
This was m8 so we'd take a look at it, which I think we have at this point. renom'ing
tracking-e10s: m8+ → ?
tracking-e10s: ? → +

Updated

3 years ago
Priority: -- → P4
You need to log in before you can comment on or make changes to this bug.