Switch to right process type for initial tab based on initial URL

VERIFIED FIXED in Firefox 66
(NeedInfo from)

Status

()

enhancement
P1
normal
VERIFIED FIXED
5 months ago
a month ago

People

(Reporter: Gijs, Assigned: Gijs, NeedInfo)

Tracking

(Blocks 1 bug)

Trunk
Firefox 65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 disabled, firefox66 verified)

Details

(Whiteboard: [fxperf:p1])

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
bug 1506608 defaults the initial browser to the default remote process.

In the common case though, we'll load about:newtab in the privileged content process, and we'll still need to switch remoteness for that. To fix this, we should check the initial URL (if available) and use that (otherwise default to the default remote process).

This should be relatively straightforward.
(Assignee)

Updated

5 months ago
Priority: -- → P1
(In reply to :Gijs (he/him) from comment #0)
> bug 1506608 defaults the initial browser to the default remote process.
> 
> In the common case though, we'll load about:newtab in the privileged content
> process, and we'll still need to switch remoteness for that. To fix this, we
> should check the initial URL (if available) and use that (otherwise default
> to the default remote process).

On the 2018 ref hardware, it takes a very long time after painting the browser window before we start painting about:home. Profiling shows that it's not Activity Stream code that takes this time, but just that the 'Privileged Content' isn't started yet. It takes more than 9s between when we start _delayedStartup and when the 'Privileged Content' starts to load activity stream JS code.

https://perfht.ml/2Ridff5

I'm hoping the patch here will help with this :-).
(Assignee)

Comment 4

5 months ago
(In reply to Florian Quèze [:florian] from comment #3)
> I'm hoping the patch here will help with this :-).

Any chance you can test if it does? My reference machine is set up for something else atm (and as we both know, doing anything... material... on the machine takes a long time), so I can't really do it myself...

(I couldn't see any material difference in sessionstore tests on my high-end mbp, but I imagine that's because we spend too much time on the *other* tabs)
Flags: needinfo?(florian)

Comment 5

5 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d05c6310d573
use the window's initial url to determine what remoteType the initial browser should have, r=dao,mconley
(Assignee)

Updated

5 months ago
Blocks: 1506608
Summary: Switch to right process based on initial URL → Switch to right process type for initial tab based on initial URL
(Assignee)

Updated

5 months ago
Whiteboard: [fxperf:p1]

Comment 6

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d05c6310d573
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
(In reply to :Gijs (he/him) from comment #4)
> (In reply to Florian Quèze [:florian] from comment #3)
> > I'm hoping the patch here will help with this :-).
> 
> Any chance you can test if it does? My reference machine is set up for
> something else atm (and as we both know, doing anything... material... on
> the machine takes a long time), so I can't really do it myself...
> 
> (I couldn't see any material difference in sessionstore tests on my high-end
> mbp, but I imagine that's because we spend too much time on the *other* tabs)

Here is a new profile captured using today's nightly: http://bit.ly/2FP70On

The Privileged Content process is now started before the WebExtensions process, and we don't have a normal Content Process. It still takes a looong time before we start the Privileged Content process, but I don't think it's a regression, so I think this confirms this bug is fixed.
Flags: needinfo?(florian)
(Assignee)

Comment 8

5 months ago
Gonna mark this verified per comment #7 - thanks!
Status: RESOLVED → VERIFIED
== Change summary for alert #17851 (as of Tue, 27 Nov 2018 15:04:02 GMT) ==

Improvements:

  2%  tart pgo e10s stylo     1.68 -> 1.64

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=17851

Updated

3 months ago
Depends on: 1518863
(Assignee)

Comment 10

3 months ago

Ryan, what do you need from me if we want to back this out of 65 for exacerbating bug 1518863 ? (The fix here shouldn't really affect the default situation on beta/release because the process specifically for activity stream doesn't exist on beta/release - but the change does affect webextension pages, where it breaks as per bug 1518863.)

Flags: needinfo?(ryanvm)

Comment 11

3 months ago
backoutuplift

Backed out from Beta for Fx65. This remains landed for Fx66+.

https://hg.mozilla.org/releases/mozilla-beta/rev/8f38ca46fb44

Flags: needinfo?(ryanvm)

Gijs, is backing this out for 66 an option? This is because of bug 1518863, which may affect around 1.5 million users by ddurst's estimate.

I just realized that I assumed it wasn't possible to back out at this point, but that I hadn't actually asked.

Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 13

a month ago

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #12)

Gijs, is backing this out for 66 an option?

Yes, but at least for me when testing locally, backing it out is not enough to fix bug 1518863. The root cause of the issue is a race condition between when we load the initial tab and when we initialize some add-on stuff.

This is because of bug 1518863, which may affect around 1.5 million users by ddurst's estimate.

I just realized that I assumed it wasn't possible to back out at this point, but that I hadn't actually asked.

So it's possible, but IME that would not actually fix the issue for everyone. If Kris is comfortable with it, I would recommend uplifting bug 1518863 given the number of affected users.

Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 14

a month ago

(In reply to :Gijs (he/him) from comment #13)

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #12)

Gijs, is backing this out for 66 an option?

Yes, but at least for me when testing locally, backing it out is not enough to fix bug 1518863. The root cause of the issue is a race condition between when we load the initial tab and when we initialize some add-on stuff.

Expanding on that: this patch made it more likely that we try to load the page before the add-on is fully initialized, but without this patch that can (and in my case, does) still happen.

You need to log in before you can comment on or make changes to this bug.