Bug 147263 – UserMediaRequest should supply IDs of devices selected by user

• Component: WebKit2
• Status: RESOLVED FIXED

[Matthew Daiter]

In order to provide the user as much flexibility as possible when selecting his/her device of preference for capture, MediaStreams MUST have an extendable, adaptable and easily connected interface from WebKit2 to Safari.

[Radar WebKit Bug Importer]

<rdar://problem/21983345>

[Matthew Daiter]

Created attachment 257487 [details]
Patch

[Matthew Daiter]

Created attachment 257849 [details]
Patch

[Matthew Daiter]

Created attachment 257850 [details]
Patch

[Eric Carlson]

Comment on attachment 257850 [details]
Patch

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

> Source/WebCore/platform/mock/UserMediaClientMock.h:48
> +            m_request->userMediaAccessGranted(WTF::emptyString(), WTF::emptyString());

This doesn't seem right. I think you will need to modify UserMediClientMock to allow tests to specify the video and/or audio UIDs that are passed through this.

> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:158
>      if (!_request)
>          return;
>  
> -    _request->userMediaAccessGranted();
> +    _request->userMediaAccessGranted(WTF::emptyString(), WTF::emptyString());
> +    RemoveRequestFromMap(_request.get());
> +#endif

Shouldn't this method be deleted?

> Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:57
>      // FIXME(147062): Need to add in the support for Safari to pass strings given from user's decision on what piece of media to open

Is this FIXME still needed?

[Matthew Daiter]

Created attachment 257961 [details]
Patch

[Matthew Daiter]

Comment on attachment 257487 [details]
Patch

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

> Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:57
>      // FIXME(147062): Need to add in the support for Safari to pass strings given from user's decision on what piece of media to open

Do we file this as well? We need everything in this patch to solve this problem, but this problem is solved with this patch. Obsolete the bug?

[Matthew Daiter]

Created attachment 257968 [details]
Patch

[Matthew Daiter]

Created attachment 257970 [details]
Patch

[Matthew Daiter]

Created attachment 257973 [details]
Patch

[Matthew Daiter]

Created attachment 257975 [details]
Patch

[Matthew Daiter]

Created attachment 257978 [details]
Patch

[Matthew Daiter]

Created attachment 257983 [details]
Patch

[Eric Carlson]

Comment on attachment 257983 [details]
Patch

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

Looks good generally with the caveats mentioned, but I am not a WK2 reviewer so you will need someone to give the official r+.

> Source/WebCore/platform/mock/UserMediaClientMock.h:48
> +            m_request->userMediaAccessGranted(WTF::emptyString(), WTF::emptyString());

As I noted before, this doesn't seem right. I think you will need to modify UserMediClientMock to allow tests to specify the video and/or audio UIDs that are passed through this. It is fine to do this in a follow-up, but please file a bug and add a FIXME.

> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:156
> +    _request->userMediaAccessGranted(WTF::emptyString(), WTF::emptyString());

You didn't add this, but it seems strange to call userMediaAccessGranted from cancelRequest - wouldn't make more sense to call userMediaAccessDenied instead? Please add a FIXME comment so we remember to look at this.

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:70
> +    priv->request->allow(String::fromUTF8(videoDeviceUID), String::fromUTF8(audioDeviceUID));

You still need to redefine WebKitPermissionRequestIface.allow

> Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:57
>      // FIXME(147062): Need to add in the support for Safari to pass strings given from user's decision on what piece of media to open

Is this comment still relevant?

> Source/WebKit2/UIProcess/UserMediaPermissionRequestProxy.cpp:47
> -void UserMediaPermissionRequestProxy::allow()
> +void UserMediaPermissionRequestProxy::allow(const String& videoDeviceUID, const String& audioDeviceUID)
>  {
> -    m_manager.didReceiveUserMediaPermissionDecision(m_userMediaID, true);
> +    m_manager.didReceiveUserMediaPermissionDecision(m_userMediaID, true, videoDeviceUID, audioDeviceUID);
>  }
>  
>  void UserMediaPermissionRequestProxy::deny()
>  {
> -    m_manager.didReceiveUserMediaPermissionDecision(m_userMediaID, false);
> +    m_manager.didReceiveUserMediaPermissionDecision(m_userMediaID, false, emptyString(), emptyString());
>  }

