1785898 - Optimization: use StartupCache to skip database access in ExtensionScriptingStore when there are no persisted scripts

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P1)

Description

[Rob Wu [:robwu]]

Currently, the implementation delays extension initialization until the ExtensionScriptingStore has read from the rkv database. This may result in a non-negligible delay. The scripting namespace is not only used for persistently registered content scripts, so that behavior is suboptimal.

We should use the StartupCache to avoid expensive delays at https://searchfox.org/mozilla-central/rev/db4b1d66c4b409bdbce43f3f3498401f5303d961/toolkit/components/extensions/Extension.jsm#3106

This StartupCache data could be maintained at https://searchfox.org/mozilla-central/rev/db4b1d66c4b409bdbce43f3f3498401f5303d961/toolkit/components/extensions/ExtensionScriptingStore.jsm#243,285-287,292

Because the StartupCache format is wiped when the application version changes, we don't need to worry about data migrations, and can afford to prioritize ease/speed of implementation over complexity that may be more involved.

For example:

• Just saving a flag that tells whether there are any persisted scripts can already offer performance benefits to extensions that don't use persistent content scripts.
• Caching the whole set of registered content scripts can offer performance benefits of all extensions including extensions with persisted scripts, but requires a more careful design to avoid the cached and persisted data getting out-of-sync.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1785898 - Cache into StartupCache a boolean flag to determine if there is any persisted content script. r?mixedpuppy!,robwu

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f906d6cc6199
Cache into StartupCache a boolean flag to determine if there is any persisted content script. r=robwu

Comment 3

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f906d6cc6199

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 9290967 [details]
Bug 1785898 - Cache into StartupCache a boolean flag to determine if there is any persisted content script. r?mixedpuppy!,robwu

Beta/Release Uplift Approval Request

• User impact if declined: Potential impact on the user perceived performance on startup due to waiting on the underlying rkv store to load all data related to content scripts registered as persistAcrossSession: true (if a user has an extension installed which is requesting the "scripting" permission and registering webRequest blocking listeners for web content requests intercepted during the browser startup).

As a site note about Android builds:

• Android builds would be expected to be potentially affected as a Desktop build would be, given that the scripting API and the area where the changes in this patch are applied are both at toolkit/ level (and so shared by both Desktop and Android builds).
• The xpcshell test covering the fix is expected to also be running on Android builds (and so to cover the expected behaviors on both Desktop and Android builds).

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): - The change is quite small and pretty much restricted to the ExtensionScriptingStore.jsm, which is only used on extensions that have access to the scripting API.

• Both the scripting API and the persistAcrossSession option have been introduced recently enough (and so they are not too commonly used yet by the current versions of the top most installed extensions).

• The fix have been baking for a few days in Nightly, manually verify the fix is a bit tricky (especially given that there may not be many existing extensions already using these features) but a new test (included in the same patch) is explicitly asserting the expected behaviors.

• String changes made/needed:
• Is Android affected?: Yes

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Am I understanding correctly that the only users incurring the startup performance penalty are those using addons which use the scripting API? Or is everyone hitting it regardless of which extensions they have installed?

Comment 6

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

Am I understanding correctly that the only users incurring the startup performance penalty are those using addons which use the scripting API? Or is everyone hitting it regardless of which extensions they have installed?

Yes, that is correct, the patch from Bug 1751436 as landed in Firefox 105 only calls await lazy.ExtensionScriptingStore.initExtension(this); if this.hasPermission("scripting") :

• https://searchfox.org/mozilla-beta/rev/a3e5db32e1cae7be6ea845eba1050658f4883f35/toolkit/components/extensions/Extension.jsm#3099-3111

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9290967 [details]
Bug 1785898 - Cache into StartupCache a boolean flag to determine if there is any persisted content script. r?mixedpuppy!,robwu

Avoid a startup performance penalty introduced in 105 for those who are using addons which use the scripting API. Approved for 105.0b5.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2764d566a835