Bug 229308 – Use UserMediaRequestIdentifier in WebKit rather than a mysterious uint64_t

• Component: WebRTC
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Use UserMediaRequestIdentifier in WebKit rather than a mysterious uint64_t

[Simon Fraser (smfr)]

Created attachment 435908 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 435911 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 435937 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 435938 [details]
Patch

[youenn fablet]

Comment on attachment 435938 [details]
Patch

Looks good.
Can you put UserMediaRequestIdentifier in its own UserMediaRequestIdentifier.h and split m_pendingDeviceRequests in another patch with another type?

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

> Source/WebCore/ChangeLog:11
> +        * Modules/mediastream/UserMediaRequest.h:

It is probably best to put UserMediaRequestIdentifier in its own UserMediaRequestIdentifier.h header.
That will reduce includes and reduce the patch size as well.

> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:649
> +    // This sends UserMediaRequestIdentifierType of 0 because this does not correspond to a UserMediaPermissionRequest in web process.

s/This sends/We use/

> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:689
> +    auto requestID = UserMediaRequestIdentifier::generate();

request is not a user media request but a permission request.
I am not sure we should use the same identifier type as it makes things more difficult to read.
We could use its own ObjectIdentifier<UserMediaPermissionRequest> type.

> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp:804
> +        auto requestID = UserMediaRequestIdentifier::generate();

Ditto

> Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h:155
> +    HashSet<WebCore::UserMediaRequestIdentifier> m_pendingDeviceRequests;

Ditto.

> Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h:23
> +#include "IdentifierTypes.h"

Why do we need this one?

[Simon Fraser (smfr)]

Created attachment 436027 [details]
Patch

[youenn fablet]

Comment on attachment 436027 [details]
Patch

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

> Source/WebKit/Scripts/webkit/messages.py:763
> +        'WebCore::UserMediaRequestIdentifier': ['<WebCore/UserMediaRequestIdentifier.h>'],

Is it actually needed now that we have its own header?

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/281360/webkit

[Radar WebKit Bug Importer]

<rdar://problem/82195936>