Instead of passing a two empty string when the request is denied, UserMediaPermissionRequestManager should have separate methods for allow and deny now that they take different parameters. Maybe

    void userMediaPermissionRequestWasGranted(uint64_t userMediaID, const String& deviceUIDVideo, const String& deviceUIDAudio);
    void userMediaPermissionRequestWasDenied(uint64_t userMediaID);

[Matthew Daiter]

Created attachment 258097 [details]
Patch

[Matthew Daiter]

Comment on attachment 257983 [details]
Patch

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

>> Source/WebCore/platform/mock/UserMediaClientMock.h:48
>> +            m_request->userMediaAccessGranted(WTF::emptyString(), WTF::emptyString());
> 
> As I noted before, this doesn't seem right. I think you will need to modify UserMediClientMock to allow tests to specify the video and/or audio UIDs that are passed through this. It is fine to do this in a follow-up, but please file a bug and add a FIXME.

See below comment on emptyString() vs choosing first "best" device.

>> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:156
>> +    _request->userMediaAccessGranted(WTF::emptyString(), WTF::emptyString());
> 
> You didn't add this, but it seems strange to call userMediaAccessGranted from cancelRequest - wouldn't make more sense to call userMediaAccessDenied instead? Please add a FIXME comment so we remember to look at this.

Yep. But this is in allow

>> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:70
>> +    priv->request->allow(String::fromUTF8(videoDeviceUID), String::fromUTF8(audioDeviceUID));
> 
> You still need to redefine WebKitPermissionRequestIface.allow

Not really. There's an architecture problem with that, because the class inherits the allow method from a generalized PermissionRequestIface, which affects other classes. Rather do this in the class and define the method as a way to choose the "best" device

>> Source/WebKit2/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:57
>>      // FIXME(147062): Need to add in the support for Safari to pass strings given from user's decision on what piece of media to open
> 
> Is this comment still relevant?

Nope! Deleted

>> Source/WebKit2/UIProcess/UserMediaPermissionRequestProxy.cpp:47
>>  }
> 
> Instead of passing a two empty string when the request is denied, UserMediaPermissionRequestManager should have separate methods for allow and deny now that they take different parameters. Maybe
> 
>     void userMediaPermissionRequestWasGranted(uint64_t userMediaID, const String& deviceUIDVideo, const String& deviceUIDAudio);
>     void userMediaPermissionRequestWasDenied(uint64_t userMediaID);

Later fix.

[Matthew Daiter]

Created attachment 258109 [details]
Patch

[Tim Horton]

Comment on attachment 258109 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Linking device query ability from WebKit2 to Safari

There's nothing Safari specific about this patch. "to clients"?

> Source/WebCore/platform/mock/UserMediaClientMock.h:48
> +            m_request->userMediaAccessGranted(m_request->videoDeviceUIDs().at(0), m_request->audioDeviceUIDs().at(0));

Are we sure there will always be at least one in both vectors?

> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:156
> +    _request->userMediaAccessGranted(_request->videoDeviceUIDs().at(0), _request->audioDeviceUIDs().at(0));

For what reason are we keeping the version of allow that takes no parameters? Also, who calls these?

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:55
> +        const char* deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Video).utf8().data();
> +        WKArrayAppendItem(array, WKStringCreateWithUTF8CString(deviceName));

You don't need to round-trip through char* here, you can use toAPI (or something like that; look around).

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:64
> +        const char* deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Audio).utf8().data();

Ditto

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.h:30
> -WK_EXPORT void WKUserMediaPermissionRequestAllow(WKUserMediaPermissionRequestRef);
> +WK_EXPORT void WKUserMediaPermissionRequestAllowBest(WKUserMediaPermissionRequestRef);

Are you 100% sure you can change the signature of this SPI? Is it new? Is anything using it?

[Matthew Daiter]

Comment on attachment 258109 [details]
Patch

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

>> Source/WebCore/ChangeLog:3
>> +        Linking device query ability from WebKit2 to Safari
> 
> There's nothing Safari specific about this patch. "to clients"?

Fixed.

