Opened 3 months ago

Closed 7 weeks ago

Last modified 3 weeks ago

#33342 closed defect (fixed)

Disconnect search addon causes error at startup

Reported by: sysrqb Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-9.5a9, TorBrowserTeam202004R, tbb-no-uplift
Cc: acat Actual Points: 0.5
Parent ID: Points:
Reviewer: brade,mcs Sponsor:

Description

Following #32767, it seems Firefox is throwing an exception. I don't think it's too important.

1581718500372   addons.xpi-utils    WARN    addMetadata: Add-on disconnect@search.mozilla.org is invalid: Error: File resource://search-extensions/disconnect/ does not contain a valid manifest(resource://gre/modules/addons/XPIInstall.jsm:671:11) JS Stack trace: loadManifest@XPIInstall.jsm:671:11
awaitPromise@XPIProvider.jsm:228:15
syncLoadManifest@XPIInstall.jsm:750:24
addMetadata@XPIDatabase.jsm:2721:32
processFileChanges@XPIDatabase.jsm:3162:26
getNewSideloads@XPIProvider.jsm:3005:28
1581718500362   addons.xpi-utils    WARN    Not uninstalling invalid item because it is a proxy file

We may not need to do anything about this, but if there is a way we can prevent this error then that'll be even better (reducing noise in logging is helpful).

acat, what do you think? I haven't looked at the code in the stacktrace at all.

Child Tickets

Change History (13)

comment:1 Changed 3 months ago by gk

FWIW, I get something like

1582213850508	addons.xpi	WARN	Exception running bootstrap method startup on disconnect@search.mozilla.org: Error: Error while loading 'jar:file:///home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_sv-SE/Browser/browser/omni.ja!/chrome/browser/search-extensions/disconnect/manifest.json' (NS_ERROR_FILE_NOT_FOUND)(resource://gre/modules/Extension.jsm:513:20) JS Stack trace: readJSON/</<@Extension.jsm:513:20

comment:2 Changed 3 months ago by Thorin

FWIW: except for the path and timestamp, on Windows I get the same error as gk

comment:3 Changed 3 months ago by pili

Keywords: TorBrowserTeam202003 added; TorBrowserTeam202002 removed

We are no longer in February, moving tickets

comment:4 Changed 3 months ago by cypherpunks

error at startup

is

1581718500072   addons.xpi-utils	WARN	updateMetadata: Add-on disconnect@search.mozilla.org is invalid: Error: File resource://search-extensions/disconnect/ does not contain a valid manifest(resource://gre/modules/addons/XPIInstall.jsm:671:11) JS Stack trace: loadManifest@XPIInstall.jsm:671:11
awaitPromise@XPIProvider.jsm:228:15
syncLoadManifest@XPIInstall.jsm:750:24
updateMetadata@XPIDatabase.jsm:2832:32
updateExistingAddon@XPIDatabase.jsm:3048:23
processFileChanges@XPIDatabase.jsm:3137:31
checkForChanges@XPIProvider.jsm:2969:55
startup@XPIProvider.jsm:2429:12
callProvider@AddonManager.jsm:215:31
_startProvider@AddonManager.jsm:651:5
startup@AddonManager.jsm:897:14
startup@AddonManager.jsm:3493:26
observe@addonManager.js:70:29

comment:5 Changed 2 months ago by acat

Actual Points: 0.5
Keywords: TorBrowserTeam202003R added; TorBrowserTeam202003 removed
Status: newneeds_review

Patch for review in https://github.com/acatarineu/tor-browser/commit/33342.

Perhaps for the next time, we may want to keep the extension files but just remove the engine from list.json, and do a migration similar to https://searchfox.org/mozilla-esr68/rev/8927c721cdd45f2d692a6f00c4444c3ce725ff10/browser/components/BrowserGlue.jsm#2950 for uninstalling the addon manually (while it can be loaded properly).

comment:6 Changed 2 months ago by mcs

Reviewer: brade,mcs

Kathy and I started to review the patch.

comment:7 Changed 2 months ago by mcs

Status: needs_reviewmerge_ready

r=brade,r=mcs
The code looks good and we tested it on macOS.

Kathy and I agree that we should try to handle this via some migration code next time.

One question: would it also work to remove the addonStartup.json.lz4 file? If I remember correctly it is a cache that would get rebuilt based on the installed addons.

comment:8 Changed 2 months ago by sysrqb

Keywords: tbb-9.5a9 added
Status: merge_readyneeds_information

Thanks! I merged this onto tor-browser-68.6.0esr-9.5-1 (commit 69dfbacfa8b3d6abd6eb5e248fcd24a03c30a962). It's in 9.5a9.

acat, do you know an answer to mcs' question?

comment:9 Changed 2 months ago by acat

Sorry, I trusted my mail classification too much and missed mcs question.
Replying to mcs:

One question: would it also work to remove the addonStartup.json.lz4 file? If I remember correctly it is a cache that would get rebuilt based on the installed addons.

I think not, in the sense that it might break some stuff. For example, if you remove addonStartup.json.lz4 the search builtin extensions will not load at all, which means that the search icons in the urlbar will be broken (similar to #31563). Launching the browser with env variable RELOAD_ENGINES=1 makes SearchService.jsm reinstall these search extensions again, so perhaps removing addonStartup.json.lz4 + forcing search extensions reinstall might do the trick.

I'm not completely sure about the details, but I think one of the reasons why addonStartup.json.lz4 is "not just a cache", at least for builtin extensions, is that the BuiltInLocation is not enumerable, which means that in scanForChanges the builtin location is skipped and it cannot notice the fact that the addon disappeared from addonStartup.json.lz4 (because actually it was the only place this was persisted on). I think this would not be an issue if the search engines were temptatively installed every time, like it's done with the themes, but that's not the case (probably to make the browser load faster?).

comment:10 Changed 8 weeks ago by pili

Keywords: TorBrowserTeam202004 added

We are no longer in March

comment:11 Changed 7 weeks ago by pili

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202003R TorBrowserTeam202004 removed

I'm a bit confused whether this is reviewed or it needs revisions or can be closed (I see a patch was merged already.)

Can someone who knows better please update :) Thanks!

comment:12 in reply to:  11 Changed 7 weeks ago by mcs

Resolution: fixed
Status: needs_informationclosed

Replying to pili:

I'm a bit confused whether this is reviewed or it needs revisions or can be closed (I see a patch was merged already.)

Can someone who knows better please update :) Thanks!

In comment:9, Alex provided a good answer to my question. Closing.

comment:13 Changed 3 weeks ago by acat

Keywords: tbb-no-uplift added
Note: See TracTickets for help on using tickets.