Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improved descriptions] for extensions.getAddons.cache.enabled #615

Closed
v00p opened this issue Jan 17, 2019 · 33 comments
Closed

[improved descriptions] for extensions.getAddons.cache.enabled #615

v00p opened this issue Jan 17, 2019 · 33 comments

Comments

@v00p
Copy link

v00p commented Jan 17, 2019

This config not only disable updating, but fetching as well.
Might worth mentionning in the comment.

Current:

/* 0306: disable extension metadata updating
 * sends daily pings to Mozilla about extensions and recent startups ***/
user_pref("extensions.getAddons.cache.enabled", false);

Proposed:

/* 0306: disable extension metadata updating
 * sends daily pings to Mozilla about extensions and recent startups
 * [WARNING]: cause partial fetching of your extension's metadata as well
 * [1] (Before/After) http://i.epvpimg.com/RdFVcab.gif ***/
user_pref("extensions.getAddons.cache.enabled", false);
Before/Afer sample:
@Thorin-Oakenpants
Copy link
Contributor

Ahh, I never knew that clicking an extension card to load in it's own "page", actually expanded to show all the text. I just assumed each extension showed a one-liner, and the own page was to set update policy per extension, show version number, etc.

Worthy of a [note] perhaps. When the pref is false, it doesn't display the extra information, even though you already had it (from toggling the pref) .. so does this mean that it then connects every single time to AMO to get that info?

@Thorin-Oakenpants
Copy link
Contributor

thanks @v00p 💋

@v00p
Copy link
Author

v00p commented Jan 18, 2019

When the pref is false, it doesn't display the extra information, even though you already had it (from toggling the pref) .. so does this mean that it then connects every single time to AMO to get that info?

No it doesn't. It seems to do on a scheduled-basis.
Toggling the config on/off doesn't. fetch from togglin on/off is performed entirely locally,
using the cached data from your initial installation of the extension.

Which means any extensions that you previously installed while this config was set to "false", won't have that missing data present locally. You'll have to reinstall with the config turned on to have em cached and shown.

@Thorin-Oakenpants
Copy link
Contributor

I'll reopen, so we can investigate further. The purpose of the pref may have changed over time

@Thorin-Oakenpants
Copy link
Contributor

This is old, probably added when FF4 came out - but it also hasn't been removed or updated AFAICT: https://blog.mozilla.org/addons/how-to-opt-out-of-add-on-metadata-updates/

In order to keep this information updated, Firefox will ask the Mozilla Add-ons gallery for information about the add-ons you have installed once a day. This involves sending the identifiers of each add-on you have installed to Mozilla, as well as information on how long it last took Firefox to start up.

@Thorin-Oakenpants
Copy link
Contributor

I am not an expert at reading this stuff....

There is a pref called extensions.getAddons.cache.lastUpdate which if you look here you can see it tries to work out if the metadata is stale or not

metadataAge() {
    let now = Math.round(Date.now() / 1000);
    let lastUpdate = Services.prefs.getIntPref(PREF_METADATA_LASTUPDATE, 0);
    return Math.max(0, now - lastUpdate);
  },

  isMetadataStale() {
    let threshold = Services.prefs.getIntPref(PREF_METADATA_UPDATETHRESHOLD_SEC, DEFAULT_METADATA_UPDATETHRESHOLD_SEC);
    return (this.metadataAge() > threshold);
  },

.. and if you follow the crumbs it seems to call async getCachedAddonByID (at line 395) and if I scroll further down (line 490)

 /**
   * Fetch metadata for a given set of addons from AMO.
   *
   * @param  aIDs
   *         The array of ids to retrieve metadata for.
   * @returns {array<AddonSearchResult>}
   */
  async getAddonsByIDs(aIDs) {
    return this._fetchPaged(aIDs, PREF_GETADDONS_BYIDS,
                            results => results.map(
                              entry => this._parseAddon(entry)));
  },

