[Webkit-unassigned] [Bug 180206] [MediaStream] Use CaptureDevice instead of device ID to identify devices

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=180206

--- Comment #8 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 328025
  --> https://bugs.webkit.org/attachment.cgi?id=328025
Updated patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=328025&action=review

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:158
>> +void UserMediaRequest::allow(std::optional<CaptureDevice>&& audioDevice, std::optional<CaptureDevice>&& videoDevice, String&& deviceIdentifierHashSalt)
> 
> An alternative to std::optional would be to consider that there is no CaptureDevice if CaptureDevice.persistentId is the null string or its type is none (i.e. default constructed CaptureDevice).
> We could also have added an operator bool or helper method to know whether CaptureDevice was valid or not like optional has.
> Not sure what is best, but optional is fine to me.

Good idea. I added an operator bool that checks type != Unknown.

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:162
>> +    m_allowedVideoDevice = videoDevice;
> 
> We can probably move audioDevice/videoDevice.

That doesn't work because we have to WTFMove() the device parameters to RealtimeMediaSourceCenter::createMediaStream.

>> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:219
>> +void UserMediaPermissionRequestManagerProxy::grantAccess(uint64_t userMediaID, std::optional<CaptureDevice> audioDevice, std::optional<CaptureDevice> videoDevice, const String& deviceIdentifierHashSalt)
> 
> Should be const std::optional<CaptureDevice>& probably.

Fixed.

>> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:221
>> +    UserMediaProcessManager::singleton().willCreateMediaStream(*this, audioDevice.has_value(), videoDevice.has_value());
> 
> could use !!audioDevice instead of has_value.

I did that at first but changed to has_value at some point. Reconsidering, I think you are right.

>> Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h:54
>> +    const Vector<String> audioDeviceUIDs() const;
> 
> Can we use Vector<> instead of const Vector<>?

Fixed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171201/3cbed7a3/attachment.html>