Bug 149527 – Clean up user media request internal API

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Clean up user media request internal API

[Radar WebKit Bug Importer]

<rdar://problem/22839956>

[Eric Carlson]

Created attachment 261884 [details]
Proposed patch.

[Tim Horton]

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?

[Eric Carlson]

(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.

[Eric Carlson]

Created attachment 261895 [details]
Updated patch.

[Eric Carlson]

Created attachment 261896 [details]
Another updated patch

[WebKit Commit Bot]

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

[Eric Carlson]

Created attachment 261924 [details]
YAUP

[Eric Carlson]

Created attachment 261935 [details]
Another try at updating the GTK build for the changes.

[Eric Carlson]

Created attachment 261944 [details]
One more time...

[Eric Carlson]

Created attachment 261959 [details]
One more time - redux

[WebKit Commit Bot]

Comment on attachment 261959 [details]
One more time - redux

Clearing flags on attachment: 261959

Committed r190262: <http://trac.webkit.org/changeset/190262>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.