Rework addon data format for desktop FHR

RESOLVED FIXED in Firefox 29

Status

Firefox Health Report
Client: Desktop
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: rnewman, Assigned: gfritzsche)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 29
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

Comment hidden (empty)
Blocks: 925484
No longer blocks: 925484
Created attachment 8363587 [details] [diff] [review]
Rework plugin & extension data

* Plugins have a different data format now
* Both extensions and plugins get human readable and (if present) descriptions

This contains the changes needed for desktop.

https://tbpl.mozilla.org/?tree=Try&rev=8e5c2a3238a7
Attachment #8363587 - Flags: review?(benjamin)
Side note: i don't think it's worth the time to separate out the small changes for extensions. It's all small changes to the addons provider anyway.

Updated

5 years ago
Attachment #8363587 - Flags: review?(benjamin) → review?(gps)
Assignee: nobody → georg.fritzsche
Status: NEW → ASSIGNED
Summary: Rework plugin data format for desktop FHR → Rework addon data format for desktop FHR
Created attachment 8364477 [details] [diff] [review]
Rework plugin & extension data, desktop, v2

I missed updating the documentation.
Attachment #8363587 - Attachment is obsolete: true
Attachment #8363587 - Flags: review?(gps)
Attachment #8364477 - Flags: review?(gps)

Comment 5

5 years ago
Comment on attachment 8364477 [details] [diff] [review]
Rework plugin & extension data, desktop, v2

Review of attachment 8364477 [details] [diff] [review]:
-----------------------------------------------------------------

I can't comment on how proper it is to use nsIPluginHost to populate this data. But I know you know how plugins work and am willing to trust you on this. Still, we may want an extra review from bsmedberg on just those bits.

The FHR-specific stuff looks mostly fine. I have a few nitpicks, but nothing too major.

Main thread I/O via nsIFile is the only hard blocker.

::: services/healthreport/docs/dataformat.rst
@@ +534,5 @@
>  -------------------------
>  
>  This measurement contains information about the currently-installed add-ons.
>  
> +Version 2

You need to preserve the documentation for all old versions.

