Convert more static Strings to use ASCIILiteral
Created attachment 161161 [details] Patch
Comment on attachment 161161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161161&action=review > Source/WebCore/Modules/mediasource/MediaSource.h:49 > + static const String& openKeyword(); > + static const String& closedKeyword(); > + static const String& endedKeyword(); Thanks for saving the linker some work. :) > Source/WebCore/css/CSSWrapShapes.cpp:127 > + result.appendLiteral(", "); I assume the compiler uniques these strings? It or the linker must... for sanity's sake. > Source/WebCore/css/StyleSheetContents.cpp:294 > + if (baseURL().string().endsWith("/KHTMLFixes.css") && !sheetText.isNull() && mediaWikiKHTMLFixesStyleSheet.startsWith(sheetText) The web it an amazing place. > Source/WebCore/editing/markup.cpp:203 > const String StyledMarkupAccumulator::styleNodeCloseTag(bool isBlock) This could be const String& > Source/WebCore/editing/markup.cpp:555 > return ""; Oh noes. > Source/WebCore/platform/KURLWTFURL.cpp:116 > + if (isNull()) > + return String(); Returning a reference to a temporary seems like a bad idea? How does the compiler allow this? > Source/WebCore/platform/MIMETypeRegistry.cpp:641 > + mimeTypeMap->add(ASCIILiteral("image/x-ms-bmp"), ASCIILiteral("image/bmp"))); I would have created an addLiterals(map, key, value) helper if I had writen this. > Source/WebCore/svg/SVGAnimatedBoolean.cpp:37 > + animtedType->boolean() = (string == "true"); // wat? Yes. Wat?
(In reply to comment #2) > (From update of attachment 161161 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161161&action=review > > > Source/WebCore/platform/KURLWTFURL.cpp:116 > > + if (isNull()) > > + return String(); > > Returning a reference to a temporary seems like a bad idea? How does the compiler allow this? Hum... You're right. We should use nullAtom.
Comment on attachment 161161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161161&action=review >> Source/WebCore/css/CSSWrapShapes.cpp:127 >> + result.appendLiteral(", "); > > I assume the compiler uniques these strings? It or the linker must... for sanity's sake. Yes.
(In reply to comment #2) > (From update of attachment 161161 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161161&action=review > > > Source/WebCore/platform/MIMETypeRegistry.cpp:641 > > + mimeTypeMap->add(ASCIILiteral("image/x-ms-bmp"), ASCIILiteral("image/bmp"))); > > I would have created an addLiterals(map, key, value) helper if I had writen this. Yeah, the issue is that mimeTypeMap is a WTF template object and that sort of thing only makes sense for String. We could make a subclass, but this seemed easier.
Created attachment 161246 [details] Patch
(In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 161161 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=161161&action=review > > > > > Source/WebCore/platform/MIMETypeRegistry.cpp:641 > > > + mimeTypeMap->add(ASCIILiteral("image/x-ms-bmp"), ASCIILiteral("image/bmp"))); > > > > I would have created an addLiterals(map, key, value) helper if I had writen this. > > Yeah, the issue is that mimeTypeMap is a WTF template object and that sort of thing only makes sense for String. We could make a subclass, but this seemed easier. I meant a static inline helper function. :)
Comment on attachment 161246 [details] Patch LGTM.
(In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #2) > > > (From update of attachment 161161 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=161161&action=review > > > > > > > Source/WebCore/platform/MIMETypeRegistry.cpp:641 > > > > + mimeTypeMap->add(ASCIILiteral("image/x-ms-bmp"), ASCIILiteral("image/bmp"))); > > > > > > I would have created an addLiterals(map, key, value) helper if I had writen this. > > > > Yeah, the issue is that mimeTypeMap is a WTF template object and that sort of thing only makes sense for String. We could make a subclass, but this seemed easier. > > I meant a static inline helper function. :) Ah... Ok.
> >> Source/WebCore/css/CSSWrapShapes.cpp:127 > >> + result.appendLiteral(", "); > > > > I assume the compiler uniques these strings? It or the linker must... for sanity's sake. > > Yes. I can confirm this, I verified that a while ago.
Comment on attachment 161246 [details] Patch Clearing flags on attachment: 161246 Committed r127062: <http://trac.webkit.org/changeset/127062>
All reviewed patches have been landed. Closing bug.