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

RESOLVED INVALID

Status

()

RESOLVED INVALID
4 years ago
4 years ago

People

(Reporter: adw, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog -

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
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
Flags: firefox-backlog+
going to consider this for 34.3

Updated

4 years ago
QA Whiteboard: [qa?]
(Reporter)

Updated

4 years ago
QA Whiteboard: [qa?] → [qa+]

Updated

4 years ago
Points: --- → 3
QA Whiteboard: [qa+]
Flags: qe-verify+
(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.
(Reporter)

Comment 3

4 years ago
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.
Flags: needinfo?(birunthan)
(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.
Flags: needinfo?(birunthan)
(Reporter)

Comment 5

4 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → INVALID
(Reporter)

Updated

4 years ago
See Also: → bug 1057484

Updated

4 years ago
Points: 3 → ---
Flags: qe-verify+
Flags: firefox-backlog-
Flags: firefox-backlog+
You need to log in before you can comment on or make changes to this bug.