Closed Bug 1660608 Opened 1 year ago Closed 1 year ago

Plugin name missing in GMP plugin crash error message

Categories

(Core :: Plug-ins, defect)

defect

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- verified
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- verified
firefox82 --- verified

People

(Reporter: selim, Assigned: Gijs)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

When a plugin crashes, the error message should indicate which plugin has crashed but currently it doesn't.

On the screenshot attached, the error message should read "Widevine CDM yan uygulaması çöktü." (The Widevine CDM plugin has crashed.)

Translations of relevant strings look fine for Turkish, and I don't know if this bug affects other locales:
https://pontoon.mozilla.org/tr/firefox/all-resources/?string=74505
https://pontoon.mozilla.org/tr/firefox/all-resources/?string=74510

As a side note, a whitespace is missing between the two strings.

I should I let you know that the specific crash I'm experiencing is related to bug 1569456. I don't know if it would affect the error message output or not.

Searching for the strings, I noticed that the code was recently touched for Fission (bug 1505913). Can it be a regression from it?

Not sure how to crash a plugin sistematically to verify in mozregression.

We format the message with this code using report.pluginName.

report is defined as the result of calling getCrashReport

    let report = PluginManager.getCrashReport(pluginCrashID);

in getCrashReport we return an item from either the map of this.gmpCrashes or this.crashReports . In the WideVine case, this would be gmpCrashes (and I checked, and the NPAPI case is correct, cf. https://searchfox.org/mozilla-central/rev/62f6cc5d9c829bc0c6f18e25f93203a98681ac97/browser/actors/PluginParent.jsm#132,151 ).

It would appear https://searchfox.org/mozilla-central/rev/62f6cc5d9c829bc0c6f18e25f93203a98681ac97/browser/actors/PluginParent.jsm#175 misses the pluginName property. We have a test for the notification bar, but it mocks the crash object, cf. https://searchfox.org/mozilla-central/rev/62f6cc5d9c829bc0c6f18e25f93203a98681ac97/browser/base/content/test/plugins/browser_globalplugin_crashinfobar.js#19-22 , so we didn't catch this.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Summary: Plugin name missing in plugin crash error message → Plugin name missing in GMP plugin crash error message

The test abstraction meant we missed that this broke in the refactor. This
fixes the bug and makes sure the test actually tests it.

Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d93b92c06268
fix plugin crash notification bar message for GMP crashes to include plugin name, r=mconley
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Comment on attachment 9171661 [details]
Bug 1660608 - fix plugin crash notification bar message for GMP crashes to include plugin name, r?mconley

Beta/Release Uplift Approval Request

  • User impact if declined: broken messaging when EME decoders crash
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: 1. open https://bitmovin.com/demos/drm
  1. play the video
  2. in activity monitor / task manager, force-quit the plugin-container process
  3. check that the resulting notification bar mentions "WidevineCdm"
  • List of other uplifts needed: n/a
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Straightforward change to code to reinstate a missing variable, adds better automated test for it, it's early in the beta cycle.
  • String changes made/needed: None

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: The broken messaging looks very sloppy in some languages (not as bad in English because the string ends up being "The plugin has crashed")
  • User impact if declined: See above
  • Fix Landed on Version: 82
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): See above.
  • String or UUID changes made by this patch: None
Attachment #9171661 - Flags: approval-mozilla-esr78?
Attachment #9171661 - Flags: approval-mozilla-beta?
Flags: qe-verify+
See Also: → 1661043

Comment on attachment 9171661 [details]
Bug 1660608 - fix plugin crash notification bar message for GMP crashes to include plugin name, r?mconley

Approved for 81.0b2. We'll revisit the ESR request after this has baked a bit more.

Attachment #9171661 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Reproduced the initial issue using old Nightly from 22-08-2020, verified that now the name of the plugin is displayed when it crashes using Firefox 81.0b2 and Latest Nightly 82.0a1 across platforms (Windows 10 64bit, macOS 10.15.6, Ubuntu 18.04).

Comment on attachment 9171661 [details]
Bug 1660608 - fix plugin crash notification bar message for GMP crashes to include plugin name, r?mconley

Thanks for the verification. Approved for 78.3esr.

Attachment #9171661 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #10)

Reproduced the initial issue using old Nightly from 22-08-2020, verified that now the name of the plugin is displayed when it crashes using Firefox 81.0b2 and Latest Nightly 82.0a1 across platforms (Windows 10 64bit, macOS 10.15.6, Ubuntu 18.04).

Also verified as fixed using Firefox 78esr from treeherder across platforms (Windows 10 64bit, macOS 10.15.6, Ubuntu 18.04).

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Regressions: 1703582
No longer regressions: 1703582
You need to log in before you can comment on or make changes to this bug.