Closed Bug 1453740 Opened 3 years ago Closed 3 years ago
Crash when shared window is minimized
59 bytes, text/x-review-board-request
From https://bugzilla.mozilla.org/show_bug.cgi?id=1432793#c40: Precondition: Make sure a folder is opened in background and not minimized (it will be needed later) Steps: 1. Start Firefox. 2. Login to hangouts.google.com. 3. Go to meet.google.com (you have to be logged in with mozilla ldap account, or else it will say that the page is unsupported on Firefox). 4. Click on Start a new meeting. 5. Click on Start Meeting. 6. Click on Present Now and select a Window. 7. The folder from step 1 will be selected for screenshare. 8. After the screenshare started, minimize the folder. On Ubuntu 16.04 (x64) this issue is not reproducible. Here is a crash log from macOS: https://crash-stats.mozilla.com/report/index/04613a3b-74fd-4a96-8870-f21120180412#tab-details Bug 1450658 says that Chrome always brings a window to the front when it is shared. I'm not sure if it also prevents it from being minimized. The problem is that a minimized window has a resolution of 1x1 pixels, which causes the check here  to fail.  https://searchfox.org/mozilla-central/rev/4114ad2cfcbc511705c7865a4a34741812f9a2a9/media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc#303
Comment on attachment 8967475 [details] Bug 1453740 - Allow 1x1 windows in VP8EncoderImpl::InitEncode; https://reviewboard.mozilla.org/r/236144/#review242178 What's the upstream status of the same issue? Have they fixed it the same way?
Attachment #8967475 - Flags: review?(apehrson) → review+
Their vp8 codec code is unchanged . I tested Chrome, and the behaviour is the same as what we have with this patch, the screen goes black when the window is minimized.  https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc?q=ValidSimu&sq=package:chromium&l=359
For what it's worth, we have a release assertion where they have a debug assertion, so it's possible that their codec init is failing silently, and the end result, a black screen, is the same in both cases.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/1cece7d5df17 Allow 1x1 windows in VP8EncoderImpl::InitEncode; r=pehrsons
Comment on attachment 8967475 [details] Bug 1453740 - Allow 1x1 windows in VP8EncoderImpl::InitEncode; Approval Request Comment [Feature/Bug causing the regression]: I think this has been present since screensharing was implemented, a few months ago we switch a debug assert to a release assert and have since caught problems with screensharing. [User impact if declined]: Crashes if the shared window is minimized. This also fixes a crash if the shared window is maximized on OS X (Bug 1452561). [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: Has landed in Nightly, has not been fixed. [Needs manual test from QE? If yes, steps to reproduce]: Yes please. Comment 1 has the steps to reproduce. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: This only affects windows which are 1x1 pixel in size. We used to crash in that case anyway. [String changes made/needed]: None.
Attachment #8967475 - Flags: approval-mozilla-beta?
I have reproduced the issue on Nightly (2018-04-12) under Windows 10 (x64) using STR from Comment 0. The issue is fixed on latest Nightly (2018-04-16). Tests were performed on Windows 10 (x64), Ubuntu 16.04 (x64) and macOS 10.12.
I should have mentioned in my uplift request that this bug affects Google hangouts / meet. We're hoping to get Firefox support for hangouts / meet enabled for 60, so this bug also impacts that.
Comment on attachment 8967475 [details] Bug 1453740 - Allow 1x1 windows in VP8EncoderImpl::InitEncode; fix a crash when minimizing a shared window on google meet, approved for 60.0b13
Attachment #8967475 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
The issue is fixed on Firefox 60.0b13 under Windows 10 (x64), Ubuntu 16.04 (x64) and macOS 10.11.
You need to log in before you can comment on or make changes to this bug.