[webkit-reviews] review granted: [Bug 228803] [macOS] Clean up Feature Flags related code : [Attachment 435023] Update a layout test expectation

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 228803: [macOS] Clean up Feature Flags related code
https://bugs.webkit.org/show_bug.cgi?id=228803

Attachment 435023: Update a layout test expectation

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




--- Comment #15 from Tim Horton <thorton at apple.com> ---
Comment on attachment 435023
  --> https://bugs.webkit.org/attachment.cgi?id=435023
Update a layout test expectation

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

I really hope we can get rid of this ASAP

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:122
>  #if PLATFORM(MAC)
> -    return true;
> +    bool defaultValue = true;
> +#else
> +    bool defaultValue = false;
>  #endif

This is wrong, async_frame_and_overflow_scrolling should be on everywhere
(confirm with smfr, but the plists agree)

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:313
> +    return isFeatureFlagEnabled("webm_webaudio", false);

Why is this `false` here if it's `true` in the plist? That means that a local
build will have it off on Monterey even though it's on in the system WebKit,
and seems wrong.

> Source/WebKitLegacy/mac/WebView/WebPreferencesDefaultValues.mm:54
> +	   NSBundle *bundle = [NSBundle
bundleForClass:NSClassFromString(@"WKWebView")];

It seems exceedingly bizarre that WebKitLegacy looks up WKWebView; maybe you
should use WebView here? (what does other code do?)

> Source/WebKitLegacy/mac/WebView/WebPreferencesDefaultValues.mm:290
> +    return isFeatureFlagEnabled("webm_webaudio", false);

Why is this `false` here if it's `true` in the plist? That means that a local
build will have it off on Monterey even though it's on in the system WebKit,
and seems wrong.

> Source/WebKitLegacy/mac/WebView/WebPreferencesDefaultValues.mm:299
> -#if HAVE(SYSTEM_FEATURE_FLAGS)
> -    return isFeatureFlagEnabled("vp8_decoder");
> -#endif
> -
> -    return false;
> +    return isFeatureFlagEnabled("vp8_decoder", false);

This key does not exist in any of the feature flags plists, so this should not
use isFeatureFlagEnabled, should just return false (at which point you should
get rid of the function and just put the `false` in WebPreferences.yaml
directly).