1053473 - Add GMP crash submissions to the crash manager with the proper type

Status: RESOLVED INVALID

(Firefox :: General, defect)

Description

[Drew Willcoxon :adw]

According to bug 1009765 comment 36, submitted GMP crashes show up in FHR as plugin-crash-submission-{succeeded,failed}, but they should be recorded as gmplugin-crash-submission-{succeeded,failed}.

I think the problem is that gPluginHandler.submitReport always passes PROCESS_TYPE_PLUGIN to CrashSubmit (and ultimately to Services.crashmanager) here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-plugins.js?rev=e76d723927e1#508  Now that bug 1009765 landed, it should pass PROCESS_TYPE_GMPLUGIN.

submitReport is called in a few places.  One is from gPluginHandler.pluginCrashed, which is called as an nsIObserver callback for the "plugin-crashed" notification: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js?rev=8fb770b10ae8#886  It should probably also be called for the "gmp-plugin-crash" notification and pass PROCESS_TYPE_GMPLUGIN then.  gmp-plugin-crash currently passes a string along with the notification instead of a property bag, but the patch in bug 1052141 fixes that.

The other places are from gPluginHandler.pluginInstanceCrashed, which is called when the "PluginCrashed" event is fired.  In that case, I think the event has a property bag passed along with it: http://mxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp?rev=56cc07922391#1695

Comment 1

[:Gavin Sharp [email: [e-mail]]]

going to consider this for 34.3

Comment 2

[Biru [:poiru]]

(In reply to Drew Willcoxon :adw from comment #0)
> I think the problem is that gPluginHandler.submitReport always passes
> PROCESS_TYPE_PLUGIN to CrashSubmit (and ultimately to Services.crashmanager)
> here:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-
> plugins.js?rev=e76d723927e1#508  Now that bug 1009765 landed, it should pass
> PROCESS_TYPE_GMPLUGIN.

This should no longer be an issue thanks to bug 1024672 part 3.

Comment 3

[Drew Willcoxon :adw]

Nice -- I briefly glanced at the patches there, and it looks like they stop reporting submissions as a type of crash and instead look up the submissions' associated crashes and then tag them with submission info.  Which means that when we record submissions, we no longer have to worry about passing crash and process types.  Is that right?  If so, we can just close this bug after yours lands.

Comment 4

[Biru [:poiru]]

(In reply to Drew Willcoxon :adw from comment #3)
> Nice -- I briefly glanced at the patches there, and it looks like they stop
> reporting submissions as a type of crash and instead look up the
> submissions' associated crashes and then tag them with submission info. 
> Which means that when we record submissions, we no longer have to worry
> about passing crash and process types.  Is that right?  If so, we can just
> close this bug after yours lands.

Yes, that is correct. The relevant patches have already landed, so feel free to close this.

Comment 5

[Drew Willcoxon :adw]

Great, thanks Birunthan.

I'm going to resolve this invalid instead of fixed since the bug's premise doesn't make sense anymore, due to bug 1024672.

There's one related thing that may or may not still be a problem:

(In reply to Drew Willcoxon :adw from comment #0)
> submitReport is called in a few places.  One is from
> gPluginHandler.pluginCrashed, which is called as an nsIObserver callback for
> the "plugin-crashed" notification:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> js?rev=8fb770b10ae8#886  It should probably also be called for the
> "gmp-plugin-crash" notification

I'll file another bug to investigate this.