Summary: | Clean up user media request internal API | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||||||||||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | commit-queue, webkit-bug-importer | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Attachments: |
|
Description
Eric Carlson
2015-09-24 09:51:18 PDT
Created attachment 261884 [details]
Proposed patch.
Comment on attachment 261884 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=261884&action=review > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:128 > + m_audioDeviceUIDs.append(audioTrack->persistentId()); while you're changing this, you should capitalize the D! > Source/WebCore/Modules/mediastream/UserMediaRequest.h:96 > + String m_videoDeviceUIDAllowed; something that ends in -allowed sounds like it would be a bool. should this be m_allowedVideoDeviceUID? > Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:41 > +PassRefPtr<UserMediaPermissionRequestProxy> UserMediaPermissionRequestManagerProxy::createRequest(uint64_t userMediaID, const Vector<String>& deviceUIDsAudio, const Vector<String>& deviceUIDsVideo) what are the 's'es for? should this be audioDeviceUIDs? (In reply to comment #3) > Comment on attachment 261884 [details] > Proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=261884&action=review > > > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:128 > > + m_audioDeviceUIDs.append(audioTrack->persistentId()); > > while you're changing this, you should capitalize the D! > Fixed. > > Source/WebCore/Modules/mediastream/UserMediaRequest.h:96 > > + String m_videoDeviceUIDAllowed; > > something that ends in -allowed sounds like it would be a bool. should this > be m_allowedVideoDeviceUID? > Hmm, then it would match the allowedVideoDeviceUID() accessor too! Fixed. > > Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:41 > > +PassRefPtr<UserMediaPermissionRequestProxy> UserMediaPermissionRequestManagerProxy::createRequest(uint64_t userMediaID, const Vector<String>& deviceUIDsAudio, const Vector<String>& deviceUIDsVideo) > > what are the 's'es for? should this be audioDeviceUIDs? Fixed. Created attachment 261895 [details]
Updated patch.
Created attachment 261896 [details]
Another updated patch
Comment on attachment 261896 [details] Another updated patch Rejecting attachment 261896 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 261896, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Tim Thorton found in /Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog does not appear to be a valid reviewer according to contributors.json. /Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.webkit.org/results/209499 Created attachment 261924 [details]
YAUP
Created attachment 261935 [details]
Another try at updating the GTK build for the changes.
Created attachment 261944 [details]
One more time...
Created attachment 261959 [details]
One more time - redux
Comment on attachment 261959 [details] One more time - redux Clearing flags on attachment: 261959 Committed r190262: <http://trac.webkit.org/changeset/190262> All reviewed patches have been landed. Closing bug. |