[webkit-reviews] review denied: [Bug 208814] [GPUP][GTK] compile GPUProcess in GTK port : [Attachment 395704] Patch

[bugzilla-daemon at webkit.org]

Don Olmstead <don.olmstead at sony.com> has denied Víctor M. Jáquez L.
<vjaquez at igalia.com>'s request for review:
Bug 208814: [GPUP][GTK] compile GPUProcess in GTK port
https://bugs.webkit.org/show_bug.cgi?id=208814

Attachment 395704: Patch

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




--- Comment #16 from Don Olmstead <don.olmstead at sony.com> ---
Comment on attachment 395704
  --> https://bugs.webkit.org/attachment.cgi?id=395704
Patch

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

I feel like to start this should just be putting stub files in the directory
and touching CMake files.

With regards to the #if PLATFORM(COCOA) stuff it really feels like its too
early to guard things with COCOA. Once we have some implementations we should
be able to figure out if LayerHostingContext should be shared or not.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:62
> +#if PLATFORM(COCOA)
> +#include "LayerHostingContext.h"

I added a LayerHostingContext.h to Source/WebKit/Platform/generic you can just
include that and get rid of these COCOA changes.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:284
> +#if PLATFORM(COCOA)
>      std::unique_ptr<LayerHostingContext> m_inlineLayerHostingContext;
>      std::unique_ptr<LayerHostingContext> m_fullscreenLayerHostingContext;
> +#endif

Ditto

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.messages.in:29
> +#if PLATFORM(COCOA)
>      PrepareForPlayback(bool privateMode, enum:uint8_t
WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool
prepareForRendering, float videoContentScale) ->
(Optional<WebKit::LayerHostingContextID> inlineLayerHostingContextId,
Optional<WebKit::LayerHostingContextID> fullscreenLayerHostingContextId) Async
> +#endif

Ditto

> Source/WebKit/SourcesGTK.txt:26
> +GPUProcess/media/gstreamer/RemoteMediaPlayerProxyGStreamer.cpp

You're missing a blank line here.

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:44
> +#if PLATFORM(COCOA)
> +#include "LayerHostingContext.h"
> +#endif

Ditto

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:367
> +#if PLATFORM(COCOA)
>      Optional<LayerHostingContextID> m_fullscreenLayerHostingContextId;
> +#endif

Ditto