Closed Bug 1308605 Opened 3 years ago Closed 3 years ago

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

Categories

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

50 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: jib, Assigned: jib)

References

()

Details

(Keywords: regression)

Attachments

(1 file)

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.
Rank: 22
Priority: -- → P2
Do you have an estimate for a fix here, Jan-Ivar? I ask because we're getting pretty close to release. Thanks.
Flags: needinfo?(jib)
Taking a look now.
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
Flags: needinfo?(jib)
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/
Summary: Regression from OverconstrainedError to InternalError on applyConstraints failure → Regression from OverconstrainedError to InternalError on failure of applyConstraints and some getUserMedia calls
In the gUM case it would fail with NotReadableError, same problem in a different spot in the code.
Comment on attachment 8800439 [details]
Bug 1308605 - Fail with OverconstrainedError on bad constraint.

https://reviewboard.mozilla.org/r/85344/#review83928
Attachment #8800439 - Flags: review?(rjesup) → review+
Pushed by jbruaroey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0b408d85c07
Fail with OverconstrainedError on bad constraint. r=jesup
https://hg.mozilla.org/mozilla-central/rev/c0b408d85c07
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
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
Attachment #8800439 - Flags: approval-mozilla-beta?
Attachment #8800439 - Flags: approval-mozilla-aurora?
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+
Attachment #8800439 - Flags: approval-mozilla-beta?
Attachment #8800439 - Flags: approval-mozilla-beta+
Attachment #8800439 - Flags: approval-mozilla-aurora?
Attachment #8800439 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.