1229240 - Crash [@ mozilla::dom::MediaStreamTrackBinding::applyConstraints_promiseWrapper]

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox)

Assertion failure: value, at BindingUtils.h:955

bp-e9d7b3be-b6b2-4975-a5c8-58e4e2151201

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Karl Tomlinson (:karlt)]

May be a MediaStreamTrack issue, but don't know the appropriate component.

Comment 3

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

->jib to look at

Comment 4

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

From debugging: this is the source of the nullptr that leads to the assert:

/home/jesup/src/mozilla/inbound/dom/media/DOMMediaStream.cpp:

already_AddRefed<Promise>
DOMMediaStream::ApplyConstraintsToTrack(TrackID aTrackID,
                                        const MediaTrackConstraints& aConstraints,
                                        ErrorResult &aRv)
{
  return nullptr;
}

Comment 5

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

Attachment: MozReview Request: Bug 1229240 - make applyConstraints() reject instead of crash on non-gUM tracks.

Bug 1229240 - make applyConstraints() reject instead of crash on non-gUM tracks.

Comment 6

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

[Tracking Requested - why for this release]: crash

Comment 7

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

Tracked for 44 since it's a crash.

Comment 8

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

Comment on attachment 8694378 [details]
MozReview Request: Bug 1229240 - make applyConstraints() reject instead of crash on non-gUM tracks.

https://reviewboard.mozilla.org/r/26737/#review24331

Comment 9

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

This is too late to fix in 43 but I'll track it in case it becomes a bigger problem on release.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Jan-Ivar, Any reason why this never landed?

Comment 11

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

Comment on attachment 8694378 [details]
MozReview Request: Bug 1229240 - make applyConstraints() reject instead of crash on non-gUM tracks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26737/diff/1-2/

Comment 12

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

Attachment: MozReview Request: Bug 1229240 - test that applyConstraints() rejects on non-Gum track.

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

Comment 13

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

(In reply to Sylvestre Ledru [:sylvestre] from comment #10)
> Jan-Ivar, Any reason why this never landed?

Thanks for catching that. Just missing a test, otherwise good to go.

Comment 14

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

Comment on attachment 8703645 [details]
MozReview Request: Bug 1229240 - test that applyConstraints() rejects on non-Gum track.

https://reviewboard.mozilla.org/r/29421/#review26169

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75cb37ee4361
https://hg.mozilla.org/integration/mozilla-inbound/rev/54d6206da9ee

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75cb37ee4361
https://hg.mozilla.org/mozilla-central/rev/54d6206da9ee

Comment 17

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

Jib, should this be uplifted to Beta44 and Aurora45?

Comment 18

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

Comment on attachment 8694378 [details]
MozReview Request: Bug 1229240 - make applyConstraints() reject instead of crash on non-gUM tracks.

Approval Request Comment
[Feature/regressing bug #]: Bug 912342
[User impact if declined]: Crashable through js.
[Describe test coverage new/current, TreeHerder]: landed on m-c with new test. 
[Risks and why]: very low. Plugs extremely specific edge-case.
[String/UUID change made/needed]: none

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8694378 [details]
MozReview Request: Bug 1229240 - make applyConstraints() reject instead of crash on non-gUM tracks.

Fix a crash, taking it.

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fe051655f22f
https://hg.mozilla.org/releases/mozilla-aurora/rev/422d3820eba6

Comment 21

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

https://hg.mozilla.org/releases/mozilla-beta/rev/541df44447ab
https://hg.mozilla.org/releases/mozilla-beta/rev/2b96b7c71910

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/541df44447ab
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/2b96b7c71910