[webkit-reviews] review granted: [Bug 208607] Use the feature flags mechanism to give default feature preference values : [Attachment 392640] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.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 392640: Patch

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




--- Comment #12 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 392640
  --> https://bugs.webkit.org/attachment.cgi?id=392640
Patch

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

> Source/WebCore/PAL/pal/spi/cocoa/FeatureFlagsSPI.h:35
> +bool _os_feature_enabled_impl(const char *domain, const char *feature);

Do we need an extern "C" around this?

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:85
> +bool defaultAsyncFrameScrollingEnabled()

Maybe make this into a static helper function called
defaultAsyncFrameAndOverflowScrollingEnabled() and call it from the two places.

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:88
> +#if PLATFORM(IOS_FAMILY) || USE(NICOSIA)

Give USE(NICOSIA) its own #if block and return true.

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:104
> +{
> +#if PLATFORM(COCOA)
> +    return defaultAsyncFrameScrollingEnabled();
> +#else
> +    return false;
> +#endif

Need a USE(NICOSIA) here

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:137
> +    return os_feature_enabled(WebKit, webrtc_in_gpu_process);
> +#else
> +    return os_feature_enabled(WebKit, canvas_and_media_in_gpu_process);

This is confusing, and seems like it would prevent us from toggling audio
capture independently if we need to.