Closed Bug 1394024 Opened 3 years ago Closed 3 years ago
[Windows only] Firefox crashes on unplugging microphone
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.101 Safari/537.36 Steps to reproduce: 1. Make sure that: (a) Flash Player is installed. If not, please install it from here: https://get.adobe.com/flashplayer (b) The Windows system should only have one microphone attached. The bug will not reproduce if more than one mic is present. 2. Start Firefox and navigate to any Flash content like this one: https://helpx.adobe.com/content/dam/help/en/flash-player/assets/flash_tree.swf 3. Right-click to bring up the Flash Player context menu and select "Settings" 4. Click the microphone icon (4th from the left) in the "Adobe Flash Player Settings" dialog 5. Now unplug the microphone Actual results: Firefox crashes at mozilla::plugins::PluginUtilsWin::AudioNotification::OnDefaultDeviceChanged * FAIL: Firefox 55.0.2 (32-bit & 64-bit) on Win7 x64 physical machine * FAIL: Firefox 55.0.2 (32-bit & 64-bit) on Win8.1 x64 physical machine * FAIL: Firefox 55.0.2 (32-bit & 64-bit) on Win10 x64 physical machine PASS: Chrome 60 on Win7 x64 physical machine PASS: Firefox 55.0.2 on Mac 10.12 Crash reports: (1) Firefox 32-bit: https://crash-stats.mozilla.com/report/index/84ba55b7-bdaf-4616-a012-eb5d61170825#allthreads (2) Firefox 64-bit: https://crash-stats.mozilla.com/report/index/19740e0d-64ab-429c-b118-a67e81170825#allthreads Expected results: There should be no crash on unplugging the microphone (as in step #5).
It looks more related to plugin. Lets have plug-ins team to check first.
Component: Audio/Video → Plug-ins
Hey David, thisd looks like a crash associated with those mic changes we made. Can you take a look? Might be related to bug 1339259 although that bug is win7 only.
Patch is pretty obvious. I'd add to the STR in comment 0 that an internal mic counts as an available mic. If you want to test this on a machine with an internal mic then you need to disable the hardware from the Sound control panel. --- Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b31f4d9d1dbabc289d5a69aea8fce3a1098789e
Attachment #8907289 - Flags: review?(jmathies)
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0f81c3cd701a Handle null string when alerting plugin about Windows microphone change. r=jimm
I guess it might be a bit late to get this into 56. Please request uplift if you want to get this into the release candidate anyway, or set to wontfix?
Comment on attachment 8907289 [details] [diff] [review] Handle null string as name of device when no mic is available This fix is trivial. Lets try for the uplift. Approval Request Comment [Feature/Bug causing the regression]: Flash player when removing the active microphone [User impact if declined]: Browser crash every time the last mic is removed [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Its one very simple line of code [String changes made/needed]: None
Attachment #8907289 - Flags: approval-mozilla-beta?
Comment on attachment 8907289 [details] [diff] [review] Handle null string as name of device when no mic is available 56 is on m-r now.
Attachment #8907289 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Comment on attachment 8907289 [details] [diff] [review] Handle null string as name of device when no mic is available This is pretty frequent on ESR52 too. Given that it's a one-liner, we should probably take it there too.
Comment on attachment 8907289 [details] [diff] [review] Handle null string as name of device when no mic is available Moderate crash rate but since the fix looks simple let's uplift for the 56 RC and for ESR.
(In reply to David Parks (dparks) [:handyman] from comment #7) > [Needs manual test from QE? If yes, steps to reproduce]: > No Updating the qe-verify flag, per David's assesment on manual testing needs.
You need to log in before you can comment on or make changes to this bug.