[webkit-reviews] review denied: [Bug 43406] WebKitTestRunner needs to be able to set the font smoothing type : [Attachment 63527] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Jon Honeycutt
<jhoneycutt at apple.com>'s request for review:
Bug 43406: WebKitTestRunner needs to be able to set the font smoothing type
https://bugs.webkit.org/show_bug.cgi?id=43406

Attachment 63527: Patch
https://bugs.webkit.org/attachment.cgi?id=63527&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +enum {
> +    FontSmoothingLevelStandard = 0,
> +    FontSmoothingLevelLight = 1,
> +    FontSmoothingLevelMedium = 2,
> +    FontSmoothingLevelStrong = 3,
> +    FontSmoothingLevelWindows = 4,
> +};
> +typedef uint32_t FontSmoothingLevel;

Why the typedef? Why not just call the enum "FontSmoothingLevel"? If the
typedef is only needed for WebKit2, it seems inappropriate (and undesirable) to
have it in WebCore.

Should we compile out FontSmoothingLevelWindows on non-Windows platforms?

> +// Defaults to kWKFontSmoothingLevelMedium.
> +WK_EXPORT void WKPreferencesSetFontSmoothingLevel(WKPreferencesRef,
WKFontSmoothingLevel);
> +WK_EXPORT WKFontSmoothingLevel
WKPreferencesGetFontSmoothingLevel(WKPreferencesRef);

kWKFontSmoothingLevelStandard doesn't seem very "Standard" if it isn't the
default!

> @@ -70,6 +70,7 @@ void TestInvocation::resetPreferencesToConsistentValues()
>  {
>      WKPreferencesRef preferences =
WKContextGetPreferences(TestController::shared().context());
>      WKPreferencesSetOfflineWebApplicationCacheEnabled(preferences, true);
> +    WKPreferencesSetFontSmoothingLevel(preferences,
kWKFontSmoothingLevelStandard);
>  }

It seems unfortunate for "Standard" on Windows to trigger an arguably
non-standard mode (i.e., CG font rendering).

The code looks fine, but I'm concerned we don't quite have the right API yet.
For instance, maybe turning on CG font rendering should be a private API on
Windows. It should at least be clearer what's going on. Sam and Anders will
probably be able to give some guidance.