[webkit-reviews] review granted: [Bug 208402] [GPUP] Implement Legacy EME API in the GPU Process : [Attachment 392684] Part 2: WebKit

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 208402: [GPUP] Implement Legacy EME API in the GPU Process
https://bugs.webkit.org/show_bug.cgi?id=208402

Attachment 392684: Part 2: WebKit

https://bugs.webkit.org/attachment.cgi?id=392684&action=review




--- Comment #10 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 392684
  --> https://bugs.webkit.org/attachment.cgi?id=392684
Part 2: WebKit

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

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:82
> +   
gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMFactoryPr
oxy::SupportsKeySystem(keySystem, WTF::nullopt),
Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem::Reply(supported), {
});

It would be good to cache results so we only need to make one sync IPC per key
system

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:89
> +   
gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMFactoryPr
oxy::SupportsKeySystem(keySystem, mimeType),
Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem::Reply(supported), {
});

Ditto for key system + MIME type

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:141
> +
> +

Nit: two blanks

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp:72
> +    return { SharedBuffer::create((const char*)array->data(),
array->byteLength()) };

Is the cast necessary (you don't have one in convertToOptionalDataReference)?

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp:131
> +   
m_factory->gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyC
DMSessionProxy::CachedKeyForKeyID(keyId),
Messages::RemoteLegacyCDMSessionProxy::CachedKeyForKeyID::Reply(ipcKey),
m_identifier);

Can these be cached here so we only need to make one sync IPC per cached key
ID?