Cache instead of re-fetching manifests on every background startup through chrome.storage.session

[DNin01]

Changes

Caches addon manifest and addons-l10n files in the session storage area to avoid having to load them from disk every time the service worker wakes up.

The session storage area is supported in Chrome 102+ and Firefox 115+. If run in a browser that does not support this storage area, this data will not be cached but Scratch Addons will still work.

This does not change the structure of Scratch Addons' global objects.

Session storage is cleared every time the extension starts up (e.g. updated, reloaded, disabled and reenabled), so changes developers make to locales or addon manifests should update after an extension reload.

Reason for changes

It is not ideal to load the same files from disk every service worker reload.

Tests

Tested in Edge 124.

[WorldLanguages]

Currently, session storage contains 724,912 bytes, which is 69% of the 1,048,576-byte quota

It may vary in each language, and we always continue to add new addons to the extension.

[WorldLanguages]

This is a quite dangerous pattern. Reading the cache once for each locale, inside the loop? If we just checked Spanish and set the cache, and we're now checking English, how would the cache we just built be useful?
Splitting the cache by locale also sounds unnecessary.

I have the following idea instead.

• Each time load() is called (which might be multiple times per session, even though it's only called once for now) the value of this.messages should be stored in the cache, after the loop has finished.
• We should add a loadFromCache() method to l10n.js that takes an object, sets this.messages to it, calls this._reconfigure() and sets this.loaded appropriately.
• The cache should not be read by l10n.js. It should only be read by load-addon-manifests.js once on startup/wakeup. If the cache exists and the addonIds list matches the one that was used to build the cache (either because the code checked it, or we add a code comment that explains why it's guaranteed that if the cache exists then the addons.json file hasn't changed) then instead of calling load() it should call the new loadFromCache() method and pass the cached messages object to it.

[WorldLanguages]

Also, to clarify, this for-loop exists to look for strings that may not be translated yet, so it fallbacks to English for those (it is basically the same algorithm that chrome.i18n.getMessage() uses internally when trying to find a string)

[DNin01]

I think I'm starting to understand this code...

[DNin01]

What would we do in the case where the user changes their display language while Scratch Addons is open? Where's the code that handles that?

[WorldLanguages]

Browsers require a restart to change their UI language, so we don't need to consider that case.

[WorldLanguages]

You should clarify your question, this is not simple. In some extreme cases, our strings could display in three different languages at the same time.
For example, if you set your Chrome language to French (the language used by Chrome for its UIs, which doesn't necessarily equal navigator.languages[0]) but you enable the Show addon names and descriptions in English toggle in More Settings, and your Scratch language is set to Japanese in the footer. In this case, the settings page will show some strings in French (those that go through chrome.i18n.getMessage()) and some in English (those that were originally fetched by the background context). Strings in the Scratch website will attempt to match its Japanese surroundings.
In the popup, I believe the names of the tabs (Messaging, Games, etc.) will be in French while the strings used within the popup will be in English. Quite a mess. There's an issue about this somewhere.

[WorldLanguages]

@DNin01 I will try to get this merged for v1.38.1

The PR needs to be changed (or is a new one needed?) to try to merge to master instead of mv3 branch

[DNin01]

I changed the base to master.

I might need your help with some of the code, so feel free to take over this PR.

[WorldLanguages]

We use fetch inside background/ in [8 places](https://github.com/search?q=repo%3AScratchAddons%2FScratchAddons%20path%3A%2F%5Ebackground%5C%2F%2F%20fetch(&type=code) in total and we just checked off 2. I would assume the 6 remaining uses that don't have a cache are not impacting performance enough to worry about, unless you want to.

@DNin01 Well, fetching the licenses each time doesn't sound like the best idea. I think I will fix that in a different PR.

Still, are we certain that caching stuff in memory helps in some way? Of course, it's better if we don't read from disk that often, but doesn't Chrome already read from disk when starting up our service worker again? It's hard for me to believe that Chrome internals do zero I/O operations on disk when waking up a service worker.

(I guess this still speeds up things in the context of our extension though, as taking too long to fetch the manifests blocks the whole extension from working until it's done. The bad news is that while the SW is alive, we're storing the manifests twice in memory, but it's still far from 5MB I believe)

[DNin01]

The bad news is that while the SW is alive, we're storing the manifests twice in memory, but it's still far from 5MB I believe

There might be some data that doesn't have to be stored persistently in memory, such as data from addons that are currently disabled, or information that is only relevant when displaying addons in the settings page (like descriptions and notices) or that will only be needed when addons are running (like custom CSS variables). Fetching the addon.json file again to get this data before it runs shouldn't be that bad since we'd also have to fetch each addon's JS and CSS from disk anyway.

We should still optimize our resource use, though. Like, if we never persistently stored any data about addons, then we'd be adding unnecessarily to disk usage, but storing everything in memory all the time can also be unnecessary. The optimal efficiency strategy will depend on what the user is doing and how long it's been since they interacted with Scratch or Scratch Addons.

But since the switch to a service worker, which I would assume eats up less memory, our memory usage is already dropping, so we can probably get away with storing redundant data for now, and optimize it later.

[WorldLanguages]

But since the switch to a service worker, which I would assume eats up less memory, our memory usage is already dropping

I believe this is correct. The absence of DOM seems to make quite a difference in overall memory usage, and we've only lost Scratch Notifier capabilities in the process. It's very unfortunate Chrome has decided to kill workers so often - maybe it's intentional so that developers are forced to handle wake ups.

[WorldLanguages]

@DNin01 Could you test it one more time?

[DNin01]

I couldn't find any problems, and the caches are working.

[DNin01]

Well, fetching the licenses each time doesn't sound like the best idea. I think I will fix that in a different PR.

Would it be that bad not to store license information in memory or load them into the service worker at all? It wouldn't take very long to load them only when the licenses page is opened, and we could display "Loading..." for those few seconds.

[WorldLanguages]

It wouldn't take very long to load them only when the licenses page is opened, and we could display "Loading..." for those few seconds.

I agree, the licenses should be requested by the licenses page itself. I'm not sure why it's handled by the background context.