1037389 - add support for deviceId in gUM constraints

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: sourceId.patch

This is my best reading of the spec, wouldn't be surprised if I got something wrong though.

Comment 1

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

Comment on attachment 8454389 [details] [diff] [review]
sourceId.patch

Review of attachment 8454389 [details] [diff] [review]:
-----------------------------------------------------------------

Hmmm. IIRC, SourceID's are supposed to be unique strings such as guids, which are used for selecting a source.   What's the source of the SourceID?  In this case, we want it to be unique-per-run (not persistent across runs).

reassigning review to jib, who has tracked this more closely

Comment 2

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

Comment on attachment 8454389 [details] [diff] [review]
sourceId.patch

(In reply to Randell Jesup [:jesup] from comment #1)
> Hmmm. IIRC, SourceID's are supposed to be unique strings such as guids,
> which are used for selecting a source.   What's the source of the SourceID? 
> In this case, we want it to be unique-per-run (not persistent across runs).

Actually a bit trickier.

From http://dev.w3.org/2011/webrtc/editor/getusermedia.html#iana-registrations :

 "The application-unique identifier for this source. The same identifier MUST be
  valid between sessions of this application, but MUST also be different for
  other applications. Some sort of GUID is recommended for the identifier."

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Jan-Ivar Bruaroey [:jib] from comment #2)
> Comment on attachment 8454389 [details] [diff] [review]
> sourceId.patch
> 
> (In reply to Randell Jesup [:jesup] from comment #1)
> > Hmmm. IIRC, SourceID's are supposed to be unique strings such as guids,
> > which are used for selecting a source.   What's the source of the SourceID? 
> > In this case, we want it to be unique-per-run (not persistent across runs).
> 
> Actually a bit trickier.
> 
> From
> http://dev.w3.org/2011/webrtc/editor/getusermedia.html#iana-registrations :
> 
>  "The application-unique identifier for this source. The same identifier
> MUST be
>   valid between sessions of this application, but MUST also be different for
>   other applications. Some sort of GUID is recommended for the identifier."

As this patch stands, passing "window" as the sourceId will result in the tab picker popping up. I plan to follow up with a patch to allow you to pass window:<window id> which will skip that dialog for privledged callers. If that's going down the wrong path, let me know.

Comment 4

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

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #3)
> If that's going down the wrong path, let me know.

I think so. If all we need is a boolean we should perhaps not overload sourceId with this as it might confuse people who are waiting for the actual sourceId behavior. We could make our own proprietary key and/or map out a proposal for standardization, unless there's a rationale behind doing this overload I'm missing. Also, why do we need the boolean? What happens differently for true and false?

Comment 5

[Martin Thomson [:mt:]]

Yeah, this is going to be difficult to get right.  For tab sharing, it is probably best if we have a separate constraint for tab/window selection.  For those, source identifier probably just needs to be something fixed but non-colliding, like "tab".

BTW, using GUID is probably not appropriate; we'll probably want to have a per-origin secret and use HMAC(per-origin-secret, device-identifier), with the secret sharing fate with cookies so that we regenerate as needed.  That likely means using indexedDB or something horrific like that.

Comment 6

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

Attachment: Part 1: deviceId prep + move facingMode constraint

- Move device-level constraints (facingMode) down w/others (so advanced works right)
- Add deviceId WebIDL
- Cleanup id code a little.

Comment 7

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

Comment on attachment 8602784 [details] [diff] [review]
Part 1: deviceId prep + move facingMode constraint

Moved patch to Bug 1173255.

Comment 9

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

Attachment: MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

Bug 1037389 - move input validation to sync step of getUserMedia

Comment 10

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

Attachment: MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

Bug 1037389 - getUserMedia calls enumerateDevices for device ids

Comment 11

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

Attachment: MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

Bug 1037389 - factor applyConstraints out of enumeration

Comment 12

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

Attachment: MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup

Bug 1037389 - make applyConstraints work again

Comment 13

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

Attachment: MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

Bug 1037389 - make GetUserMediaDevices work again

Comment 14

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

Attachment: MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Bug 1037389 - add support for deviceId in gUM constraints

Comment 15

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

Notes for jesup (smaug, for webidl change in last patch, see URL for relevant spec)

/r/12031 - Bug 1037389 - move input validation to sync step of getUserMedia

  - Spec change cleanup: https://github.com/w3c/mediacapture-main/issues/174
    Goal is to return already-rejected promises for everything that can be
    validated synchronously.

  - Removed mostly redundant GetUserMediaTask constructor

/r/12033 - Bug 1037389 - getUserMedia calls enumerateDevices for device ids

  - Streamline order in GetUserMedia() function further, for more "sync" errors.

  - note: EnumerateDevicesImpl still takes constraints in this version (gone later).

/r/12035 - Bug 1037389 - factor applyConstraints out of enumeration

  - Take constraints out of EnumerateDevicesImpl. i.e. split GetSources()
    into GetSources() and ApplyConstraints() (internal functions)
  - Remove SelectDevice() codepath, a separate mostly redundant codepath
    for applying constraints used only by privileged code.
  - GetUserMediaDevices (used by doorhangers) is gutted in this version.

/r/12037 - Bug 1037389 - make applyConstraints work again

  - Make ApplyConstraints internal function take a passed-in device list
    which is a combined audio/video list, so split it up into audio and video.

/r/12039 - Bug 1037389 - make GetUserMediaDevices work again

  - To avoid changing the doorhanger interface, GetUserMediaDevices
    still returns the device list, but no longer enumerates itself.
    Instead it returns the one already obtained by a recent getUserMedia
    request with the same inner window id. For a device list, it shouldn't
    matter which one (if there are multiple gUM requests), as long as they
    have the same origin, which should be guaranteed by the fact that
    onNavigation tears all requests down. Frames have their own window-id's I hope?

/r/12041 - Bug 1037389 - add support for deviceId in gUM constraints

  - As I'm compiling these notes, I'm realizing I added deviceId based on how
    facingMode used to work (they're both device-level constraints) which is
    actually wrong. E.g. { deviceId: "unknownid" } will fail now when it
    shouldn't. { deviceId: { exact: "unknownid" } } is needed to fail.

    I plan to address this in a follow-up patch before landing this that will
    push deviceId handling down to where facingMode is handled today (in case
    someone uses it in advanced). Expect some refactoring needed to cover audio.

Comment 16

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

Test fiddle: http://jsfiddle.net/s552r8ez

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

So MediaTrackConstraintSet will have deviceId even in the spec?

Comment 18

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

Yes, here's a better link.

Comment 19

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

Comment on attachment 8626251 [details]
MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

Bug 1037389 - move input validation to sync step of getUserMedia

Comment 20

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

Comment on attachment 8626252 [details]
MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

Bug 1037389 - getUserMedia calls enumerateDevices for device ids

Comment 21

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

Comment on attachment 8626253 [details]
MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

Bug 1037389 - factor applyConstraints out of enumeration

Comment 22

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

Comment on attachment 8626254 [details]
MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup

Bug 1037389 - make applyConstraints work again

Comment 23

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

Comment on attachment 8626255 [details]
MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

Bug 1037389 - make GetUserMediaDevices work again

Comment 24

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

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Bug 1037389 - add support for deviceId in gUM constraints

Comment 25

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

Attachment: MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work

Comment 26

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

Attachment: MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

move constraint code to helper base class to share with audio

Comment 27

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

That should be the last patch. Works for me. Test script: comment 16.

Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=37e6717c3063

Comment 28

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

Attachment: MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

Bug 1037389 - unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr

Comment 29

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

Last patch should fix try issues.

New try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ccd1df8d83f

Comment 30

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

Comment on attachment 8626251 [details]
MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

https://reviewboard.mozilla.org/r/12031/#review10761

::: dom/media/MediaManager.cpp:1337
(Diff revision 2)
> +    // Initialize MediaPermissionManager before send out any permission request.

send -> sending

Comment 31

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

Comment on attachment 8626251 [details]
MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

Bug 1037389 - move input validation to sync step of getUserMedia

Comment 32

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

Comment on attachment 8626252 [details]
MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

Bug 1037389 - getUserMedia calls enumerateDevices for device ids

Comment 33

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

Comment on attachment 8626253 [details]
MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

Bug 1037389 - factor applyConstraints out of enumeration

Comment 34

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

Comment on attachment 8626254 [details]
MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup

Bug 1037389 - make applyConstraints work again

Comment 35

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

Comment on attachment 8626255 [details]
MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

Bug 1037389 - make GetUserMediaDevices work again

Comment 36

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

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Bug 1037389 - add support for deviceId in gUM constraints

Comment 37

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

Comment on attachment 8626832 [details]
MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work

Comment 38

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

Comment on attachment 8626833 [details]
MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

move constraint code to helper base class to share with audio

Comment 39

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

Comment on attachment 8626942 [details]
MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

Bug 1037389 - unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr

Comment 40

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

Attachment: MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup

Bug 1037389 - centralize some constraints code in a MediaEngine helper

Comment 41

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

Attachment: MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

Bug 1037389 - set fake streams using pref in tree
* * *
Bug 1037389 - more encompassing fake-stream enabling in tree using pref

Comment 42

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Didn't I review this already?

Comment 43

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

Yes, sorry, not sure why Review Board thinks there are changes where there are none.

Once I stop using mq I hope review board will be a nicer experience.

Comment 44

[Olli Pettay [:smaug][[e-mail]]]

oh, maybe it is because I just r+ in bugzilla, and mozreview might not know about it.

Comment 45

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

I'm sure that's it. I think Review board only emits to Bugzilla, not the other way.

Comment 46

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

Comment on attachment 8626251 [details]
MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

Bug 1037389 - move input validation to sync step of getUserMedia

Comment 47

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

Comment on attachment 8626252 [details]
MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

Bug 1037389 - getUserMedia calls enumerateDevices for device ids

Comment 48

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

Comment on attachment 8626253 [details]
MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

Bug 1037389 - factor applyConstraints out of enumeration

Comment 49

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

Comment on attachment 8626254 [details]
MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup

Bug 1037389 - make applyConstraints work again

Comment 50

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

Comment on attachment 8626255 [details]
MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

Bug 1037389 - make GetUserMediaDevices work again

Comment 51

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

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Bug 1037389 - add support for deviceId in gUM constraints

Comment 52

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

Comment on attachment 8626832 [details]
MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work

Comment 53

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

Comment on attachment 8626833 [details]
MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

move constraint code to helper base class to share with audio

Comment 54

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

Comment on attachment 8626942 [details]
MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

Bug 1037389 - unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr

Comment 55

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

Comment on attachment 8628233 [details]
MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup

Bug 1037389 - centralize some constraints code in a MediaEngine helper

Comment 56

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

Comment on attachment 8628234 [details]
MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

Bug 1037389 - set fake streams using pref in tree
* * *
Bug 1037389 - more encompassing fake-stream enabling in tree using pref

Comment 57

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

Odd, in comment 28 Review Board was smart enough to know I'd only touched one patch in the set.

I suspect the lesson (with mq at least) is never hg pull during review. Sorry for the bug spam! :(

Comment 58

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

https://reviewboard.mozilla.org/r/12031/#review10761

> send -> sending

This code just moved, but sure.

Comment 59

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

(In reply to Olli Pettay [:smaug] from comment #42)
> Didn't I review this already?

That's Bug 1175166 apparently.

Comment 60

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

Comment on attachment 8628234 [details]
MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

Bug 1037389 - set fake streams using pref in tree
* * *
Bug 1037389 - more encompassing fake-stream enabling in tree using pref

Comment 61

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

This one's actually a new webidl file to review.

Comment 62

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

All green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcf7101567cf

Comment 63

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

Comment on attachment 8626252 [details]
MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

https://reviewboard.mozilla.org/r/12033/#review10765

::: dom/media/MediaManager.cpp:1503
(Diff revision 2)
> -                           const MediaStreamConstraints& aConstraints,
> +                           const MediaStreamConstraints& aConstraintsFDGSHJ,

What does FDGSHJ mean?  Please, something a little more descriptive.  Or a comment

::: dom/media/MediaManager.cpp:1736
(Diff revision 2)
> +//!!      ApplyConstraints(devices, c);

what does this mean?  Is it in another patch?

::: dom/media/MediaManager.cpp:1637
(Diff revision 2)
> -         src == dom::MediaSourceEnum::Screen)) {
> +                 src == dom::MediaSourceEnum::Screen)) {

if those are tabs, remove.  If RB is just indicating indent change, np

Comment 64

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

Comment on attachment 8626253 [details]
MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

https://reviewboard.mozilla.org/r/12035/#review10945

::: dom/media/MediaManager.cpp:1900
(Diff revision 4)
> -  nsRefPtr<PledgeSourceSet> p = EnumerateDevicesImpl(aWindowId, aConstraints);
> +  ScopedDeletePtr<SourceSet> devices; // !!!

// !!!
?

Assuming this is nothing important (just update the comment), ship it

Comment 65

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

Comment on attachment 8626254 [details]
MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup

https://reviewboard.mozilla.org/r/12037/#review10947

Ship It!

Comment 66

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

Comment on attachment 8626255 [details]
MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

https://reviewboard.mozilla.org/r/12039/#review10949

Ship It!

::: dom/media/MediaManager.cpp:1956
(Diff revision 4)
> -  ScopedDeletePtr<SourceSet> devices; // !!!
> +  // Ignore passed-in constraints, instead locate + return already-constrained list.

Aha, so you can ignore the question on the earlier patch

Comment 67

[Martin Thomson [:mt:]]

https://reviewboard.mozilla.org/r/12035/#review10945

s/ScopedDelete/Unique

Comment 68

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

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

https://reviewboard.mozilla.org/r/12041/#review10953

Ship It!

Comment 69

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

Comment on attachment 8626832 [details]
MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

https://reviewboard.mozilla.org/r/12161/#review10957

::: dom/media/webrtc/MediaEngineDefault.h:63
(Diff revision 3)
> -    return true;
> +    return 0;

That was an oops... Good.

Comment 70

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

Comment on attachment 8626833 [details]
MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

https://reviewboard.mozilla.org/r/12163/#review10959

Ship It!

Comment 71

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

Comment on attachment 8626942 [details]
MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

https://reviewboard.mozilla.org/r/12175/#review10961

Ship It!

Comment 72

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

Comment on attachment 8628233 [details]
MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup

https://reviewboard.mozilla.org/r/12353/#review10963

Ship It!

Comment 73

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

Comment on attachment 8628234 [details]
MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

https://reviewboard.mozilla.org/r/12355/#review10965

Ship It!

Comment 74

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

Comment on attachment 8626251 [details]
MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

https://reviewboard.mozilla.org/r/12031/#review10967

Ship It!

Comment 75

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

https://reviewboard.mozilla.org/r/12033/#review10765

> what does this mean?  Is it in another patch?

Yes it is replaced two patches down.

Comment 76

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

https://reviewboard.mozilla.org/r/12035/#review10945

> // !!!
> ?

Replaced two patches down from this patch.

Both seem used in the tree, and have different static analysis attached to them (according to Bug 1142816). Lets do what you suggest in a follow-up as I've been through a gauntlet of static analysis and try bustage to get to the current all-green try, and I'd like to land this asap.

Comment 77

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

Comment on attachment 8626251 [details]
MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup

Comment 78

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

Comment on attachment 8626252 [details]
MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup

Comment 79

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

Comment on attachment 8626253 [details]
MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

Bug 1037389 - factor applyConstraints out of enumeration. r=jesup

Comment 80

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

Comment on attachment 8626254 [details]
MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup

Bug 1037389 - make applyConstraints work again. r=jesup

Comment 81

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

Comment on attachment 8626255 [details]
MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

Bug 1037389 - make GetUserMediaDevices work again. r=jesup

Comment 82

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

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

Comment 83

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

Comment on attachment 8626832 [details]
MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

Comment 84

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

Comment on attachment 8626833 [details]
MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

Comment 85

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

Comment on attachment 8626942 [details]
MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

Comment 86

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

Comment on attachment 8628233 [details]
MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup

Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup

Comment 87

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

Comment on attachment 8628234 [details]
MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

Comment 88

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

https://reviewboard.mozilla.org/r/12031/#review10987

Ship It!

Comment 89

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

https://reviewboard.mozilla.org/r/12033/#review10989

Ship It!

Comment 90

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

https://reviewboard.mozilla.org/r/12035/#review10993

Ship It!

Comment 91

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

https://reviewboard.mozilla.org/r/12037/#review10995

Ship It!

Comment 92

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

Comment on attachment 8626255 [details]
MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup

https://reviewboard.mozilla.org/r/12039/#review10997

Ship It!

Comment 93

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

Comment on attachment 8626256 [details]
MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup

https://reviewboard.mozilla.org/r/12041/#review10999

Ship It!

Comment 94

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

Comment on attachment 8626832 [details]
MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup

https://reviewboard.mozilla.org/r/12161/#review11001

Ship It!

Comment 95

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

Comment on attachment 8626833 [details]
MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup

https://reviewboard.mozilla.org/r/12163/#review11003

Ship It!

Comment 96

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

Comment on attachment 8626942 [details]
MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup

https://reviewboard.mozilla.org/r/12175/#review11005

Ship It!

Comment 97

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

Comment on attachment 8628233 [details]
MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup

https://reviewboard.mozilla.org/r/12353/#review11007

Ship It!

Comment 98

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

Comment on attachment 8628234 [details]
MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup

https://reviewboard.mozilla.org/r/12355/#review11009

Ship It!

Comment 99

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

This should be good to go. Not sure why "status" is "pending". Who's loving Review Board right now?

Comment 100

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

Attachment: add support for deviceId in gUM constraints (merge of 11 patches). r=smaug, r=jesup

Merge of all patches here, for easier landing.

Comment 101

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a8484884b6b

Comment 102

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0a8484884b6b

Comment 103

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This broke --disable-webrtc.

Comment 104

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

Whoops, fix in Bug 1180748.

Comment 106

[Karl Tomlinson (:karlt)]

The last versions of patches, before they were collapsed into one changeset, are at https://hg.mozilla.org/mozreview/gecko/pushloghtml?changeset=619d46dbd622314f086cff443f512df0c6e43020