[Webkit-unassigned] [Bug 136183] WebKit and WebKit2: Use ASCIILiteral where possible

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=136183


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #237010|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #2 from Benjamin Poulain <benjamin at webkit.org>  2014-08-22 17:57:59 PST ---
(From update of attachment 237010)
View in context: https://bugs.webkit.org/attachment.cgi?id=237010&action=review

> Source/WebKit/cf/WebCoreSupport/WebInspectorClientCF.cpp:84
> -        *setting = static_cast<bool>(CFBooleanGetValue(static_cast<CFBooleanRef>(value.get()))) ? "true" : "false";
> +        *setting = static_cast<bool>(CFBooleanGetValue(static_cast<CFBooleanRef>(value.get()))) ? ASCIILiteral("true") : ASCIILiteral("false");

>From how this is used, it looks like populateSetting should return a TriState and the call site should create the string as needed.

> Source/WebKit/cf/WebCoreSupport/WebInspectorClientCF.cpp:86
>          *setting = "";

Unrelated but that looks like a bad use of strings. When a value does not exist, the return should be a null string, not an empty string.

> Source/WebKit2/Shared/mac/ChildProcessMac.mm:143
>      String path = String::fromUTF8(pwd.pw_dir);
> -    path.append("/Library");
> +    path.append(ASCIILiteral("/Library"));

String path = String::fromUTF8(pwd.pw_dir) + "/Library";

> Source/WebKit2/Shared/mac/ChildProcessMac.mm:147
> -    path.append("/Preferences");
> +    path.append(ASCIILiteral("/Preferences"));

Let's use StringOperators here, no need to create a new StringImpl for "/Preferences"

> Source/WebKit2/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.mm:57
>      case NSLeftTextAlignment:
> -        return "left";
> +        return ASCIILiteral("left");

I hope clang only generate one constructor here.

> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:144
> +    map->add(ASCIILiteral("insertNewlineIgnoringFieldEditor:"), "InsertNewline");
> +    map->add(ASCIILiteral("insertParagraphSeparator:"), "InsertNewline");
> +    map->add(ASCIILiteral("insertTabIgnoringFieldEditor:"), "InsertTab");
> +    map->add(ASCIILiteral("pageDown:"), "MovePageDown");
> +    map->add(ASCIILiteral("pageDownAndModifySelection:"), "MovePageDownAndModifySelection");
> +    map->add(ASCIILiteral("pageUp:"), "MovePageUp");
> +    map->add(ASCIILiteral("pageUpAndModifySelection:"), "MovePageUpAndModifySelection");

Why only the first of the two strings?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.