1220735 - Stop listing invalid GMPlugins in AddonManager.getAddonsByTypes

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect)

Description

[Alessio Placitelli [:Dexter]]

In TelemetryEnvironment.jsm [0] we gather and report active GMPlugins. We noticed that |AddonManager.getAddonsByTypes| also reports invalid GMPlugins (with plugin.isValid being false).

Does it make sense to return invalid plugins in getAddonsByTypes?

[0] - https://dxr.mozilla.org/mozilla-central/rev/96377bdbcdf3e444a22aeaa677da696243b00d98/toolkit/components/telemetry/TelemetryEnvironment.jsm#624

Comment 1

[Alessio Placitelli [:Dexter]]

spohl, what do you think about not returning invalid plugins in getAddonsByTypes?

Comment 2

[Stephen A Pohl [:spohl]]

Chris, do you know the answer to this? It looks like getAddonByID has the same issue, no?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Just so we're clear, what's being asked here is should we not report GMPAddons for which GMPAddon.isValid [1] returns false?

What does it mean for this to return false? That the install hasn't happened yet? So we'll report via telemetry this when on startup the GMPs haven't had a chance to install yet?

We have telemetry to explicitly track why the Adobe GMP fails to install (VIDEO_EME_ADOBE_INSTALL_FAILED_REASON), so I guess it's not useful to track when installation hasn't happened yet.

Or am I missing something?

[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#354

Comment 4

[Georg Fritzsche [:gfritzsche]]

So, as far as i can tell, the "isValid" check is there to handle when .version is null.
This means that that GMP is not actually installed or disabled.

Currently it shows up like an active addon/plugin, which can be confusing to users of the addon manager API (e.g. collecting lists active addons for Telemetry data).

Questions here:
* do "invalid" GMPs have to show up via getAddonsByTypes() at all? (is this needed to show them as "installing shortly" in about:addons?)
* if yes, can we at least have them .appDisabled or similar?

Comment 5

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.