1660608 - Plugin name missing in GMP plugin crash error message

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Selim Şumlu]

Attachment: Plugin crash screenshot on Netflix

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.

Comment 1

[Selim Şumlu]

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.

Comment 2

[Francesco Lodolo [:flod]]

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.

Comment 3

[:Gijs (he/him)]

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.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1660608 - fix plugin crash notification bar message for GMP crashes to include plugin name, r?mconley

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d93b92c06268
fix plugin crash notification bar message for GMP crashes to include plugin name, r=mconley

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d93b92c06268

Comment 7

[:Gijs (he/him)]

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

2. play the video
3. in activity monitor / task manager, force-quit the plugin-container process
4. 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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fbc608e25093

Comment 10

[Bogdan Maris, Desktop Test Engineering]

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 11

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/bb0fbc3f097e

Comment 13

[Bogdan Maris, Desktop Test Engineering]

(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).