>> Source/WebCore/platform/mock/UserMediaClientMock.h:48
>> +            m_request->userMediaAccessGranted(m_request->videoDeviceUIDs().at(0), m_request->audioDeviceUIDs().at(0));
> 
> Are we sure there will always be at least one in both vectors?

Yep. There must be, or else the request at the base will throw an error.

>> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:156
>> +    _request->userMediaAccessGranted(_request->videoDeviceUIDs().at(0), _request->audioDeviceUIDs().at(0));
> 
> For what reason are we keeping the version of allow that takes no parameters? Also, who calls these?

The class inherits from another class, which has an "allow" and "deny" method. So, to make it easier on the developer and to make this class implement its superclass's methods correctly, we MUST keep this function. There's also talk of only allowing the "best" device to be selected for a certain task. This method makes making that request easy (vectors are organized so best device for selection is at 0 index).

>> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:55
>> +        WKArrayAppendItem(array, WKStringCreateWithUTF8CString(deviceName));
> 
> You don't need to round-trip through char* here, you can use toAPI (or something like that; look around).

Fixed.

>> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:64
>> +        const char* deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Audio).utf8().data();
> 
> Ditto

Ditto.

>> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.h:30
>> +WK_EXPORT void WKUserMediaPermissionRequestAllowBest(WKUserMediaPermissionRequestRef);
> 
> Are you 100% sure you can change the signature of this SPI? Is it new? Is anything using it?

Positive. And we also have another method that lets us use more specific selection if needed.

[Matthew Daiter]

Created attachment 258208 [details]
Patch

[Tim Horton]

Comment on attachment 258208 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:54
> +        WTF::String deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Video);

No WTF::. Also, why does getDeviceNameForUID have a "get" in its name? No out argument that I can see!

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:55
> +        WKArrayAppendItem(array, toAPI(&API::String::create(deviceName).leakRef()));

My confidence in the memory management situation here is not strong, but I haven't looked enough to say it's wrong either.

I really think you want a .ptr() here, though, not a leakRef.

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:64
> +        WTF::String deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Audio);

Ditto

[Matthew Daiter]

Created attachment 258281 [details]
Patch

[Matthew Daiter]

Created attachment 258282 [details]
Patch

[Matthew Daiter]

Created attachment 258283 [details]
Patch

[Matthew Daiter]

Comment on attachment 258208 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:54
>> +        WTF::String deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Video);
> 
> No WTF::. Also, why does getDeviceNameForUID have a "get" in its name? No out argument that I can see!

Fixed.

>> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:55
>> +        WKArrayAppendItem(array, toAPI(&API::String::create(deviceName).leakRef()));
> 
> My confidence in the memory management situation here is not strong, but I haven't looked enough to say it's wrong either.
> 
> I really think you want a .ptr() here, though, not a leakRef.

You had said to do it the way WKString was doing it, and that was how WKString was doing it here. However, I can change that to a ptr. Fixed.

>> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:64
>> +        WTF::String deviceName = toImpl(userMediaPermissionRef)->getDeviceNameForUID(name, WebCore::RealtimeMediaSource::Type::Audio);
> 
> Ditto

Fixed.

[Eric Carlson]

Comment on attachment 258283 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Linking device query ability from WebKit2 to clients

Please use the updated bug title.

> Source/WebCore/ChangeLog:13
> +        * platform/graphics/mac/WebGLLayer.mm:
> +        (-[WebGLLayer initWithGraphicsContext3D:]):

This wasn't changed.

> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:156
> +    _request->userMediaAccessGranted(_request->videoDeviceUIDs().at(0), _request->audioDeviceUIDs().at(0));

Will both of these vectors really have at least one element when a user chooses a device that provides only video or audio. What if the call to getUserMedia only request video or audio?

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:37
> +    toImpl(userMediaPermissionRequestRef)->allow(toImpl(userMediaPermissionRequestRef)->videoDeviceUIDs().at(0), toImpl(userMediaPermissionRequestRef)->audioDeviceUIDs().at(0));

Ditto.

[Matthew Daiter]

Created attachment 258286 [details]
Patch

[Matthew Daiter]

Created attachment 258287 [details]
Patch

[Eric Carlson]

Comment on attachment 258286 [details]
Patch

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

> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:176
> +    if (!_request->videoDeviceUIDs().isEmpty() && !_request->audioDeviceUIDs().isEmpty())
> +        _request->userMediaAccessGranted(_request->videoDeviceUIDs().at(0), _request->audioDeviceUIDs().at(0));
> +    else if (!_request->videoDeviceUIDs().isEmpty())
> +        _request->userMediaAccessGranted(_request->videoDeviceUIDs().at(0), emptyString());
> +    else if (!_request->audioDeviceUIDs().isEmpty())
> +        _request->userMediaAccessGranted(emptyString(), _request->audioDeviceUIDs().at(0));

Nit: I think this would be easier to follow with a couple of local variables:

  const String& videoUID = _request->videoDeviceUIDs().isEmpty() ? _request->videoDeviceUIDs().at(0) : emptyString()
  const String& audioUID = _request->audioDeviceUIDs().isEmpty() ? _request->audioDeviceUIDs().at(0) : emptyString()

  _request->userMediaAccessGranted(videoUID, audioUID);

> Source/WebKit2/UIProcess/API/C/WKUserMediaPermissionRequest.cpp:37
> +    toImpl(userMediaPermissionRequestRef)->allow(toImpl(userMediaPermissionRequestRef)->videoDeviceUIDs().at(0), toImpl(userMediaPermissionRequestRef)->audioDeviceUIDs().at(0));

Is this OK when one or both of the vectors is empty?

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:70
> +    priv->request->allow(priv->request->videoDeviceUIDs().at(0), priv->request->audioDeviceUIDs().at(0));

Ditto.

[Matthew Daiter]

Comment on attachment 258286 [details]
Patch

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

>> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:176
>> +        _request->userMediaAccessGranted(emptyString(), _request->audioDeviceUIDs().at(0));
> 
> Nit: I think this would be easier to follow with a couple of local variables:
> 
>   const String& videoUID = _request->videoDeviceUIDs().isEmpty() ? _request->videoDeviceUIDs().at(0) : emptyString()
>   const String& audioUID = _request->audioDeviceUIDs().isEmpty() ? _request->audioDeviceUIDs().at(0) : emptyString()
> 
>   _request->userMediaAccessGranted(videoUID, audioUID);

I think you mean:
  const String& videoUID = !_request->videoDeviceUIDs().isEmpty() ? _request->videoDeviceUIDs().at(0) : emptyString()
  const String& audioUID = !_request->audioDeviceUIDs().isEmpty() ? _request->audioDeviceUIDs().at(0) : emptyString()

  _request->userMediaAccessGranted(videoUID, audioUID);

But fixed!

[Matthew Daiter]

Created attachment 258848 [details]
Patch

[Jer Noble]

Comment on attachment 258848 [details]
Patch

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

> Source/WebKit2/ChangeLog:11
> +        * Platform/mac/LayerHostingContext.mm:
> +        (WebKit::LayerHostingContext::setColorMatchUntaggedContent):
> +        (WebKit::LayerHostingContext::colorMatchUntaggedContent):

This looks unrelated.

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:75
> +    const String& videoDevice = !priv->request->videoDeviceUIDs().isEmpty() ? priv->request->videoDeviceUIDs().at(0) : emptyString();
> +    const String& audioDevice = !priv->request->audioDeviceUIDs().isEmpty() ? priv->request->audioDeviceUIDs().at(0) : emptyString();

This snippet (`.isEmpty() : .at(0) : emptyString()`) is repeated now 6 times. Is there a better way to do this?  Can the request vend a "firstVideoDeviceUID()" that returns "emptyString()" if its array is empty?

> Tools/WebKitTestRunner/TestController.cpp:1498
> +            if (WKArrayGetSize(WKUserMediaPermissionRequestDeviceNamesVideo(request.get())) && WKArrayGetSize(WKUserMediaPermissionRequestDeviceNamesAudio(request.get())))
> +                WKUserMediaPermissionRequestAllow(request.get(), reinterpret_cast<WKStringRef>(WKArrayGetItemAtIndex(WKUserMediaPermissionRequestDeviceNamesVideo(request.get()), 0)), reinterpret_cast<WKStringRef>(WKArrayGetItemAtIndex(WKUserMediaPermissionRequestDeviceNamesAudio(request.get()), 0)));

