Bug 47714 - Replace some String::format() usages by StringConcatenate in WebKit
: Replace some String::format() usages by StringConcatenate in WebKit
Status: RESOLVED FIXED
: WebKit
WebKit Misc.
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
: 18994
  Show dependency treegraph
 
Reported: 2010-10-15 01:44 PST by
Modified: 2010-10-15 05:30 PST (History)


Attachments
Patch (16.52 KB, patch)
2010-10-15 01:48 PST, Nikolas Zimmermann
no flags Review Patch | Details | Formatted Diff | Diff
Patch v2 (16.60 KB, patch)
2010-10-15 02:12 PST, Nikolas Zimmermann
no flags Review Patch | Details | Formatted Diff | Diff
Patch v3 (16.63 KB, patch)
2010-10-15 02:44 PST, Nikolas Zimmermann
no flags Review Patch | Details | Formatted Diff | Diff
Patch v4 (16.75 KB, patch)
2010-10-15 04:34 PST, Nikolas Zimmermann
no flags Review Patch | Details | Formatted Diff | Diff
Patch v5 (18.09 KB, patch)
2010-10-15 05:12 PST, Nikolas Zimmermann
krit: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-10-15 01:44:27 PST
As done in patch 47664 for WebCore, do the same in WebKit.
------- Comment #1 From 2010-10-15 01:48:07 PST -------
Created an attachment (id=70839) [details]
Patch
------- Comment #2 From 2010-10-15 01:51:16 PST -------
Attachment 70839 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/src/BackForwardListClientImpl.cpp:106:  url_string is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #3 From 2010-10-15 01:54:26 PST -------
(In reply to comment #2)
> Attachment 70839 [details] [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
> WebKit/chromium/src/BackForwardListClientImpl.cpp:106:  url_string is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
> Total errors found: 1 in 14 files

I'm not planing to rename existing variables in chromium code, I think it's safe to ignore this error.
------- Comment #4 From 2010-10-15 02:12:01 PST -------
Created an attachment (id=70842) [details]
Patch v2

Fix a typo s/char/UChar/ in the new StringTypeAdapter. Also changed my mind, renamed url_string to urlString, to have a green style bot result.
------- Comment #5 From 2010-10-15 02:28:16 PST -------
Attachment 70839 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4462042
------- Comment #6 From 2010-10-15 02:44:16 PST -------
Created an attachment (id=70845) [details]
Patch v3

Add explicit cast from WebString -> String, attempting to fix the chromium build.
------- Comment #7 From 2010-10-15 04:16:48 PST -------
Attachment 70845 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4455056
------- Comment #8 From 2010-10-15 04:34:16 PST -------
Created an attachment (id=70850) [details]
Patch v4

Arrgl, missed another WebString -> String conversion in WebPageSerializer. Fix and optimize it even further.
------- Comment #9 From 2010-10-15 05:12:05 PST -------
Created an attachment (id=70854) [details]
Patch v5

*sigh* Forgot to include JavaScriptCore changes. Added StringTypeAdapter<UChar> to avoid having to convert UChars, we can directly append them.
------- Comment #10 From 2010-10-15 05:21:21 PST -------
(From update of attachment 70854 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=70854&action=review

LGTM. r=me

> WebKit/chromium/src/BackForwardListClientImpl.cpp:107
> +    m_pendingHistoryItem = HistoryItem::create(urlString, String(), 0.0);

Please use 0
------- Comment #11 From 2010-10-15 05:30:53 PST -------
Committed r69850: <http://trac.webkit.org/changeset/69850>