Summary: | Improve string use in NavigatorBase.cpp | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryuan Choi <ryuan.choi> | ||||
Component: | WebCore Misc. | Assignee: | Ryuan Choi <ryuan.choi> | ||||
Status: | NEW --- | ||||||
Severity: | Normal | CC: | benjamin, lucas.de.marchi | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ryuan Choi
2013-11-26 21:27:27 PST
Created attachment 217925 [details]
Patch
Cc benjamin, Could you take a look at this? Comment on attachment 217925 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217925&action=review This patch is out of date now (we don't use DEFINE_STATIC_LOCAL anymore), but otherwise looks good. Ben? > Source/WebCore/page/NavigatorBase.cpp:95 > + DEFINE_STATIC_LOCAL(String, platformName, (uname(&osname) >= 0 ? String(osname.sysname) + ' ' + String(osname.machine) : String())); I don't understand the purpose of this change. Is adding the single space more efficient in this context? Comment on attachment 217925 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217925&action=review Yes, this is quite old. If this patch is still valid, I will willingly rebase it. >> Source/WebCore/page/NavigatorBase.cpp:95 >> + DEFINE_STATIC_LOCAL(String, platformName, (uname(&osname) >= 0 ? String(osname.sysname) + ' ' + String(osname.machine) : String())); > > I don't understand the purpose of this change. Is adding the single space more efficient in this context? I thought that single character is more efficient in string concatenation than String(" "). If I understood correctly, StringTypeAdaper<char> will be used in that case. Comment on attachment 217925 [details] Patch Is this patch still valid ? Anyway, cleared review? from attachment 217925 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). |