And this seems weird; this will only allow if BOTH the video and audio lists are non-empty.  What if someone just requests a video camera with no audio capabilities, or vice versa?

[Matthew Daiter]

Comment on attachment 258848 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:75
>> +    const String& audioDevice = !priv->request->audioDeviceUIDs().isEmpty() ? priv->request->audioDeviceUIDs().at(0) : emptyString();
> 
> This snippet (`.isEmpty() : .at(0) : emptyString()`) is repeated now 6 times. Is there a better way to do this?  Can the request vend a "firstVideoDeviceUID()" that returns "emptyString()" if its array is empty?

We could, but that seems like an awfully specific function. However, that could easily be written.

>> Tools/WebKitTestRunner/TestController.cpp:1498
>> +                WKUserMediaPermissionRequestAllow(request.get(), reinterpret_cast<WKStringRef>(WKArrayGetItemAtIndex(WKUserMediaPermissionRequestDeviceNamesVideo(request.get()), 0)), reinterpret_cast<WKStringRef>(WKArrayGetItemAtIndex(WKUserMediaPermissionRequestDeviceNamesAudio(request.get()), 0)));
> 
> And this seems weird; this will only allow if BOTH the video and audio lists are non-empty.  What if someone just requests a video camera with no audio capabilities, or vice versa?

I can change this so that the system passes back an empty string if the array size is 0.

[Matthew Daiter]

Created attachment 258866 [details]
Patch

[Eric Carlson]

Comment on attachment 258866 [details]
Patch

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

> Source/WebKit2/ChangeLog:11
> +        * Platform/mac/LayerHostingContext.mm:
> +        (WebKit::LayerHostingContext::setColorMatchUntaggedContent):
> +        (WebKit::LayerHostingContext::colorMatchUntaggedContent):

Looks like you forgot to remove these.

> Source/WebCore/Modules/mediastream/UserMediaRequest.h:79
> +    const String& firstVideoDeviceUID() const { return !videoDeviceUIDs().isEmpty() ? videoDeviceUIDs().at(0) : emptyString(); }
> +    const String& firstAudioDeviceUID() const { return !audioDeviceUIDs().isEmpty() ? audioDeviceUIDs().at(0) : emptyString(); }

Nit: you can use the instance variables directly instead of calling the functions.

> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:157
> +    const String& videoUID = _request->firstVideoDeviceUID();
> +    const String& audioUID = _request->firstAudioDeviceUID();

Nit: no need for these local variables now:

_request->userMediaAccessGranted(_request->firstVideoDeviceUID(), _request->firstAudioDeviceUID());

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:77
> +    const String& videoDevice = priv->request->videoDeviceUIDs().firstVideoDeviceUID();
> +    const String& audioDevice = priv->request->audioDeviceUIDs().firstAudioDeviceUID();
> +    
> +    priv->request->allow(videoDevice, audioDevice);

Ditto.

[Matthew Daiter]

Created attachment 258876 [details]
Patch

[Jer Noble]

Comment on attachment 258876 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 258876 [details]
Patch

Clearing flags on attachment: 258876

Committed r188385: <http://trac.webkit.org/changeset/188385>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

Comment on attachment 258876 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:91
> +static void webkitUserMediaPermissionRequestAllow(WebKitPermissionRequest* request, const char* videoDeviceUID, const char* audioDeviceUID)
> +{
> +    ASSERT(WEBKIT_IS_USER_MEDIA_PERMISSION_REQUEST(request));
> +
> +    WebKitUserMediaPermissionRequestPrivate* priv = WEBKIT_USER_MEDIA_PERMISSION_REQUEST(request)->priv;
> +
> +    // Only one decision at a time.
> +    if (priv->madeDecision)
> +        return;
> +
> +    priv->madeDecision = true;
> +    priv->request->allow(String::fromUTF8(videoDeviceUID), String::fromUTF8(audioDeviceUID));

I think this is dead code. The interface is defined as void (* allow) (WebKitPermissionRequest *request), and webkitUserMediaPermissionRequestAllow is assigned to WebKitPermissionRequestIface.allow. We have two symbols with the same name, but I guess the compiler chooses the one that matches the signature. So, this one is simply never used.