Closed
Bug 1037389
Opened 10 years ago
Closed 9 years ago
add support for deviceId in gUM constraints
Categories
(Core :: WebRTC, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla42
backlog | webrtc/webaudio+ |
People
(Reporter: blassey, Assigned: jib)
References
()
Details
Attachments
(12 files, 2 obsolete files)
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
patch
|
jib
:
review+
|
Details | Diff | Splinter Review |
This is my best reading of the spec, wouldn't be surprised if I got something wrong though.
Attachment #8454389 -
Flags: review?(rjesup)
Comment 1•10 years ago
|
||
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
Attachment #8454389 -
Flags: review?(rjesup) → review?(jib)
Assignee | ||
Comment 2•10 years ago
|
||
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."
Attachment #8454389 -
Flags: review?(jib) → review-
Reporter | ||
Comment 3•10 years ago
|
||
(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.
Assignee | ||
Comment 4•10 years ago
|
||
(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?
Flags: needinfo?(martin.thomson)
Comment 5•10 years ago
|
||
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.
Flags: needinfo?(martin.thomson)
Reporter | ||
Updated•10 years ago
|
tracking-fennec: ? → -
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jib
Assignee | ||
Updated•10 years ago
|
Summary: add support for sourceId in gUM constraints → add support for deviceId in gUM constraints
Updated•10 years ago
|
Assignee | ||
Comment 6•10 years ago
|
||
- Move device-level constraints (facingMode) down w/others (so advanced works right) - Add deviceId WebIDL - Cleanup id code a little.
Attachment #8454389 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Priority: -- → P2
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8602784 [details] [diff] [review] Part 1: deviceId prep + move facingMode constraint Moved patch to Bug 1173255.
Attachment #8602784 -
Attachment is obsolete: true
Updated•9 years ago
|
backlog: --- → webRTC+
Rank: 15
Priority: P2 → P1
Assignee | ||
Comment 9•9 years ago
|
||
Bug 1037389 - move input validation to sync step of getUserMedia
Attachment #8626251 -
Flags: review?(rjesup)
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1037389 - getUserMedia calls enumerateDevices for device ids
Attachment #8626252 -
Flags: review?(rjesup)
Assignee | ||
Comment 11•9 years ago
|
||
Bug 1037389 - factor applyConstraints out of enumeration
Attachment #8626253 -
Flags: review?(rjesup)
Assignee | ||
Comment 12•9 years ago
|
||
Bug 1037389 - make applyConstraints work again
Attachment #8626254 -
Flags: review?(rjesup)
Assignee | ||
Comment 13•9 years ago
|
||
Bug 1037389 - make GetUserMediaDevices work again
Attachment #8626255 -
Flags: review?(rjesup)
Assignee | ||
Comment 14•9 years ago
|
||
Bug 1037389 - add support for deviceId in gUM constraints
Attachment #8626256 -
Flags: review?(rjesup)
Attachment #8626256 -
Flags: review?(bugs)
Assignee | ||
Comment 15•9 years ago
|
||
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.
Updated•9 years ago
|
Attachment #8626256 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 19•9 years ago
|
||
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
Assignee | ||
Comment 20•9 years ago
|
||
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
Assignee | ||
Comment 21•9 years ago
|
||
Comment on attachment 8626253 [details] MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup Bug 1037389 - factor applyConstraints out of enumeration
Assignee | ||
Comment 22•9 years ago
|
||
Comment on attachment 8626254 [details] MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup Bug 1037389 - make applyConstraints work again
Assignee | ||
Comment 23•9 years ago
|
||
Comment on attachment 8626255 [details] MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup Bug 1037389 - make GetUserMediaDevices work again
Assignee | ||
Updated•9 years ago
|
Attachment #8626256 -
Flags: review+ → review?(bugs)
Assignee | ||
Comment 24•9 years ago
|
||
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
Assignee | ||
Comment 25•9 years ago
|
||
Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work
Attachment #8626832 -
Flags: review?(rjesup)
Assignee | ||
Comment 26•9 years ago
|
||
move constraint code to helper base class to share with audio
Attachment #8626833 -
Flags: review?(rjesup)
Assignee | ||
Comment 27•9 years ago
|
||
That should be the last patch. Works for me. Test script: comment 16. Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=37e6717c3063
Updated•9 years ago
|
Attachment #8626256 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 28•9 years ago
|
||
Bug 1037389 - unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr
Attachment #8626942 -
Flags: review?(rjesup)
Assignee | ||
Comment 29•9 years ago
|
||
Last patch should fix try issues. New try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ccd1df8d83f
Comment 30•9 years ago
|
||
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
Attachment #8626251 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 31•9 years ago
|
||
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
Attachment #8626251 -
Flags: review+ → review?(rjesup)
Assignee | ||
Comment 32•9 years ago
|
||
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
Assignee | ||
Comment 33•9 years ago
|
||
Comment on attachment 8626253 [details] MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup Bug 1037389 - factor applyConstraints out of enumeration
Assignee | ||
Comment 34•9 years ago
|
||
Comment on attachment 8626254 [details] MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup Bug 1037389 - make applyConstraints work again
Assignee | ||
Comment 35•9 years ago
|
||
Comment on attachment 8626255 [details] MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup Bug 1037389 - make GetUserMediaDevices work again
Assignee | ||
Updated•9 years ago
|
Attachment #8626256 -
Flags: review+ → review?(bugs)
Assignee | ||
Comment 36•9 years ago
|
||
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
Assignee | ||
Comment 37•9 years ago
|
||
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
Assignee | ||
Comment 38•9 years ago
|
||
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
Assignee | ||
Comment 39•9 years ago
|
||
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
Assignee | ||
Comment 40•9 years ago
|
||
Bug 1037389 - centralize some constraints code in a MediaEngine helper
Attachment #8628233 -
Flags: review?(rjesup)
Assignee | ||
Comment 41•9 years ago
|
||
Bug 1037389 - set fake streams using pref in tree * * * Bug 1037389 - more encompassing fake-stream enabling in tree using pref
Attachment #8628234 -
Flags: review?(rjesup)
Comment 42•9 years ago
|
||
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?
Attachment #8626256 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 43•9 years ago
|
||
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•9 years ago
|
||
oh, maybe it is because I just r+ in bugzilla, and mozreview might not know about it.
Assignee | ||
Comment 45•9 years ago
|
||
I'm sure that's it. I think Review board only emits to Bugzilla, not the other way.
Assignee | ||
Comment 46•9 years ago
|
||
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
Assignee | ||
Comment 47•9 years ago
|
||
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
Assignee | ||
Comment 48•9 years ago
|
||
Comment on attachment 8626253 [details] MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup Bug 1037389 - factor applyConstraints out of enumeration
Assignee | ||
Comment 49•9 years ago
|
||
Comment on attachment 8626254 [details] MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup Bug 1037389 - make applyConstraints work again
Assignee | ||
Comment 50•9 years ago
|
||
Comment on attachment 8626255 [details] MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup Bug 1037389 - make GetUserMediaDevices work again
Assignee | ||
Updated•9 years ago
|
Attachment #8626256 -
Flags: review+ → review?(bugs)
Assignee | ||
Comment 51•9 years ago
|
||
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
Assignee | ||
Comment 52•9 years ago
|
||
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
Assignee | ||
Comment 53•9 years ago
|
||
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
Assignee | ||
Comment 54•9 years ago
|
||
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
Assignee | ||
Comment 55•9 years ago
|
||
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
Assignee | ||
Comment 56•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Attachment #8626256 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 57•9 years ago
|
||
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! :(
Assignee | ||
Comment 58•9 years ago
|
||
https://reviewboard.mozilla.org/r/12031/#review10761 > send -> sending This code just moved, but sure.
Assignee | ||
Comment 59•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #42) > Didn't I review this already? That's Bug 1175166 apparently.
Assignee | ||
Comment 60•9 years ago
|
||
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
Attachment #8628234 -
Flags: review?(bugs)
Assignee | ||
Comment 62•9 years ago
|
||
All green try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcf7101567cf
Updated•9 years ago
|
Attachment #8628234 -
Flags: review?(bugs) → review+
Comment 63•9 years ago
|
||
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
Attachment #8626252 -
Flags: review?(rjesup) → review+
Updated•9 years ago
|
Attachment #8626253 -
Flags: review?(rjesup) → review+
Comment 64•9 years ago
|
||
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•9 years ago
|
||
Comment on attachment 8626254 [details] MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup https://reviewboard.mozilla.org/r/12037/#review10947 Ship It!
Attachment #8626254 -
Flags: review?(rjesup) → review+
Comment 66•9 years ago
|
||
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
Attachment #8626255 -
Flags: review?(rjesup) → review+
Comment 68•9 years ago
|
||
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!
Attachment #8626256 -
Flags: review?(rjesup) → review+
Updated•9 years ago
|
Attachment #8626832 -
Flags: review?(rjesup) → review+
Comment 69•9 years ago
|
||
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•9 years ago
|
||
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!
Attachment #8626833 -
Flags: review?(rjesup) → review+
Comment 71•9 years ago
|
||
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!
Attachment #8626942 -
Flags: review?(rjesup) → review+
Comment 72•9 years ago
|
||
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!
Attachment #8628233 -
Flags: review?(rjesup) → review+
Comment 73•9 years ago
|
||
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!
Attachment #8628234 -
Flags: review?(rjesup) → review+
Comment 74•9 years ago
|
||
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!
Attachment #8626251 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 75•9 years ago
|
||
https://reviewboard.mozilla.org/r/12033/#review10765 > what does this mean? Is it in another patch? Yes it is replaced two patches down.
Assignee | ||
Comment 76•9 years ago
|
||
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.
Assignee | ||
Comment 77•9 years ago
|
||
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
Attachment #8626251 -
Attachment description: MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia → MozReview Request: Bug 1037389 - move input validation to sync step of getUserMedia. r=jesup
Attachment #8626251 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8626252 -
Attachment description: MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids → MozReview Request: Bug 1037389 - getUserMedia calls enumerateDevices for device ids. r=jesup
Attachment #8626252 -
Flags: review+
Assignee | ||
Comment 78•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Attachment #8626253 -
Attachment description: MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration → MozReview Request: Bug 1037389 - factor applyConstraints out of enumeration. r=jesup
Attachment #8626253 -
Flags: review+
Assignee | ||
Comment 79•9 years ago
|
||
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
Assignee | ||
Comment 80•9 years ago
|
||
Comment on attachment 8626254 [details] MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup Bug 1037389 - make applyConstraints work again. r=jesup
Attachment #8626254 -
Attachment description: MozReview Request: Bug 1037389 - make applyConstraints work again → MozReview Request: Bug 1037389 - make applyConstraints work again. r=jesup
Attachment #8626254 -
Flags: review+
Assignee | ||
Comment 81•9 years ago
|
||
Comment on attachment 8626255 [details] MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup Bug 1037389 - make GetUserMediaDevices work again. r=jesup
Attachment #8626255 -
Attachment description: MozReview Request: Bug 1037389 - make GetUserMediaDevices work again → MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup
Attachment #8626255 -
Flags: review+
Assignee | ||
Comment 82•9 years ago
|
||
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
Attachment #8626256 -
Attachment description: MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints → MozReview Request: Bug 1037389 - add support for deviceId in gUM constraints. r=smaug, r=jesup
Attachment #8626256 -
Flags: review+
Assignee | ||
Comment 83•9 years ago
|
||
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
Attachment #8626832 -
Attachment description: MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work → MozReview Request: Bug 1037389 - fix plain deviceId constraint the right way, to make advanced etc work. r=jesup
Attachment #8626832 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8626833 -
Attachment description: MozReview Request: move constraint code to helper base class to share with audio → MozReview Request: Bug 1037389 - move constraint code to helper base class to share with audio. r=jesup
Attachment #8626833 -
Flags: review+
Assignee | ||
Comment 84•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Attachment #8626942 -
Attachment description: MozReview Request: Bug 1037389 - unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr → MozReview Request: Bug 1037389 - fixes: unremove recording-window-ended event on DENY_ACTION + avoid capturing rawptr. r=jesup
Attachment #8626942 -
Flags: review+
Assignee | ||
Comment 85•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Attachment #8628233 -
Attachment description: MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper → MozReview Request: Bug 1037389 - centralize some constraints code in a MediaEngine helper. r=jesup
Attachment #8628233 -
Flags: review+
Assignee | ||
Comment 86•9 years ago
|
||
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
Assignee | ||
Comment 87•9 years ago
|
||
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
Attachment #8628234 -
Attachment description: MozReview Request: Bug 1037389 - set fake streams using pref in tree → MozReview Request: Bug 1037389 - set fake streams using pref in tree. r=smaug, r=jesup
Attachment #8628234 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Attachment #8626251 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8626252 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8626253 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8626254 -
Flags: review+
Assignee | ||
Comment 92•9 years ago
|
||
Comment on attachment 8626255 [details] MozReview Request: Bug 1037389 - make GetUserMediaDevices work again. r=jesup https://reviewboard.mozilla.org/r/12039/#review10997 Ship It!
Attachment #8626255 -
Flags: review+
Assignee | ||
Comment 93•9 years ago
|
||
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!
Attachment #8626256 -
Flags: review+
Assignee | ||
Comment 94•9 years ago
|
||
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!
Attachment #8626832 -
Flags: review+
Assignee | ||
Comment 95•9 years ago
|
||
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!
Attachment #8626833 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8626942 -
Flags: review+
Assignee | ||
Comment 96•9 years ago
|
||
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!
Assignee | ||
Comment 97•9 years ago
|
||
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!
Attachment #8628233 -
Flags: review+
Assignee | ||
Comment 98•9 years ago
|
||
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!
Attachment #8628234 -
Flags: review+
Assignee | ||
Comment 99•9 years ago
|
||
This should be good to go. Not sure why "status" is "pending". Who's loving Review Board right now?
Assignee | ||
Comment 100•9 years ago
|
||
Merge of all patches here, for easier landing.
Flags: needinfo?(rjesup)
Attachment #8629107 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(rjesup)
Comment 101•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a8484884b6b
Keywords: checkin-needed
Comment 102•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0a8484884b6b
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
This broke --disable-webrtc.
Comment 106•3 years ago
|
||
The last versions of patches, before they were collapsed into one changeset, are at https://hg.mozilla.org/mozreview/gecko/pushloghtml?changeset=619d46dbd622314f086cff443f512df0c6e43020
Updated•3 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•