[webkit-reviews] review denied: [Bug 232555] [WinCairo] Enable gpu_process_canvas_rendering and gpu_process_webgl by default : [Attachment 442959] Patch

[bugzilla-daemon at webkit.org]

Don Olmstead <don.olmstead at sony.com> has denied Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 232555: [WinCairo] Enable gpu_process_canvas_rendering and
gpu_process_webgl by default
https://bugs.webkit.org/show_bug.cgi?id=232555

Attachment 442959: Patch

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




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

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

Think there's a better way to address this.

As an aside it would be nice to be able to set these registry values through
the MiniBrowser.

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:164
> -#if ENABLE(GPU_PROCESS_BY_DEFAULT)
> +#if ENABLE(GPU_PROCESS_BY_DEFAULT) || PLATFORM(WIN)

Can't we just add `ENABLE_GPU_PROCESS_BY_DEFAULT` in the CMake code?

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:196
> -    return isFeatureFlagEnabled("gpu_process_webgl", false);
> +#if PLATFORM(WIN)
> +    bool defaultValue = true;
> +#else
> +    bool defaultValue = false;
> +#endif
> +    return isFeatureFlagEnabled("gpu_process_webgl", defaultValue);

Couldn't this be written in terms of `ENABLE(GPU_PROCESS_BY_DEFAULT)`?

Maybe need to ask the Apple folks if this is ok.

> Source/WebKit/Shared/win/WebPreferencesDefaultValuesWin.cpp:46
> -	   return false;
> +	   return defaultValue;
>      if (keyType != REG_DWORD)
> -	   return false;
> +	   return defaultValue;
>      if (dataSize != sizeof data)
> -	   return false;
> +	   return defaultValue;

Whoops! This is a good catch here.