Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(16)

Issue 16778002: MediaStream API: Changing the device enumeration to be async (Closed)

Created:
7 years, 2 months ago by Tommy Widenflycht
Modified:
7 years, 1 month ago
CC:
blink-reviews, jeez, eae+blinkwatch, jochen+watch_chromium.org, hta - Chromium, vrk (LEFT CHROMIUM)
Visibility:
Public.

Description

MediaStream API: Changing the device enumeration to be async When we stared to implement this in chrome we discovered that sometimes device enumeration could take some time and therefore the API needed to be adjusted. This has been discussed on the W3C email list without objections but not yet reached the editors draft. The old name getSourceInfos got changed to getSources as well. BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152771

Patch Set 1 #

Patch Set 2 : Win fix #

Total comments: 10

Patch Set 3 : Fixed review comments #

Total comments: 2

Patch Set 4 : Moved files #

Total comments: 1

Patch Set 5 : Win build fix and url->origin #

Patch Set 6 : MediaStreamTrackSourcesRequest can now handle a sync result #

Patch Set 7 : merge #

Total comments: 2

Patch Set 8 : Review comment fixed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+340 lines, -137 lines) Patch
M LayoutTests/fast/mediastream/MediaStreamTrack-getSourceInfos.html View 1 2 3 4 5 1 chunk +0 lines, -49 lines 0 comments Download
M LayoutTests/fast/mediastream/MediaStreamTrack-getSourceInfos-expected.txt View 1 2 3 4 5 1 chunk +0 lines, -15 lines 0 comments Download
A + LayoutTests/fast/mediastream/MediaStreamTrack-getSources.html View 1 2 3 4 5 2 chunks +20 lines, -7 lines 0 comments Download
A + LayoutTests/fast/mediastream/MediaStreamTrack-getSources-expected.txt View 1 2 3 4 5 1 chunk +6 lines, -3 lines 0 comments Download
M Source/core/platform/mediastream/MediaStreamCenter.h View 1 2 3 4 5 6 3 chunks +2 lines, -2 lines 0 comments Download
M Source/core/platform/mediastream/MediaStreamCenter.cpp View 1 2 3 4 5 6 2 chunks +5 lines, -2 lines 0 comments Download
M Source/modules/mediastream/MediaStreamTrack.h View 2 chunks +2 lines, -1 line 0 comments Download
M Source/modules/mediastream/MediaStreamTrack.cpp View 1 2 2 chunks +6 lines, -13 lines 0 comments Download
M Source/modules/mediastream/MediaStreamTrack.idl View 1 chunk +1 line, -1 line 0 comments Download
A + Source/modules/mediastream/MediaStreamTrackSourcesCallback.h View 1 2 3 1 chunk +10 lines, -8 lines 0 comments Download
A + Source/modules/mediastream/MediaStreamTrackSourcesCallback.idl View 1 chunk +3 lines, -5 lines 0 comments Download
A Source/modules/mediastream/MediaStreamTrackSourcesRequest.h View 1 2 3 4 5 1 chunk +78 lines, -0 lines 0 comments Download
A Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp View 1 2 3 4 5 6 7 1 chunk +71 lines, -0 lines 0 comments Download
A Source/modules/mediastream/WebMediaStreamTrackSourcesRequest.cpp View 1 2 3 4 1 chunk +93 lines, -0 lines 0 comments Download
M Source/modules/modules.gypi View 1 2 3 4 5 6 3 chunks +5 lines, -0 lines 0 comments Download
M Tools/DumpRenderTree/chromium/TestRunner/src/MockWebMediaStreamCenter.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M Tools/DumpRenderTree/chromium/TestRunner/src/MockWebMediaStreamCenter.cpp View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M public/platform/WebMediaStreamCenter.h View 1 2 3 4 5 6 2 chunks +2 lines, -1 line 0 comments Download
A + public/platform/WebMediaStreamTrackSourcesRequest.h View 1 2 3 4 1 chunk +32 lines, -27 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
Tommy Widenflycht
7 years, 2 months ago (2013-06-11 14:27:41 UTC) #1
abarth-chromium
https://codereview.chromium.org/16778002/diff/4001/Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h File Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h (right): https://codereview.chromium.org/16778002/diff/4001/Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h#newcode50 Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h:50: virtual String url() = 0; Why do we need ...
7 years, 2 months ago (2013-06-11 23:50:44 UTC) #2
Tommy Widenflycht
https://codereview.chromium.org/16778002/diff/4001/Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h File Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h (right): https://codereview.chromium.org/16778002/diff/4001/Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h#newcode50 Source/core/platform/mediastream/MediaStreamTrackSourcesRequest.h:50: virtual String url() = 0; On 2013/06/11 23:50:44, abarth ...
7 years, 2 months ago (2013-06-12 12:29:34 UTC) #3
jamesr
https://codereview.chromium.org/16778002/diff/12001/Source/core/platform/chromium/support/WebMediaStreamTrackSourcesRequest.cpp File Source/core/platform/chromium/support/WebMediaStreamTrackSourcesRequest.cpp (right): https://codereview.chromium.org/16778002/diff/12001/Source/core/platform/chromium/support/WebMediaStreamTrackSourcesRequest.cpp#newcode30 Source/core/platform/chromium/support/WebMediaStreamTrackSourcesRequest.cpp:30: #include "modules/mediastream/MediaStreamTrackSourcesRequest.h" hmm, it's not good to #include modules/ ...
7 years, 2 months ago (2013-06-12 17:39:40 UTC) #4
abarth-chromium
We should just move all the code in core/platform/mediastream to modules/mediastream. Then you won't need ...
7 years, 2 months ago (2013-06-12 20:43:35 UTC) #5
Tommy Widenflycht
On 2013/06/12 20:43:35, abarth wrote: > We should just move all the code in core/platform/mediastream ...
7 years, 1 month ago (2013-06-13 11:32:39 UTC) #6
jamesr
On 2013/06/13 11:32:39, Tommy Widenflycht wrote: > On 2013/06/12 20:43:35, abarth wrote: > > We ...
7 years, 1 month ago (2013-06-13 19:59:43 UTC) #7
Tommy Widenflycht
On 2013/06/13 19:59:43, jamesr wrote: > On 2013/06/13 11:32:39, Tommy Widenflycht wrote: > > On ...
7 years, 1 month ago (2013-06-14 07:30:40 UTC) #8
jochen (gone - plz use gerrit)
On 2013/06/14 07:30:40, Tommy Widenflycht wrote: > On 2013/06/13 19:59:43, jamesr wrote: > > On ...
7 years, 1 month ago (2013-06-14 07:59:52 UTC) #9
Tommy Widenflycht
Jochen offered to help debugging my linker issues but when I prepared a patch for ...
7 years, 1 month ago (2013-06-14 12:04:08 UTC) #10
abarth-chromium
This LGTM You might have trouble in the Windows shared-library build. Would you be willing ...
7 years, 1 month ago (2013-06-14 14:17:47 UTC) #11
Tommy Widenflycht
On 2013/06/14 14:17:47, abarth wrote: > This LGTM > > You might have trouble in ...
7 years, 1 month ago (2013-06-17 11:22:53 UTC) #12
abarth-chromium
On 2013/06/17 11:22:53, Tommy Widenflycht wrote: > I have renamed it origin; does that sound ...
7 years, 1 month ago (2013-06-17 18:22:56 UTC) #13
jochen (gone - plz use gerrit)
On 2013/06/17 18:22:56, abarth wrote: > On 2013/06/17 11:22:53, Tommy Widenflycht wrote: > > I ...
7 years, 1 month ago (2013-06-17 20:13:00 UTC) #14
Tommy Widenflycht
PTAL While waiting for the gyp file fix I have modified the Request object to ...
7 years, 1 month ago (2013-06-18 09:22:39 UTC) #15
jochen (gone - plz use gerrit)
I landed my change, so this CL should be unblocked
7 years, 1 month ago (2013-06-18 21:14:15 UTC) #16
jochen (gone - plz use gerrit)
https://codereview.chromium.org/16778002/diff/42001/Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp File Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp (right): https://codereview.chromium.org/16778002/diff/42001/Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp#newcode55 Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp:55: if (m_callback && !m_scheduledEventTimer.isActive()) { is it possible that ...
7 years, 1 month ago (2013-06-19 13:09:32 UTC) #17
Tommy Widenflycht
https://codereview.chromium.org/16778002/diff/42001/Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp File Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp (right): https://codereview.chromium.org/16778002/diff/42001/Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp#newcode55 Source/modules/mediastream/MediaStreamTrackSourcesRequest.cpp:55: if (m_callback && !m_scheduledEventTimer.isActive()) { On 2013/06/19 13:09:33, jochen ...
7 years, 1 month ago (2013-06-19 13:11:59 UTC) #18
jochen (gone - plz use gerrit)
lgtm
7 years, 1 month ago (2013-06-19 15:32:11 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tommyw@chromium.org/16778002/65001
7 years, 1 month ago (2013-06-20 07:23:59 UTC) #20
commit-bot: I haz the power
7 years, 1 month ago (2013-06-20 11:16:46 UTC) #21
Message was sent while issue was closed.
Change committed as 152771

Powered by Google App Engine
This is Rietveld 408576698