Crash when shared window is minimized

VERIFIED FIXED in Firefox 60

Status

()

defect
P2
normal
Rank:
15
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: dminor, Assigned: dminor)

Tracking

(Blocks 2 bugs)

60 Branch
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 verified, firefox61 verified)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
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 [1] to fail.

[1] https://searchfox.org/mozilla-central/rev/4114ad2cfcbc511705c7865a4a34741812f9a2a9/media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc#303
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Rank: 15
Priority: P1 → P2
Blocks: hangouts

Comment 2

a year ago
mozreview-review
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+
(Assignee)

Comment 3

a year ago
Their vp8 codec code is unchanged [1]. 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.

[1] 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
(Assignee)

Comment 4

a year ago
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.

Comment 5

a year ago
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1cece7d5df17
Allow 1x1 windows in VP8EncoderImpl::InitEncode; r=pehrsons
(Assignee)

Updated

a year ago
Status: NEW → ASSIGNED

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1cece7d5df17
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(Assignee)

Comment 7

a year ago
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?
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
(Assignee)

Comment 9

a year ago
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+
(Assignee)

Updated

a year ago
Duplicate of this bug: 1452561
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.