[webkit-reviews] review granted: [Bug 207904] Support in-band metadata cues when loading media in the GPU Process : [Attachment 391331] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 207904: Support in-band metadata cues when loading media in the GPU Process
https://bugs.webkit.org/show_bug.cgi?id=207904

Attachment 391331: Patch

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




--- Comment #13 from Dean Jackson <dino at apple.com> ---
Comment on attachment 391331
  --> https://bugs.webkit.org/attachment.cgi?id=391331
Patch

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

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.h:49
> +    WEBCORE_EXPORT static NSArray* allowedClassesForNativeValues();

Nit, I think the * goes near the function name.

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:66
> +SerializedPlatformDataCueMac::~SerializedPlatformDataCueMac()

Do you need this here now? Could it just be left out or marked = default?

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:116
> +NSArray* SerializedPlatformDataCueMac::allowedClassesForNativeValues()

Nit: * again (I think... I'm never sure of the rules in .mm files)

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:211
>  static JSValue *jsValueWithAVMetadataItemInContext(AVMetadataItem *item,
JSContext *context)

Yeah, I guess it does go on that side.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in:28
> -    CreateMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id,
enum:uint8_t WebCore::MediaPlayerEnums::MediaEngineIdentifier
remoteEngineIdentifier, struct WebKit::RemoteMediaPlayerProxyConfiguration
proxyConfiguration) -> (struct WebKit::RemoteMediaPlayerConfiguration
playerConfiguration) Synchronous
> -    DeleteMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id)
> +    CreateMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id,
enum:uint8_t WebCore::MediaPlayerEnums::MediaEngineIdentifier
remoteEngineIdentifier, struct WebKit::RemoteMediaPlayerProxyConfiguration
proxyConfiguration) -> (struct WebKit::RemoteMediaPlayerConfiguration
playerConfiguration) Async
> +	DeleteMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id)

Sam already gave you love for this, so I don't need to!