::: services/healthreport/providers.jsm
@@ +911,5 @@
> +    function getUserdisabled(tag) {
> +      if (tag.disabled)
> +        return true;
> +      if (tag.clicktoplay)
> +        return "askToActivate";

Nit: Always use braces: it helps avoid future bugs due to refactoring and keeps diffs more readable.

@@ +918,5 @@
> +
> +    function getModifiedDay(provider, tag) {
> +      let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
> +      file.initWithPath(tag.fullpath);
> +      return provider._dateToDays(new Date(file.lastModifiedTime));

This performs main thread I/O and may cause jank. Use OS.File.stat().

@@ +927,5 @@
> +                       getPluginTags({});
> +
> +    for (let tag of pluginTags) {
> +      data.counts["plugin"] = (data.counts["plugin"] || 0) + 1;
> +      let obj = {

For peace of mind, could you please document somewhere here that we do not store the plugin ID because it can be used for fingerprinting. I worry someone may come along in the future and want to store it. A comment should help prevent that.

@@ +936,5 @@
> +        appDisabled: tag.blocklisted,
> +        userDisabled: getUserdisabled(tag),
> +        updateDay: getModifiedDay(this, tag),
> +        mimeTypes: tag.getMimeTypes({}),
> +      };

It feels odd to me that we're grabbing non-plugin add-ons and plugins from different sources yet stuffing them in the same list.
The overlap between the two sources seems significant.

Please answer the following questions:

1) Why can't we treat plugins specially when iterating over the add-ons from the AddonsManager? i.e. what's available in nsIPluginHost that isn't reported by the AddonsManager?
2) Why don't we put add-ons and plugins in separate lists in the FHR data?

::: services/healthreport/tests/xpcshell/test_provider_addons.js
@@ +171,5 @@
> +  let addonIds = getAddonIds(value);
> +  do_check_true(addonIds.indexOf("addon0") != -1);
> +  do_check_true(addonIds.indexOf("addon1") == -1);
> +  do_check_true(addonIds.indexOf("addon2") == -1);
> +  do_check_true(addonIds.indexOf("addon3") != -1);

You seem to be using addonIds as a set. Might as well have getAddonIds return a Set.
Attachment #8364477 - Flags: review?(gps) → feedback+
(Reporter)

Updated

5 years ago
Blocks: 928574
(In reply to Gregory Szorc [:gps] from comment #5)
> @@ +918,5 @@
> > +
> > +    function getModifiedDay(provider, tag) {
> > +      let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
> > +      file.initWithPath(tag.fullpath);
> > +      return provider._dateToDays(new Date(file.lastModifiedTime));
> 
> This performs main thread I/O and may cause jank. Use OS.File.stat().

As far as i can tell this doesn't really change things here. Without this patch we hit the same problem:
http://hg.mozilla.org/mozilla-central/annotate/9d650c07b547/toolkit/mozapps/extensions/PluginProvider.jsm#l421

I'm wondering though why we don't store this as a property on the plugin tags - we need to hit the file system
anyway when scanning plugins, so i would rather store it at that point in a follow-up bug.
Does that sound reasonable?

> 1) Why can't we treat plugins specially when iterating over the add-ons from
> the AddonsManager? i.e. what's available in nsIPluginHost that isn't
> reported by the AddonsManager?

This change is per bug 922318, comment 5. AFAIR especially the grouping PluginProvider
does might lead to unexpected results and we can't get to the plugin tags directly via it's interface.
Getting the data directly from the plugin host & tags seems most reliable here.

> 2) Why don't we put add-ons and plugins in separate lists in the FHR data?

Good point, let's do that.
(In reply to Georg Fritzsche [:gfritzsche] from comment #6)
> (In reply to Gregory Szorc [:gps] from comment #5)
> > 2) Why don't we put add-ons and plugins in separate lists in the FHR data?
> 
> Good point, let's do that.

Does this seem reasonable:

"org.mozilla.addons.active": {
  "_v": 2,
  "addons": {
    // ... as before, but without plugins
  },
  "plugins": [
    // ... plugin data list (they don't have unique identifiers)
  ]
}
Created attachment 8365121 [details] [diff] [review]
Rework plugin & extension data, desktop, v3

This should cover the comments so far.
Attachment #8364477 - Attachment is obsolete: true
Attachment #8365121 - Flags: feedback?(gps)

Comment 9

5 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #6)
> (In reply to Gregory Szorc [:gps] from comment #5)
> > @@ +918,5 @@
> > > +
> > > +    function getModifiedDay(provider, tag) {
> > > +      let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
> > > +      file.initWithPath(tag.fullpath);
> > > +      return provider._dateToDays(new Date(file.lastModifiedTime));
> > 
> > This performs main thread I/O and may cause jank. Use OS.File.stat().
> 
> As far as i can tell this doesn't really change things here. Without this
> patch we hit the same problem:
> http://hg.mozilla.org/mozilla-central/annotate/9d650c07b547/toolkit/mozapps/
> extensions/PluginProvider.jsm#l421
> 
> I'm wondering though why we don't store this as a property on the plugin
> tags - we need to hit the file system
> anyway when scanning plugins, so i would rather store it at that point in a
> follow-up bug.
> Does that sound reasonable?

Capturing it as part of plugin scanning in a follow-up sounds reasonable.

The only point I was trying to make is "nsIFile usage from the main thread is considered harmful [most of the time]." Just because nsIPluginHost does synchronous I/O (I assume) doesn't give this code the excuse to do it as well. The rule is no new main thread I/O. There are few exceptions. Lots of old code is grandfathered in. I assume nsIPluginHost hasn't been converted because it's not used frequently enough to cause performance concerns?

> > 1) Why can't we treat plugins specially when iterating over the add-ons from
> > the AddonsManager? i.e. what's available in nsIPluginHost that isn't
> > reported by the AddonsManager?
> 
> This change is per bug 922318, comment 5. AFAIR especially the grouping
> PluginProvider
> does might lead to unexpected results and we can't get to the plugin tags
> directly via it's interface.
> Getting the data directly from the plugin host & tags seems most reliable
> here.

Follow-up: why aren't these things exposed to AddonsManager? They could be, right? Should they be?

I'm not opposed to using nsIPluginHost. Just trying to figure all this out.
Comment on attachment 8365121 [details] [diff] [review]
Rework plugin & extension data, desktop, v3

Review of attachment 8365121 [details] [diff] [review]:
-----------------------------------------------------------------

::: services/healthreport/providers.jsm
@@ +876,5 @@
>  
>    _createDataStructure: function (addons) {
> +    let data = {
> +      addons: {"extensions": {}, "plugins": []},
> +      counts: {}

<bikeshed> I'd put plugins as a top-level property and leave addons as-is.

@@ +916,5 @@
> +    }
> +
> +    let pluginTags = Cc["@mozilla.org/plugin/host;1"].
> +                       getService(Ci.nsIPluginHost).
> +                       getPluginTags({});

Nit: The style convention is to put the period on the following line.

@@ +929,5 @@
> +        description: tag.description,
> +        appDisabled: tag.blocklisted,
> +        userDisabled: tag.disabled || (tag.clicktoplay && "askToActivate"),
> +        updateDay: getModifiedDay(this, tag),
> +        mimeTypes: tag.getMimeTypes({}),

Now that plugins are a separate list, IMO it doesn't make sense to shoehorn this data structure into what we use for regular add-ons.

I think storing tag.blocklisted as "blocklisted" (not appDisabled) is fine. We should also consider storing tag.disabled and tag.clicktoplay separately and/or as-is. Is there a benefit to that?

Feel free to disagree: I don't have a super strong opinion on this.
Attachment #8365121 - Flags: feedback?(gps) → feedback+
(In reply to Gregory Szorc [:gps] from comment #9)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #6)
> > As far as i can tell this doesn't really change things here. Without this
> > patch we hit the same problem:
> > http://hg.mozilla.org/mozilla-central/annotate/9d650c07b547/toolkit/mozapps/
> > extensions/PluginProvider.jsm#l421
> > 
> > I'm wondering though why we don't store this as a property on the plugin
> > tags - we need to hit the file system
> > anyway when scanning plugins, so i would rather store it at that point in a
> > follow-up bug.
> > Does that sound reasonable?
> 
> Capturing it as part of plugin scanning in a follow-up sounds reasonable.
> 
> The only point I was trying to make is "nsIFile usage from the main thread
> is considered harmful [most of the time]." Just because nsIPluginHost does
> synchronous I/O (I assume) doesn't give this code the excuse to do it as
> well. The rule is no new main thread I/O. There are few exceptions. Lots of
> old code is grandfathered in. I assume nsIPluginHost hasn't been converted
> because it's not used frequently enough to cause performance concerns?

So, i was just saying that this basically hit the same statements in the linked addon manager parts before and hence not adding new main thread IO. That part just didn't stand out enough before i guess?
I think i can add this on the plugin tag and allow both the addon manager and FHR to use it.

> Follow-up: why aren't these things exposed to AddonsManager? They could be,
> right? Should they be?
> 
> I'm not opposed to using nsIPluginHost. Just trying to figure all this out.

I'm not sure if the plugin tags should be exposed if the addon manager is trying to wrap/abstract from them.
We can & should fix e.g. the grouping in the addon manager and it's caching of data (if that wasn't fixed yet), but that won't happen right away.

On the other hand we can just get the data directly from nsIPluginHost without these concerns and the plugin data differs sufficiently to handle it separately anyway.
In general, if you have the opportunity to replace synchronous main thread I/O with non-blocking I/O, do it. The main reason we still have main thread I/O is that we don't have the resources to fix all the legacy offenders.

Comment 13

5 years ago
Since we have to get the plugin data from the plugin host, we don't have that opportunity here. There are several other bugs about making that asynchronous, tied up in web-compat concerns.
(In reply to Gregory Szorc [:gps] from comment #10)
> <bikeshed> I'd put plugins as a top-level property and leave addons as-is.

Hm, so leave "org.mozilla.addons.active" as is and add plugins as say "org.mozilla.addons.plugins"?
(Reporter)

Comment 15

5 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #7)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #6)
> > (In reply to Gregory Szorc [:gps] from comment #5)
> > > 2) Why don't we put add-ons and plugins in separate lists in the FHR data?
> > 
> > Good point, let's do that.
> 
> Does this seem reasonable:
> 
> "org.mozilla.addons.active": {
>   "_v": 2,
>   "addons": {
>     // ... as before, but without plugins
>   },
>   "plugins": [
>     // ... plugin data list (they don't have unique identifiers)
>   ]
> }

Addons are part of the v3 environment. Speaking solely to that format, not desktop's legacy v2 format: 

* We're going to need to bump the environment version regardless, because the contents of plugin records will change, as will the list of add-ons.

* The plugin list must be consistently sorted (or we must consistently sort it), because these values *must* consistently hash to the same value. That means that either the list we get from storage must be ordered, or there has to be a value in each plugin record that we can use to do that.

* We already have code for handling {} here, not [], so having no consistent identifier for plugins is vaguely annoying (the identifier is how we sort add-ons).

* I would strongly favor creating org.mozilla.plugins.active, rather than nesting in o.m.addons.active. It makes environment handling much more straightforward.
We don't have a unique consistent identifier for plugins like extension ids and we can have multiple versions of the same plugin installed.
Maybe name+description+version would work. Do we have to do this on the clients though?

> * I would strongly favor creating org.mozilla.plugins.active, rather than
> nesting in o.m.addons.active. It makes environment handling much more
> straightforward.

gps, does that also sound good to you?
(Reporter)

Comment 17

5 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #16)
> We don't have a unique consistent identifier for plugins like extension ids
> and we can have multiple versions of the same plugin installed.
> Maybe name+description+version would work. Do we have to do this on the
> clients though?

The goal is that two identical phones, with identical versions of Fennec installed, and identical plugins, will have an identical environment hash. That work must be done on the client.

That implies that they'll have the same JSON representations of each plugin, sorted in the same order.

A reasonable solution IMO is to hash the plugin, or (as a stand-in) the stable part of the plugin descriptor, to compute a stable ID for the plugin.

(Perhaps that could be part of the plugin record itself, so FHR doesn't have to care? It seems that the current plugin ID isn't really useful.)
(In reply to Richard Newman [:rnewman] from comment #17)
> A reasonable solution IMO is to hash the plugin

That would definitely add new I/O.

> or (as a stand-in) the
> stable part of the plugin descriptor, to compute a stable ID for the plugin.

As mentioned, that could be based on name+description+version, so we end up with something identifying a specific version of a plugin. We will submit those fields though, so that wouldn't need to be done on the client.

> (Perhaps that could be part of the plugin record itself, so FHR doesn't have to care? It seems that the current plugin ID isn't really useful.)

I think we'll only use that for FHR, so i don't see a need to add this to the core code.


> 
> (Perhaps that could be part of the plugin record itself, so FHR doesn't have
> to care? It seems that the current plugin ID isn't really useful.)

Comment 19

5 years ago
> The goal is that two identical phones, with identical versions of Fennec installed, and identical plugins,
> will have an identical environment hash.

What is important about this goal? I think we should just use the plugin name/description/filename/version or somesuch. We certainly don't want to hash the actual plugin .dll: I/O costs are high and value seems low.
(Reporter)

Comment 20

5 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #19)
> > The goal is that two identical phones, with identical versions of Fennec installed, and identical plugins,
> > will have an identical environment hash.
> 
> What is important about this goal?

It's how the v3 document format works. Same environment => same environment hash.


> We certainly don't want to
> hash the actual plugin .dll: I/O costs are high and value seems low.

Fine by me.


> As mentioned, that could be based on name+description+version, so we end up
> with something identifying a specific version of a plugin. We will submit
> those fields though, so that wouldn't need to be done on the client.

Plugins are part of the environment; changing plugins will affect crashes and performance, for example. That means this needs to be done on the client.


> > (Perhaps that could be part of the plugin record itself, so FHR doesn't have to care? It seems that the current plugin ID isn't really useful.)
> 
> I think we'll only use that for FHR, so i don't see a need to add this to
> the core code.

Just depends on whether you want two different implementations of the identifier computation (one desktop, one Android). No big deal.
(In reply to Richard Newman [:rnewman] from comment #20)
> (In reply to Benjamin Smedberg  [:bsmedberg] from comment #19)
> > > The goal is that two identical phones, with identical versions of Fennec installed, and identical plugins,
> > > will have an identical environment hash.
> > 
> > What is important about this goal?
> 
> It's how the v3 document format works. Same environment => same environment
> hash.

Can we change it (with version bump or not) to not require hashs for plugins?
Can we just punt the v3/stable ordering bits until desktop actually uses v3? Although, I think I told Metrics that may not be far off, so maybe that's not such a good idea :(
(Reporter)

Comment 23

5 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #21)

> Can we change it (with version bump or not) to not require hashs for plugins?

Either plugins are included in the environment, and are thus longitudinal dividers for events, or they're not included in the environment.

The former is obviously preferred, but requires that plugins have a consistent ordering and can be somehow hashed to a consistent value in order to contribute to the environment's hash.

See <http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/background/healthreport/EnvironmentV1.java#209> for how this would work.

The latter involves you building out a new type of measurement (daily) for FHR on Android, and renders any kind of plugin-related observation suspect -- environments would no longer be accurate buckets for some kinds of events.

It seems straightforward to me to compute an identifier for a plugin (filename + name + description + version), and contribute all of the fields of each plugin to the environment, just as we do now (by virtue of their presence in the add-ons list). We have all of the fields. So why the pushback?
I was wondering what we need it for and if we need it to pushed along-side possibly the same fields it is based on.
Not so much pushback as trying to understand the needs here.
(Reporter)

Comment 25

5 years ago
(In reply to Georg Fritzsche [:gfritzsche] from comment #24)
> I was wondering what we need it for and if we need it to pushed along-side
> possibly the same fields it is based on.
> Not so much pushback as trying to understand the needs here.

OK, great. So to reiterate my proposal:

  // These two continue to live in the v3 environment, or
  // the current place in the v2 payload.
  org.mozilla.addons.active: {
    _v: 2,                        // Bumped because no longer includes plugins.
    ...
  },
  org.mozilla.plugins.active: {
    _v: 1,
    "$computedid1": {        // Computed from stable textual plugin fields.
      description: "...",
      appDisabled: ...,
      ...
    },
    "$computedid2": {
      ...
    }
  }

  ...

  // This is unchanged.
  org.mozilla.addons.counts {
    ...
  }
(Reporter)

Comment 26

5 years ago
And, for plugins that decline to provide textual details, I suggest using the opaque local plugin ID, or something else unique. Multiple anonymous plugins will collide, and we need to avoid that.
Ok,(In reply to Richard Newman [:rnewman] from comment #25)
> OK, great. So to reiterate my proposal:
[...]
>   // This is unchanged.
>   org.mozilla.addons.counts {
>     ...
>   }

Should we still track plugins in this?

> And, for plugins that decline to provide textual details, I suggest using
> the opaque local plugin ID, or something else unique. Multiple anonymous
> plugins will collide, and we need to avoid that.

We don't something like a local plugin id (besides the path, but that is potentially sensitive data).
I'm not sure if we even support for a plugin to provide empty name & description. If we do and there are actually any around i haven't seen it yet.

Comment 28

5 years ago
Plugins have to have a name and a filename. They don't have to have a description or version but in those cases we just treat it as the empty string. So I don't think we need to generate any kind of ID, just use "<filename>:<name>:<version>:<description>"
(Reporter)

Comment 29

5 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #28)
> Plugins have to have a name and a filename.

Great!

(In reply to Georg Fritzsche [:gfritzsche] from comment #27)
> Should we still track plugins in this?

I think so. The counts are broken out by type anyway.
Created attachment 8366812 [details] [diff] [review]
Rework plugin & extension data, desktop, v4

Speculatively putting this up assuming that we'll end up with something roughly like the previous comments.

* Moving plugin data reporting out of org.mozilla.addons.active means adding a new provider AFAICT.
* I'm holding back on documentation changes until we have settled for something.
* Counting & reporting plugins in 2 different providers feels weird, but i don't really mind either way.
Attachment #8365121 - Attachment is obsolete: true
Attachment #8366812 - Flags: review?(gps)
Created attachment 8367505 [details] [diff] [review]
Rework plugin & extension data, desktop, v5

The data format is settled now, so here is the patch including documentation changes.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=931bdacdf1cf

Notes still applying:
* Moving plugin data reporting out of org.mozilla.addons.active means adding a new provider AFAICT.
* Counting & reporting plugins in 2 different providers feels weird, but i don't really mind either way.
Attachment #8366812 - Attachment is obsolete: true
Attachment #8366812 - Flags: review?(gps)
Attachment #8367505 - Flags: review?(gps)
Comment on attachment 8367505 [details] [diff] [review]
Rework plugin & extension data, desktop, v5

Review of attachment 8367505 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh. I'm not a fan of the multiple providers, especially since there's a lot of copied or at least very similar code in here.

Rule of thumb: if you copy more than a line of non-boilerplate code, chances are you should be sharing that code instead.

Options:

1) Refactor the shared bits of the provider into a base type
2) Move the plugins code back into the AddonsProvider as a separate measurement
3) Leave the measurements combined
4) Create a new, refactored and unified provider with both addons and plugins data

I like #2 or #4.

I would strongly prefer we address that before landing. I hate "lowering the bar" to get this in before 29. But I'm prepared to relegate it to a follow-up. But that limits us to #1 unless we want to introduce yet another measurement value for Metrics to deal with. I think it makes sense to minimize measurement churn and do the refactoring now.

::: services/healthreport/docs/dataformat.rst
@@ +660,5 @@
>  
> +org.mozilla.plugins.active
> +-------------------------
> +
> +This measurement contains information about the currently-installed add-ons.

plugins.

@@ +683,5 @@
> +* mimeTypes
> +* updateDay
> +
> +With the exception of *updateDay* and *mimeTypes*, all these properties come
> +directly from nsIPluginTag via nsIPluginHost.

Surround the interface names with `` `` to get <code> treatment.

::: services/healthreport/providers.jsm
@@ +910,5 @@
>  
> +    let pluginTags = Cc["@mozilla.org/plugin/host;1"]
> +                       .getService(Ci.nsIPluginHost)
> +                       .getPluginTags({});
> +    data.counts["plugin"] = (data.counts["plugin"] || 0) + pluginTags.length;

data.counts.plugin should always be undefined, right? Why not assign directly?

@@ +1048,5 @@
> +
> +    function getModifiedDay(provider, tag) {
> +      let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
> +      file.initWithPath(tag.fullpath);
> +      return provider._dateToDays(new Date(file.lastModifiedTime));

No new synchronous I/O!

::: services/healthreport/tests/xpcshell/test_provider_addons.js
@@ +155,5 @@
>  
>    value = data.days.getDay(now);
>    do_check_eq(value.size, 4);
>    do_check_eq(value.get("extension"), 1);
> +  do_check_eq(value.get("plugin"), 2);

Why did this increase?
Attachment #8367505 - Flags: review?(gps) → feedback+
(In reply to Gregory Szorc [:gps] from comment #32)
> @@ +1048,5 @@
> > +
> > +    function getModifiedDay(provider, tag) {
> > +      let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
> > +      file.initWithPath(tag.fullpath);
> > +      return provider._dateToDays(new Date(file.lastModifiedTime));
> 
> No new synchronous I/O!

I thought per comment 9 et al we are ok with a follow-up? That is bug 964272.

> 
> ::: services/healthreport/tests/xpcshell/test_provider_addons.js
> @@ +155,5 @@
> >  
> >    value = data.days.getDay(now);
> >    do_check_eq(value.size, 4);
> >    do_check_eq(value.get("extension"), 1);
> > +  do_check_eq(value.get("plugin"), 2);
> 
> Why did this increase?

Because we are counting our 2 test plugins now, not the plugin entry that the test provides.
Created attachment 8367609 [details] [diff] [review]
Rework plugin & extension data, desktop, v6

Merged the providers back again per IRC discussion, we now have org.mozilla.addons.addons and .plugins
Attachment #8367505 - Attachment is obsolete: true
Attachment #8367609 - Flags: review?(gps)
Attachment #8367609 - Flags: feedback?(rnewman)
Comment on attachment 8367609 [details] [diff] [review]
Rework plugin & extension data, desktop, v6

Review of attachment 8367609 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!
Attachment #8367609 - Flags: review?(gps) → review+
Attachment #8367609 - Flags: feedback?(rnewman)
https://hg.mozilla.org/mozilla-central/rev/c78f9d502f0f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29

Comment 38

4 years ago
On about:healthreport I'm now seeing

Add-ons
enabled 0
disabled 0

Plugins
enabled 0
disabled 0

fallout from this bug?

Updated

4 years ago
Depends on: 966871

Comment 39

4 years ago
Filed bug 966871 on comment 38: that's a server-side change so it doesn't need to block the trains.

Updated

4 years ago
Depends on: 971420

Updated

4 years ago
No longer depends on: 971420

Updated

4 years ago
Blocks: 1055102
You need to log in before you can comment on or make changes to this bug.