| Summary: | Use UserMediaRequestIdentifier in WebKit rather than a mysterious uint64_t | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
| Component: | WebRTC | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | achristensen, annulen, eric.carlson, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, philipj, ryuan.choi, sergio, simon.fraser, tommyw, webkit-bug-importer, youennf | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | 229278 | ||||||||||||||
| Bug Blocks: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Simon Fraser (smfr)
2021-08-19 14:48:45 PDT
Created attachment 435908 [details]
Patch
Created attachment 435911 [details]
Patch
Created attachment 435937 [details]
Patch
Created attachment 435938 [details]
Patch
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? Created attachment 436027 [details]
Patch
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? |