1529321 - extension locale files all read on install

Status: NEW

(Toolkit :: Add-ons Manager, defect, P3)

Description

[Shane Caraveo (:mixedpuppy)]

• XPIInstall calls extension.initAllLocales, which reads every locale file in the extension.
• it also has a custom localization routine, it should update to use the new extension.getLocalizedManifest
• the addon object has addon.locales which is a full array of translated manifests (partial), but I cannot locate any consumer of this array, it should be removed

Comment 1

[Shane Caraveo (:mixedpuppy)]

This is a perf issue (not measured) when there are lots of extensions with lots of locale files, it should be addressed. However I do not feel it should block landing any work on search (though I blocked the meta bug) as it doesn't prevent search engines from working (ie. being tested).

Comment 2

[Dale Harvey (:daleharvey)]

Reading >100 extra files on startup is a blocker, I would be very surprised if it didnt break any tests but either way it visibly slow, we cant land it in that state

Comment 3

[Kris Maglione [:kmag]]

This doesn't happen at startup, it happens at extension install time, which will admittedly be at startup after a browser upgrade, but not in general.

Comment 4

[Dale Harvey (:daleharvey)]

Just to update what we are going to do after discussion on IRC. I have run talos and not seen any specific regression, will verify with the desktop perf team if there is anything I need to be checking before landing and then will do so

Comment 5

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

So from what I understand in this bug, we'll be reading 100+ new files off of the disk during start-up when the profile is new or being upgraded?

Why do we have to read all of these individual files?

Comment 6

[Shane Caraveo (:mixedpuppy)]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #5)

So from what I understand in this bug, we'll be reading 100+ new files off of the disk during start-up when the profile is new or being upgraded?

From omni.jar

Why do we have to read all of these individual files?

I'm not really clear about that, and from what I understand it would be a larger undertaking to address it. Kris seems to understand why its necessary.

Comment 7

[Andrew Swan [:aswan]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #6)

Why do we have to read all of these individual files?

I'm not really clear about that, and from what I understand it would be a larger undertaking to address it. Kris seems to understand why its necessary.

Two reasons really:

1. To validate the localization files at the moment of installation.
2. The addon manager can localize addon attributes like name, description, etc. Right now there is a ton of code that assumes those attributes can be read synchronously.

#2 could be addressed in various ways, but all the ways I can think of would be, as Shane says, a large undertaking. Doing that work still wouldn't address #1 though.

My questions are:

• Do we have an accurate count for how many locales are actually going to be read? The number "> 100" was mentioned above, are there built-in search engines that we use in that many different locales?
• Why does this even have to happen during startup? I would think we don't actually need to have search extensions all set up until a user actually tries to do a search (or until they see something search-related such as typing in the location bar)

Comment 8

[Dale Harvey (:daleharvey)]

The list of default engines are "google" - 6, "amazondotcom" - 1, "bing" - 1, "ddg" - 1, "ebay" - 13, "twitter" - 2, "wikipedia" - 91, so 115 by default, wikipedia is included in every configuration so its always at least 91

We dont load the engines in the SearchService until they are used, about:home is usually the first caller fairly early though, it has a search box and shows the installed engines on startup

Comment 9

[Mike de Boer [:mikedeboer]]

(In reply to Andrew Swan [:aswan] from comment #7)

• Why does this even have to happen during startup? I would think we don't actually need to have search extensions all set up until a user actually tries to do a search (or until they see something search-related such as typing in the location bar)

I can answer this one: We are initializing lazily, but there are pieces of main UI that require the list of visible engines to be there and, preferably, correct:

1. The URLBar says 'Search with Google or enter address'. This 'Google' thing is coming from the Search Service.
2. The about:newtab and/ or about:home page has a big fat search box that sports the default search engine's favicon, which is coming from the Search Service.

If the searchbar (yes, that old crufty thing) is placed in the URLBar, it kicks off the same machinery as the ones mentioned above.