| Summary: | Continue reducing use of String::format, now focusing on hex: "%p", "%x", etc. | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||||||||
| Component: | WebKit Misc. | Assignee: | Darin Adler <darin> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | dbates, ews-watchlist, guijemont, guijemont+jsc-armv7-ews, rniwa, webkit-bug-importer | ||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||
| Bug Blocks: | 30342 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Darin Adler
2019-02-16 21:34:55 PST
Created attachment 362226 [details]
Patch
Probably not *quite* ready for review; I’m guessing there will be compilation errors on some non-Apple platforms. Comment on attachment 362226 [details] Patch Attachment 362226 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11178805 Number of test failures exceeded the failure limit. Created attachment 362227 [details]
Archive of layout-test-results from ews100 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 362226 [details] Patch Attachment 362226 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11178801 Number of test failures exceeded the failure limit. Created attachment 362228 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 362226 [details] Patch Attachment 362226 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11178775 Number of test failures exceeded the failure limit. Created attachment 362229 [details]
Archive of layout-test-results from ews113 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 362226 [details] Patch Attachment 362226 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11178835 Number of test failures exceeded the failure limit. Created attachment 362230 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 362249 [details]
Patch
Created attachment 362287 [details]
Patch
Created attachment 362293 [details]
Patch
Comment on attachment 362293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362293&action=review > Source/WTF/wtf/text/StringConcatenateNumbers.h:119 > + : m_number(number) No change necessary. Uniform initializer syntax (UIS)? > Source/WebCore/html/HTMLMediaElement.h:1210 > + static String toString(const WebCore::HTMLMediaElement::AutoplayEventPlaybackState reason) { return convertEnumerationToString(reason); } Ok as-is. Const not necessary. > Source/WebCore/html/HTMLMediaElement.h:1217 > + static String string(WebCore::TextTrackCue* const& cue) { return cue->debugString(); } Ok as-is. const type without const pointer and the ref would be better. Find yourself with more free time, it would be better to pass by const lvalue ref. > Source/WebCore/platform/graphics/Color.cpp:393 > + if (alpha() < 0xFF) > + return makeString('#', hex(red(), 2), hex(green(), 2), hex(blue(), 2), hex(alpha(), 2)); > + return makeString('#', hex(red(), 2), hex(green(), 2), hex(blue(), 2)); Nice. > Source/WebCore/rendering/RenderFragmentedFlow.h:287 > + static String string(const WeakPtr<WebCore::RenderFragmentContainer> value) { return value.get() ? value->debugString() : String(); } No change necessary. .get() is not necessary. Could use UIS for nullptr branch. Comment on attachment 362293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362293&action=review > Source/JavaScriptCore/ChangeLog:11 > + keep behavior the same, so lets do that. Ok as-is. lets => let's Committed r241751: <https://trac.webkit.org/changeset/241751> |