366973 - crash reporter should send list of installed extensions

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[(not currently active) Ted Mielczarek]

bug 325943 talks about submitting the list of installed extensions and plugins with Talkback reports.  We should do that for airbag crash reports.  I don't know if we'd want to just submit extensions.rdf, since we'd have to parse it on the server.  We might need to write some code somewhere to spit out a text file listing extensions installed and plugins installed.

Comment 1

[Benjamin Smedberg]

I think that for the time being we should focus on extensions. We can get the plugin data we need from the DLL list in the minidump.

Comment 2

[Adam Guthrie]

Attachment: patch (WIP)

Thanks to Dave for pointing me to some code in NTT that does this. This patch is basically a copy and paste of that code.

The other option is to add this to the extension manager code. Comments appreciated.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Hey, rob, what do you think of this approach?

Comment 4

[Dave Townsend [:mossop]]

This is probably a bad way to do this, it will cause a perf hit for initialising the extensions datasource when we don't currently.

The better solution is probably to put this code into the EM where we should be able to annotate with the currently active add-on id's, but not further information without any hit.

Comment 5

[(not currently active) Ted Mielczarek]

Yeah, we definitely don't want to make this a perf hit.  If we could have the EM maintain a simple comma-delimited list of extensions when it figures that out, (Even just extension IDs would be fine with me) then we could easily grab it and stuff it into the report.

Comment 6

[Dave Townsend [:mossop]]

I'll see if I can knock something up this afternoon, should be pretty simple I think.

Comment 7

[(not currently active) Ted Mielczarek]

Mossop is working on extensions.  Plugins are actually not quite as important in my mind, since they're already listed in the modules list.  We may want to add that later, but we can do that elsewhere.

Comment 8

[Samuel Sidler (old account; do not CC)]

(In reply to comment #7)
> Mossop is working on extensions.  Plugins are actually not quite as important
> in my mind, since they're already listed in the modules list.  We may want to
> add that later, but we can do that elsewhere.

Err... not really. Plug-ins don't have versions right now, which I thought was part of this bug. That's actually really important ("Flash 9 r772 *always* crashes Firefox", etc).

Comment 9

[Dave Townsend [:mossop]]

Attachment: EM based approach

A quick dtrace tests shows the previous patch taking around 150ms. It is only in delayedStartup so that isn't the end of the world however by basing the code in the EM we can improve on this somewhat and it'll work for all toolkit apps.

In this approach we simply the list of enabled items (id and version) in a pref then on every startup add an annotation for the enabled addons and the chrome name of the theme in use.

As this stands this does still cause a little under a 1ms hit on every startup on my machine, I'm not sure how that translates to the tinderboxes. However we could also move that code directly into delayedStartup (albeit only for Firefox) and it would still work since we are just pulling values from preferences at that point. I kind of prefer keeping this contained in the EM though.

Comment 10

[(not currently active) Ted Mielczarek]

(In reply to comment #8)
> Err... not really. Plug-ins don't have versions right now, which I thought was
> part of this bug. That's actually really important ("Flash 9 r772 *always*
> crashes Firefox", etc).

Ok, can you spin that off into a separate bug then?  Mossop is making progress on the extensions portion here.

Comment 11

[Samuel Sidler (old account; do not CC)]

(In reply to comment #10)
> Ok, can you spin that off into a separate bug then?  Mossop is making progress
> on the extensions portion here.

Yeah, filed bug 412633.

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 297356 [details] [diff] [review]
EM based approach

r=me. Though I really hate to use prefs for this it is reasonable under the circumstances. I think a better long term solution would be to push the extensions.rdf and parse it on the server.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 297356 [details] [diff] [review]
EM based approach

a=beltzner for 1.9, this will be super awesome, I bet

Comment 14

[Dave Townsend [:mossop]]

Checked in, will nudge beltzner into actually marking the flag on the patch.

Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.271; previous revision: 1.270
done

Comment 15

[Dave Townsend [:mossop]]

Initial numbers from the tinderboxes aren't showing any noticeable perf impact from this.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 297356 [details] [diff] [review]
EM based approach

oops, a=beltzner for realz

Comment 17

[timeless]

Comment on attachment 297356 [details] [diff] [review]
EM based approach

> unnofficial

:( can't people use Firefox's wonderful spell checker?

Comment 18

[Jesse Ruderman]

How can we use this data?  Bug 434752 could sure use it, for example.

Comment 19

[(not currently active) Ted Mielczarek]

It's not being collected by Socorro currently, despite being submitted by the client. That's filed as bug 412605.