[webkit-reviews] review granted: [Bug 208607] Use the feature flags mechanism to give default feature preference values : [Attachment 392665] Update the patch based on Simon's comments

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 208607: Use the feature flags mechanism to give default feature preference
values
https://bugs.webkit.org/show_bug.cgi?id=208607

Attachment 392665: Update the patch based on Simon's comments

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




--- Comment #20 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 392665
  --> https://bugs.webkit.org/attachment.cgi?id=392665
Update the patch based on Simon's comments

r=me.
Please make sure GTK/WPE bots are compiling.
It might be worth testing that this works as expected on Mac/iOS.

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

> Source/WebKit/Shared/WebPreferences.yaml:1755
> +  condition: ENABLE(GPU_PROCESS)

If you add ENABLE(GPU_PROCESS) to RenderCanvasInGPUProcessEnabled, you should
probably add it for other GPU stuff like CaptureAudioInGPUProcessEnabled,
CaptureVideoInGPUProcessEnabled and WebRTCPlatformCodecsInGPUProcessEnabled.
And keep that in sync with the ifdef of defaultRenderCanvasInGPUProcessEnabled
definition.
Not sure this is worth it though.

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:90
> +#if PLATFORM(MAC) && HAVE(HAVE_SYSTEM_FEATURE_FLAGS)

This should probably be
#if HAVE(SYSTEM_FEATURE_FLAGS)
Ditto elsewhere

We could use #elif here and below.

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:150
> +    return !defaultCaptureAudioInGPUProcessEnabled();

Let's change this to:
#if PLATFORM(MAC)
    return !defaultCaptureAudioInGPUProcessEnabled();
#else
   return false;
#endif