1308605 - Regression from OverconstrainedError to InternalError on failure of applyConstraints and some getUserMedia calls

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

STR:
1. Open URL and share camera to see video playing.
2. Hit the [Fail] button.

Expected result:
- OverconstrainedError: Constraints could be not satisfied. constraint=height, line 0

Actual result:
- InternalError: Internal error. constraint=, line 0

26:02.07 INFO: Last good revision: feaaf1af1065257b9178faca8b67eed9657b4a17
26:02.07 INFO: First bad revision: 37cc0da01187534ea9f8d384c0c9c5c57eb67b56
26:02.07 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=feaaf1af1065257b9178faca8b67eed9657b4a17&tochange=37cc0da01187534ea9f8d384c0c9c5c57eb67b56

Likely caused by bug 1213517.

Comment 1

[Andrew Overholt [:overholt]]

Do you have an estimate for a fix here, Jan-Ivar? I ask because we're getting pretty close to release. Thanks.

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Taking a look now.

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Bug 1088621 partly fixed this, but I never got to land it, due blocking bugs that bounced, and to other distractions.

I'll fashion a minimal patch based on https://reviewboard.mozilla.org/r/67362/diff/4#index_header

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Bug 1308605 - Fail with OverconstrainedError on bad constraint.

Review commit: https://reviewboard.mozilla.org/r/85344/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85344/

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Here's a second fiddle showing the problem happens with getUserMedia as well, in cases where there are competing constraints from other tabs/requests with active gUM streams.

https://jsfiddle.net/kdmruyg6/

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

In the gUM case it would fail with NotReadableError, same problem in a different spot in the code.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8800439 [details]
Bug 1308605 - Fail with OverconstrainedError on bad constraint.

https://reviewboard.mozilla.org/r/85344/#review83928

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0b408d85c07
Fail with OverconstrainedError on bad constraint. r=jesup

Comment 9

[Carsten Book [:Tomcat]]

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

Comment 10

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8800439 [details]
Bug 1308605 - Fail with OverconstrainedError on bad constraint.

Approval Request Comment
[Feature/regressing bug #]: bug 1213517
[User impact if declined]:

Regression in 50 where users see unhelpful InternalError instead of OverconstrainedError with .constraint member whenever they're either:

1. Calling track.applyConstraints(someImpossibleConstraints), or

2. Calling getUserMedia(someImpossibleConstraints) while other tabs are constraining the same device in a manner that conflicts.

[Describe test coverage new/current, TreeHerder]:

Landed on m-c, and tested manually. Part of patch is from previous bug with tests run locally, see comment 3.

[Risks and why]:

Extremely low risk. Basically only changes code where we're already failing, and just fixes a wrong if-statement that switches between when to call OverconstrainedError and when to call with a generic error (InternalError in the case of applyConstraints, and NotReadableError in the cass of gUM).

The right thing to do is always fail with OverconstrainedError whenever there's a badConstraint to report.

[String/UUID change made/needed]: none

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8800439 [details]
Bug 1308605 - Fail with OverconstrainedError on bad constraint.

Fixes up code on some failure conditions, seems low risk, Aurora51+, Beta50+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7accfe73f212

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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