I need someone like @earthlng to confirm what exactly is actually going on, and also that time factor (it's probably once a day)

I guess the question is, does this do any harm - given we set extension update checks = on: I'm not worried about Mozilla getting info TBH, but blocking this would seem to be a bit pointless: so at the very least we could make it inactive (but leave the pref in for those who want to disable it)

I also wonder about lightweightThemes.update.enabled - aren't personas dead?

@v00p
Copy link
Author

v00p commented Jan 18, 2019

Hmmm you might be right. I completely forgot the nasty "scheduled phone stab" scenario..
And they removed all of the former extensions.getAddons.cache.* configs... a workaround could have been to increase the extensions.getAddons.cache.updateThreshold value, but they removed it as well..

I'll digg a little more...

@Atavic
Copy link

Atavic commented Jan 18, 2019

browser_preferences_usage.js at line 100 uses function checkPrefGetters and says it is accessed in debug only.

@claustromaniac
Copy link
Contributor

claustromaniac commented Jan 18, 2019

metadataAge() and isMetadataStale() methods are never called (it's unused code).

Some help...

getCachedAddonByID() and cacheAddons() are called in XPIInstall.jsm (here and here).

The metadata seems to be fetched mostly on addon install/update, with few exceptions, but I don't have more time to keep digging ATM. EDIT: actually, fuck this. ✌️😎🍷

@Thorin-Oakenpants

This comment has been minimized.

@claustromaniac

This comment has been minimized.

@v00p

This comment has been minimized.

@Thorin-Oakenpants

This comment has been minimized.

@Thorin-Oakenpants

This comment has been minimized.

@v00p

This comment has been minimized.

@Thorin-Oakenpants

This comment has been minimized.

@v00p

This comment has been minimized.

@claustromaniac

This comment has been minimized.

@Thorin-Oakenpants
Copy link
Contributor

bump .. anyone any wiser as to when / if addon metadata cache checks for updates?

@Atavic
Copy link

Atavic commented Jan 28, 2019

Not with updates disabled. I have updates disabled, plus the entry
extensions.update.url is twartwed with the value http://0.0.0.0
This way in about:Addons even a manual click on Check for Updates via the top right Icon gives:
No updates Found

Note: I also have browser.addon-watch.interval twartwed with -1

@Atavic
Copy link

Atavic commented Jan 28, 2019

^I'm on ESR.

Btw by checking around I see that going to Help>About Firefox and clicking for updates still gives me the opportunity to update to the latest ESR version.

@Thorin-Oakenpants
Copy link
Contributor

@earthlng bump .. can you work this out (follow the code man!) as in when it gets updated ... I don't mean that we remove it, but maybe make it inactive if it doesn't call home any more than when you update/install

@Atavic it's not about "thwarting" it, it's about does it call home (which is not a risk, but useful for those who want zero outbound) in the background (i.e not when updating/installing)

@earthlng
Copy link
Contributor

it's a bit tough to follow but as far as I can tell it's only used on install, update and the daily background update checks. (thanks @claustromaniac for the breakdown and all the links)
There's also something in the new discovery thingy code that uses it but I don't think that's a concern because we disable the "Get Addons" page anyway and that discovery thing also needs telemetry enabled.
Since we don't disable extension updates by default, I think it's fine to make this inactive as well. Or just use the proposed change from OP, IDC

@Thorin-Oakenpants
Copy link
Contributor

I think it's fine to make this inactive as well. Or just use the proposed change from OP

Already did 1c09ec3 .. but I think inactive is a good choice as well .. will do some rewording and a commit later

@Thorin-Oakenpants
Copy link
Contributor

it's a bit tough to follow but as far as I can tell it's only used on install, update and the daily background update checks

OK, flipping this to inactive. We check auto-CHECK for updates, so this does nothing except limit information.

@Thorin-Oakenpants
Copy link
Contributor

FWIW, with the pref reset, I just checked for extension updates (there were none), and nothing changed. I used uBO, since that is OP's exaample, but of course my uBO is already up to date, so we'll see what happens down the track , i.e when an update is available then it should start building the cache?

@Thorin-Oakenpants
Copy link
Contributor

the extra metadata displayed only ends up in your cache when you actually update the extension. I just had CanvasBlocker with a single line (remember, I checked for updates a few hours ago), checked for updates, CB needed one, so I did it, and now CB has a mountain of info displayed

So when "update checking" can be crossed off the list.

Thorin-Oakenpants added a commit that referenced this issue Feb 4, 2019
see #615 (comment) - checking for updates is not a trigger, having an update **and** applying it is
@earthlng
Copy link
Contributor

earthlng commented Feb 4, 2019

it's only used on install, update and the daily background update checks.

... the daily background update checks!

@earthlng
Copy link
Contributor

earthlng commented Feb 4, 2019

hashtag blackwordsmatter :)

@Thorin-Oakenpants
Copy link
Contributor

I was going to check in the next 20+ hours to see if any of the other extensions I have update their meta data then add that in. I'll re-open as a reminder.

IF you're sure, then just do a commit and close

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Feb 4, 2019

I wonder what URL it uses, because we block extensions.webservice.discoverURL - probably doesn't use this one, but the same one as checking for updates, IDK .. guess I'll find out

@earthlng
Copy link
Contributor

earthlng commented Feb 4, 2019

probably extensions.update.background.url

@Thorin-Oakenpants
Copy link
Contributor

OK .. practically 24hrs after resetting the pref, the addons got their metadata

PatrickMcKenzier pushed a commit to PatrickMcKenzier/user.js that referenced this issue Oct 10, 2022
see arkenfox/user.js#615 (comment) - checking for updates is not a trigger, having an update **and** applying it is
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants