Closed Bug 568917 Opened 11 years ago Closed 5 years ago

urlclassifier should be lazily init'd on first use, not just 2s after startup

Categories

(Toolkit :: Safe Browsing, defect, P5)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: johnath, Unassigned)

References

Details

(Whiteboard: [ts])

Right now, the urlclassifier is initialized on a 2s timeout fired on load of the browser. Since urlclassifier init is often the first use of crypto code, this also necessitates psm and nss init, which is a costly thing to be doing just as the user starts to want to use their browser.

This should probably be init'd off of the idle service, or lazily at first use, rather than on a fixed timer.
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [ts]
wouldn't urlclassifier be initialized anyway when safebrowsing checks the url of the first content page loaded?
(In reply to comment #1)
> wouldn't urlclassifier be initialized anyway when safebrowsing checks the url
> of the first content page loaded?

Yes.
This almost got fixed in bug 769960, then undid it in bug 778855 due to a perf regression.

_delayedStartup() in browser.js currently has:

1378 #ifdef MOZ_SAFE_BROWSING
1379     // Bug 778855 - Perf regression if we do this here. To be addressed in bug 779008.
1380     setTimeout(function() { SafeBrowsing.init(); }, 2000);
1381 #endif

Leaving _this_ bug open because bug 779008 will remove the timeout, but ultimately this should be out of the startup path entirely.
Product: Firefox → Toolkit
Given comment 2, I don't see how lazily initializing Safe Browsing would change anything. Marking as WONTFIX, but feel free to reopen if there's something I'm missing.
Status: NEW → RESOLVED
Closed: 5 years ago
Priority: -- → P5
Resolution: --- → WONTFIX
Here's the thing with startup (at least the last time I profiled it, which was some time ago):

You get non-deterministic results from changing around the order or modifying the set of services that start when the first browser window loads. For example: Sometimes removing a service will *hurt* window load time, because a thing that had previously loaded after our measurement was taken, has now moved up in the queue and initializes before the measurement, and takes longer than the thing you removed. Followed by inhuman howling and much gnashing of teeth.

So comment 2 *might* be correct. And if it is, it *might* impact performance in a way that is counter to our assumptions.

I took a quick look in _delayedStartup(), and didn't see the urlclassifier there. But I didn't look very hard. Some of these services now load by observing the startup topic, so don't load the same way anymore. Which means this bug might be irrelevant as originally filed, or the change might need be done in a different place.

However, we are apparently still playing games to avoid NSS/PSM initialization:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1346

I wonder if we have telemetry around when NSS/PSM *actually* initialize in real life... For example, seems as if the default loading page at startup was https then NSS would initialize regardless, and that might be a more likely scenario these days.
Tim, do you know anything about what triggers NSS/PSM initialization and whether we have telemetry that tracks at what point in the startup timeline it happens?

(Kinda seems like we just should force initialize it and call it game over. If our mission to encrypt the world is successful, that's where we'll end up anyway.)
Flags: needinfo?(ttaubert)
(In reply to Dietrich Ayala (:dietrich) from comment #5)
> I wonder if we have telemetry around when NSS/PSM *actually* initialize in
> real life... For example, seems as if the default loading page at startup
> was https then NSS would initialize regardless, and that might be a more
> likely scenario these days.

It seems like if you take into account Safe Browsing, Tracking Protection, snipets, tiles and the new remote about:newtab, NSS initialization will happen pretty early on.
(In reply to Dietrich Ayala (:dietrich) from comment #6)
> Tim, do you know anything about what triggers NSS/PSM initialization and
> whether we have telemetry that tracks at what point in the startup timeline
> it happens?

I don't know what exactly triggers it. I suspect David knows more?

> (Kinda seems like we just should force initialize it and call it game over.
> If our mission to encrypt the world is successful, that's where we'll end up
> anyway.)

We might already do that anyway? Should check this.

(In reply to François Marier [:francois] from comment #7)
> (In reply to Dietrich Ayala (:dietrich) from comment #5)
> > I wonder if we have telemetry around when NSS/PSM *actually* initialize in
> > real life... For example, seems as if the default loading page at startup
> > was https then NSS would initialize regardless, and that might be a more
> > likely scenario these days.
> 
> It seems like if you take into account Safe Browsing, Tracking Protection,
> snipets, tiles and the new remote about:newtab, NSS initialization will
> happen pretty early on.

Sounds about right.
Flags: needinfo?(ttaubert) → needinfo?(dkeeler)
Currently PSM/NSS is initialized when observers of "addons-startup" are notified:

0 <TOP LEVEL> ["resource://gre/modules/addons/XPIProvider.jsm":1647]
1 AddonManagerInternal.startup() ["resource://gre/modules/AddonManager.jsm":892]
    this = [object Object]
2 this.AddonManagerPrivate.startup() ["resource://gre/modules/AddonManager.jsm":2774]
    this = [object Object]
3 amManager.prototype.observe(aSubject = null, aTopic = "addons-startup", aData = null) ["file:///home/keeler/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/components/addonManager.js":57]
    this = [object Object]

I believe this is unintentional and perhaps unwanted. That top line is https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/toolkit/mozapps/extensions/internal/XPIProvider.jsm#1647 :

let gCertDB = Cc["@mozilla.org/security/x509certdb;1"]
              .getService(Ci.nsIX509CertDB);

Some interfaces are backed by implementations that know they require NSS, so they initialize PSM/NSS when they are instantiated. If something like that doesn't happen, though, PSM/NSS is initialized by necko via net_EnsurePSMInit: https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/netwerk/base/nsNetUtil.cpp#2131

It would save us a great many headaches if PSM/NSS were initialized in a deterministic, consistent manner rather than "whenever", which is what we have now.
Flags: needinfo?(dkeeler)
(Whoops, didn't answer all of Dietrich's questions.)

(In reply to Dietrich Ayala (:dietrich) from comment #6)
> Tim, do you know anything about what triggers NSS/PSM initialization and
> whether we have telemetry that tracks at what point in the startup timeline
> it happens?

We don't have telemetry on that.
You need to log in before you can comment on or make changes to this bug.