Bug 194752 – Continue reducing use of String::format, now focusing on hex: "%p", "%x", etc.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Darin Adler]

Continue reducing use of String::format, now focusing on hex: "%p", "%x", etc.

[Darin Adler]

Created attachment 362226 [details]
Patch

[Darin Adler]

Probably not *quite* ready for review; I’m guessing there will be compilation errors on some non-Apple platforms.

[EWS Watchlist]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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.

[EWS Watchlist]

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

[Darin Adler]

Created attachment 362249 [details]
Patch

[Darin Adler]

Created attachment 362287 [details]
Patch

[Darin Adler]

Created attachment 362293 [details]
Patch

[Daniel Bates]

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.

[Daniel Bates]

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

[Darin Adler]

Committed r241751: <https://trac.webkit.org/changeset/241751>

[Radar WebKit Bug Importer]

<rdar://problem/48186054>