move built-in add-ons into omni.ja
Categories
(Toolkit :: Add-ons Manager, enhancement, P3)
Tracking
()
Performance Impact | medium |
People
(Reporter: rhelmer, Unassigned)
References
(Blocks 3 open bugs)
Details
(Keywords: perf:startup, Whiteboard: triaged)
Attachments
(3 obsolete files)
Updated•8 years ago
|
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Updated•7 years ago
|
Comment 5•7 years ago
|
||
Comment 6•7 years ago
|
||
Comment 7•7 years ago
|
||
Updated•7 years ago
|
Comment 8•7 years ago
|
||
Comment 9•7 years ago
|
||
Comment 10•7 years ago
|
||
Comment 11•7 years ago
|
||
Comment 12•7 years ago
|
||
Reporter | ||
Comment 13•7 years ago
|
||
Reporter | ||
Comment 14•7 years ago
|
||
Comment 15•7 years ago
|
||
Reporter | ||
Comment 16•6 years ago
|
||
Comment 17•6 years ago
|
||
Reporter | ||
Comment 18•6 years ago
|
||
Reporter | ||
Comment 20•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #19)
Rob, are you still actively working on this?
I am not, updated bug to reflect this. Happy to help anyone who wants to take it if needed.
Comment 21•5 years ago
|
||
I'd like to transform this bug into "deprecate system add-ons".
The immediate motivation is that we spend a lot of time (nearly 2 seconds in a recent profile on the 2018 reference device: https://perfht.ml/2KfLqkX) "installing" system add-ons (this is just initializing the addon manager state for them, the actual xpis are just read from the application directory) when creating a new profile. I suspect we spend a similar amount of time starting up after an application update, though I haven't measured it.
We currently ship a lot of code in system addons that we never update out-of-cycle. In some cases (screenshots, webcompat-reporter) this is since the code is written as an extension and system addons are currently the only way to deploy such an extension. I'm going to explore using the new built-in extension capability for screenshots in bug 1568270. In other cases (formautofill), the non-changing code is simply bundled with code that we want to update periodically (ie, site heuristics for finding form fields to autofill). Moving the non-changing code into built-in components will mitigate some of the startup cost of system addons but it will also yield other benefits. For instance, system addons can't use fluent and use various hacks to do localization. Moving code that uses localized strings to built-in components will let us use the same localization tools as other browser code, which will be much smoother. It will also make it easier to make changes to when these components are loaded and initialized during startup.
For the situations where we want to be able to ship changes out-of-cycle (eg webcompat interventions, formautofill heuristics), we have Normandy, which isn't able to deliver extensions in this way yet but that work is planned in bug 1436111. At least formautofill and perhaps webcompat will need a bit of work to separate the unchanging bits that we want to just ship with the browser from the parts that we want to be able to update on the fly, but that should be a modest amount of work. I'll file bugs for that work shortly. But once Normandy is ready, there's so reason for us to maintain two different methods to ship updates.
Comment 22•5 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #21)
For the situations where we want to be able to ship changes out-of-cycle (eg webcompat interventions, formautofill heuristics), we have Normandy, which isn't able to deliver extensions in this way yet but that work is planned in bug 1436111.
We could also use remote settings here, for these two examples, I think? That may have some benefits over using normandy (mostly not needing to wrap code in extension wrappers at all anymore, which would likely keep some of the extant overhead). That said, for webcompat interventions we may need to do some thinking, given that we may in some cases need to block content loads until we've loaded interventions, in case they e.g. modify UA headers for the content requests, just like for blocking webrequest listeners in the add-on world...
At least formautofill and perhaps webcompat will need a bit of work to separate the unchanging bits that we want to just ship with the browser from the parts that we want to be able to update on the fly, but that should be a modest amount of work. I'll file bugs for that work shortly. But once Normandy is ready, there's so reason for us to maintain two different methods to ship updates.
Did you intend to file bugs blocking this bug?
Comment 23•5 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #21)
I'd like to transform this bug into "deprecate system add-ons".
I read this as "system add-ons => built-in add-ons, system add-on upgrades => remote settings". That sounds great: it would have avoided complicated races like that in Bug 1570690.
Reporter | ||
Comment 24•5 years ago
|
||
(In reply to Nick Alexander :nalexander [he/him] from comment #23)
(In reply to Andrew Swan [:aswan] from comment #21)
I'd like to transform this bug into "deprecate system add-ons".
I read this as "system add-ons => built-in add-ons, system add-on upgrades => remote settings". That sounds great: it would have avoided complicated races like that in Bug 1570690.
Yes I think it's fair to characterize it this way. The distinction between "Remote Settings" and "Normandy" isn't super useful for the purposes of this bug (Normandy uses Remote Settings to do its thing, for instance), but the gist is that Normandy is the thing that can do complex user targeting. System add-ons used AUS aka Balrog for this purpose (which wasn't as expressive as Normandy FWIW).
The goal of deprecating the system add-ons code in the add-ons manager should be the main thing for this bug, I think. One thing we'll need to take care about is to keep the old system going until we've phased out the last release we care about - for armag-add-on this was 2 ESRs back. This is mostly a server-side concern though I think ripping out the client code shouldn't be a problem, although I think we should have at least one release where both Normandy and SAOs are functional for delivering add-ons, just in case.
(In reply to :Gijs (he/him) from comment #22)
We could also use remote settings here, for these two examples, I think? That may have some benefits over using normandy (mostly not needing to wrap code in extension wrappers at all anymore, which would likely keep some of the extant overhead). That said, for webcompat interventions we may need to do some thinking, given that we may in some cases need to block content loads until we've loaded interventions, in case they e.g. modify UA headers for the content requests, just like for blocking webrequest listeners in the add-on world...
I agree that in general we should try to send data updates to users, and not privileged code. Sometimes extensions make sense but in many cases I think we can still update users rapidly and restartlessly without the overhead.
Comment 25•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #22)
(In reply to Andrew Swan [:aswan] from comment #21)
For the situations where we want to be able to ship changes out-of-cycle (eg webcompat interventions, formautofill heuristics), we have Normandy, which isn't able to deliver extensions in this way yet but that work is planned in bug 1436111.
We could also use remote settings here, for these two examples, I think? That may have some benefits over using normandy (mostly not needing to wrap code in extension wrappers at all anymore, which would likely keep some of the extant overhead).
Maybe? This may be dated now but in the case of formautofill, :MattN told me at one point that form detection heuristics need to do such a variety of things that they are shipped as javascript rather than some sort of declarative pattern-matching rules. So, I think using remote settings would mean shipping a string that we take care to evaluate in a sandbox of some sort? I had originally thought that would be undesirable but perhaps it wouldn't be so bad.
That said, for webcompat interventions we may need to do some thinking, given that we may in some cases need to block content loads until we've loaded interventions, in case they e.g. modify UA headers for the content requests, just like for blocking webrequest listeners in the add-on world...
Yes, I think Tom has been thinking through this exact scenario and discussing it with :kmag. I'm not sure where that discussion left off or if there's an open bug for planned work on webcompat (ni to Tom for this question)
At least formautofill and perhaps webcompat will need a bit of work to separate the unchanging bits that we want to just ship with the browser from the parts that we want to be able to update on the fly, but that should be a modest amount of work. I'll file bugs for that work shortly. But once Normandy is ready, there's so reason for us to maintain two different methods to ship updates.
Did you intend to file bugs blocking this bug?
I did intend to, yes :/ As mentioned above, I think Tom and the webcompat team are already making plans. Tom, can you make any relevant bugs a dependency of this one? I can file bugs for webcompat-reporter and formautofill, leaving the ni to myself to remember to do that.
Comment 26•5 years ago
|
||
There are no relevant bugs to mark as dependencies yet, as my proposal to kmag may not be worthwhile. Do we know how long it might take to block content loads until the webcompat addon starts up? If it won't take long, then my proposal is moot.
I was just concerned that it may take a long time before we can improve the webcompat module, so I proposed moving much of it into Core as its own module, so it could start up independently of the addon framework, and do so before content loads occur. But I wouldn't want to go this route if we can just fix the addons code relatively soon.
Of course we need webRequest and its related filter capabilities (modifying headers or incoming script content, blocking requests). We also need to be able to run content scripts before page scripts run.
But ideally the webcompat module needs to be able to detect more conditions on all content frames that we can currently detect with existing addon APIs. For instance, we cannot reliably tell if a broken JS library is loading based on its URL, because there are so many webpacked scripts out there. We need to be able to detect when (say) jQuery
is defined on a window, and run some JS to check its version and fix it.
One way to handle that would be to allow the compat addon to run a simple content script on all content frames so it can detect the variables we're interested in, and potentially let it load further content scripts to polyfill things before the rest of the page scripts may run. Of course we could presumably also add a new addon filter which does the same for us.
(Note also that our addon registers the about:compat
page using an experiment. It would be nice if that could happens before session restore tries to load it, resulting in an "address isn't valid" error page).
Updated•5 years ago
|
Comment 27•5 years ago
|
||
It looks like Shane is taking this over, I'm not sure if he wants to handle these extensions one-by-one or all at once.
It appears to me that that most straightforward path here is to use Normandy to move these to the built-in location and use Normandy to ship updates to them. I think this depends on bug 1613647, but there was some confusion about this on Matrix. Perhaps Shane could fill us in here on his plans...
Comment 28•5 years ago
|
||
I don't have definitive plans. I took a look and it seemed simple to load these into omni.jar. My initial goal is to see if we still have issues or not per bug 1631932 comment 7.
to use Normandy to move these to the built-in location
I don't understand that, why would normandy need to move these?
use Normandy to ship updates to them
That's the plan, to transition from Balrog to Normandy, then shut Balrog down.
Comment 29•5 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #28)
to use Normandy to move these to the built-in location
I don't understand that, why would normandy need to move these?
Sorry, that was an abbreviated way of describing the approach of distributing the default addons in the builtin location and any out-of-cycle updates via Normandy. The current system add-on implementation is all-or-nothing, moving the defaults to a different location is going to cause it to think it has to download updated versions of everything. You could try changing the default system addon location to read from resource: urls but that would be throwaway work if we're just going to get rid of the old system addon implementation.
Comment 30•5 years ago
|
||
use Normandy to ship updates to them
That's the plan, to transition from Balrog to Normandy, then shut Balrog down.
Shane -- does "shut Balrog down" mean "stop using Balrog to serve add-on updates"? Or is it more general?
Comment 31•5 years ago
|
||
(In reply to Nick Alexander :nalexander [he/him] from comment #30)
use Normandy to ship updates to them
That's the plan, to transition from Balrog to Normandy, then shut Balrog down.
Shane -- does "shut Balrog down" mean "stop using Balrog to serve add-on updates"? Or is it more general?
I suppose that balrog will be necessary for some period of time after normandy is delivering updates. I don't know what that period of time is, but yes, ultimately there would be some point in time where it could be shut down. Unfortunately, it probably will have to live long enough to serve ESR 78 (if we do that). I think the normandy team will have a better idea of a timeline in which they are handling the functionality that balrog provides.
Reporter | ||
Comment 32•5 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #31)
(In reply to Nick Alexander :nalexander [he/him] from comment #30)
use Normandy to ship updates to them
That's the plan, to transition from Balrog to Normandy, then shut Balrog down.
Shane -- does "shut Balrog down" mean "stop using Balrog to serve add-on updates"? Or is it more general?
I suppose that balrog will be necessary for some period of time after normandy is delivering updates. I don't know what that period of time is, but yes, ultimately there would be some point in time where it could be shut down. Unfortunately, it probably will have to live long enough to serve ESR 78 (if we do that). I think the normandy team will have a better idea of a timeline in which they are handling the functionality that balrog provides.
I suspect Nick's concern is around Balrog-the-server being shut down, vs. us not using it to serve system add-ons anymore. Nick is that correct?
If I have that right, then the answer is that Balrog will still be around for App Update and Media Plugins, just not System Add-ons.
Comment 33•5 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #32)
(In reply to Shane Caraveo (:mixedpuppy) from comment #31)
(In reply to Nick Alexander :nalexander [he/him] from comment #30)
use Normandy to ship updates to them
That's the plan, to transition from Balrog to Normandy, then shut Balrog down.
Shane -- does "shut Balrog down" mean "stop using Balrog to serve add-on updates"? Or is it more general?
I suppose that balrog will be necessary for some period of time after normandy is delivering updates. I don't know what that period of time is, but yes, ultimately there would be some point in time where it could be shut down. Unfortunately, it probably will have to live long enough to serve ESR 78 (if we do that). I think the normandy team will have a better idea of a timeline in which they are handling the functionality that balrog provides.
I suspect Nick's concern is around Balrog-the-server being shut down, vs. us not using it to serve system add-ons anymore. Nick is that correct?
Correct.
If I have that right, then the answer is that Balrog will still be around for App Update and Media Plugins, just not System Add-ons.
Thanks!
Updated•3 years ago
|
Updated•2 years ago
|
Description
•