[webkit-reviews] review granted: [Bug 95282] Deploy ASCIILiteral hotness throughout WebCore : [Attachment 161132] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 28 21:38:10 PDT 2012


Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 95282: Deploy ASCIILiteral hotness throughout WebCore
https://bugs.webkit.org/show_bug.cgi?id=95282

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=161132&action=review


> Source/WebCore/css/CSSPropertySourceData.cpp:91
> +    DEFINE_STATIC_LOCAL(String, emptyValue, (ASCIILiteral("e")));

Maw?

> Source/WebCore/dom/MicroDataItemList.cpp:45
>      DEFINE_STATIC_LOCAL(String, undefinedItemTypeString, (""));
> +    // FIXME: Why not just return emptyString(); ?
>      return undefinedItemTypeString;

Someone hit me with a spoon.

> Source/WebCore/editing/MarkupAccumulator.cpp:238
> -    DEFINE_STATIC_LOCAL(String, xmlnsWithColon, ("xmlns:"));
> +    DEFINE_STATIC_LOCAL(String, xmlnsWithColon, (ASCIILiteral("xmlns:")));

This seems odd.

> Source/WebCore/inspector/InspectorStyleSheet.cpp:638
> +    DEFINE_STATIC_LOCAL(String, defaultPrefix, (ASCIILiteral("    ")));

!?!

> Source/WebCore/loader/MainResourceLoader.cpp:374
> +	       DEFINE_STATIC_LOCAL(String, consoleMessage,
(ASCIILiteral("Refused to display document because display forbidden by
X-Frame-Options.\n")));

Why the \n?

> Source/WebCore/page/Page.cpp:395
>      DEFINE_STATIC_LOCAL(String, nullString, ());
> +    // FIXME: Why not just return String()?

Oh please.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:126
> -    DEFINE_STATIC_LOCAL(String, fontFace, ("Arial"));
> +    DEFINE_STATIC_LOCAL(String, fontFace, (ASCIILiteral("Arial")));

WTF?

> Source/WebCore/xml/parser/XMLTokenizer.cpp:485
> -	   DEFINE_STATIC_LOCAL(String, xmlString, ("xml"));
> +	   DEFINE_STATIC_LOCAL(String, xmlString, (ASCIILiteral("xml")));

XMLNames bro.

> Source/WebCore/xml/parser/XMLTreeBuilder.cpp:341
> +    DEFINE_STATIC_LOCAL(String, ampS, (ASCIILiteral("&")));
> +    DEFINE_STATIC_LOCAL(String, aposS, (ASCIILiteral("'")));
> +    DEFINE_STATIC_LOCAL(String, gtS, (ASCIILiteral(">")));
> +    DEFINE_STATIC_LOCAL(String, ltS, (ASCIILiteral("<")));
> +    DEFINE_STATIC_LOCAL(String, quotS, (ASCIILiteral("\"")));

We have the Entity technology to help you. :)


More information about the webkit-reviews mailing list