[webkit-reviews] review denied: [Bug 115781] [BlackBerry] Fix usage of BlackBerry::Platform::String : [Attachment 201044] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Alberto Garcia
<agarcia at igalia.com>'s request for review:
Bug 115781: [BlackBerry] Fix usage of BlackBerry::Platform::String
https://bugs.webkit.org/show_bug.cgi?id=115781

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=201044&action=review


> Source/WebKit/blackberry/Api/WebSettings.cpp:209
> +    DEFINE_STATIC_LOCAL(AtomicString, WebKitMonospace,
("-webkit-monospace"));
> +    DEFINE_STATIC_LOCAL(AtomicString, WebKitSansSerif,
("-webkit-sans-serif"));
> +    DEFINE_STATIC_LOCAL(AtomicString, WebKitSerif, ("-webkit-serif"));
> +    DEFINE_STATIC_LOCAL(AtomicString, WebKitStandard, ("-webkit-standard"));


Given that "settings" is static local, you will never hit this path again. Why
would you define those static?

It also looks like your "WebSettings" class takes a String, not an
AtomicString.