1505185 - Privileged content process should be restricted to only loading pages that have URI_CAN_LOAD_IN_PRIVILEGED_CHILD

Status: RESOLVED WONTFIX

(Core :: DOM: Content Processes, enhancement)

Description

[Mike Conley (:mconley) (:⚙️)]

See summary. There might be some more comprehensive(?) things that we could do with the triggering principal here as well.

Comment 1

[Eric Rahm [:erahm]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #0)
> See summary. There might be some more comprehensive(?) things that we could
> do with the triggering principal here as well.

Is this something we're going to work on soon, or should we pursue a workaround with a pref so that the about:blank pages are loaded in separate processes for the awsy(sb) tests?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Eric Rahm [:erahm] from comment #1)
> Is this something we're going to work on soon, or should we pursue a
> workaround with a pref so that the about:blank pages are loaded in separate
> processes for the awsy(sb) tests?

I'm going to work on this next week, if that's alright - or is there a rush on this?

Comment 3

[Kris Maglione [:kmag]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)
> (In reply to Eric Rahm [:erahm] from comment #1)
> > Is this something we're going to work on soon, or should we pursue a
> > workaround with a pref so that the about:blank pages are loaded in separate
> > processes for the awsy(sb) tests?
> 
> I'm going to work on this next week, if that's alright - or is there a rush
> on this?

It makes our AWSY results kind of unreliable... but next week is probably OK.

Comment 4

[Eric Rahm [:erahm]]

(In reply to Kris Maglione [:kmag] from comment #3)
> It makes our AWSY results kind of unreliable... but next week is probably OK.

Agreed, sooner is better than later. If it's going to slip further please let us know.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

I'll try to take this today.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1505185 - Only let pages with nsIAboutModule.URI_CAN_LOAD_IN_PRIVILEGED_CHILD load in the privileged content process. r?Gijs

Before, we were allowing pages with nsIAboutModule.URI_CAN_LOAD_IN_CHILD, like
about:blank, load in the privileged content process.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Ugh, a bunch of failing tests. :/ 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=21931dfb7574b7269e4b10fdd02f09299361dd71

erahm, if you disable the preloaded about:newtab for your AWSY tests by setting browser.newtab.preload to false, does that cause the noise in AWSY to go away? That might be a faster path to success while I figure out what's going wrong here.

Comment 8

[Eric Rahm [:erahm]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #7)
> Ugh, a bunch of failing tests. :/ 
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=21931dfb7574b7269e4b10fdd02f09299361dd71
> 
> erahm, if you disable the preloaded about:newtab for your AWSY tests by
> setting browser.newtab.preload to false, does that cause the noise in AWSY
> to go away? That might be a faster path to success while I figure out what's
> going wrong here.

That seems to have made things slightly worse on windows [1], we we still end up with 3 about:blanks in the privileged process.

[1] https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=52af79ef901dbf46ab5e7fcbac7f3c203d0055c8&framework=4&filter=win&showOnlyComparable=1&selectedTimeRange=172800

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Okay, coming back to this then.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

So this original patch isn't going to work out because of how new tabs will often try to browse to about:blank early in their lifecycle before loading the actual requested content. (Gijs is well aware of this class of problem, I believe, due to his work in bug 1493606).

So with this patch, we can get into a case where a new about:newtab page first loads about:blank, so it process flips to the unprivileged process type, and then it tries to load about:newtab, so it process flips again to the privileged process type, and then it tries to go to about:blank again, ad infinitum. :/

I'll look at alternative solutions.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

So the solution I'm looking at right now is to special-case about:blank from within the privileged content process. I'm hoping I can convince it to load in the default web process if about:blank is navigated to via the parent process. I'm hoping about:blank can be loaded in the privileged content process only when the navigation starts from within the content process.

(We need the privileged content process to be able to load about:blank's, since the content process will often create an about:blank content viewer early in its lifetime).

Comment 12

[Mike Conley (:mconley) (:⚙️)]

I have this mostly working, but I think I broke the file URI process affinity stuff (the code that keeps the first domain browsed to from a local file within the local file process, but any other domains cause a process flip). So digging into that now.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

I'm bailing out of this in favour of bug 1515201 instead.

I think it's still true that we need to ensure that only the right pages are loaded in the privileged content process, but I don't think we can practically remove about:blank from that list. Not yet, anyhow.

I'm going to WONTFIX this for now, I guess.