Bug 198918 – Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Darin Adler]

Use even more "shortest form" formatting, and less "fixed precision" and "fixed width"

[Darin Adler]

Created attachment 372248 [details]
Patch

[Darin Adler]

Not looking to make these changes yet, just hoping to use EWS to get some ideas about test output without running them locally.

[EWS Watchlist]

Attachment 372248 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Created attachment 372424 [details]
Patch

[EWS Watchlist]

Comment on attachment 372424 [details]
Patch

Attachment 372424 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12516589

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 372428 [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 372424 [details]
Patch

Attachment 372424 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12516476

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 372429 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.5

[EWS Watchlist]

Comment on attachment 372424 [details]
Patch

Attachment 372424 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12516645

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 372430 [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 372424 [details]
Patch

Attachment 372424 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12516694

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 372432 [details]
Archive of layout-test-results from ews215 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews215  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[EWS Watchlist]

Comment on attachment 372424 [details]
Patch

Attachment 372424 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12516624

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 372433 [details]
Archive of layout-test-results from ews116 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Darin Adler]

Created attachment 386589 [details]
Patch

[Darin Adler]

Created attachment 386592 [details]
Patch

[Darin Adler]

Created attachment 386689 [details]
Patch

[Darin Adler]

Created attachment 386693 [details]
Patch

[Darin Adler]

Created attachment 386697 [details]
Patch

[Darin Adler]

Created attachment 386700 [details]
Patch

[Darin Adler]

Created attachment 386719 [details]
Patch

[Darin Adler]

Created attachment 386738 [details]
Patch

[Darin Adler]

Created attachment 386747 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 386747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386747&action=review

> Source/WebKit/NetworkProcess/cache/NetworkCacheEntry.cpp:242
> +    json.appendLiteral("\n}\n" "}");

This can probably be combined:

    - json.appendLiteral("\n}\n" "}");
    + json.appendLiteral("\n}\n}");

[Darin Adler]

Comment on attachment 386747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386747&action=review

>> Source/WebKit/NetworkProcess/cache/NetworkCacheEntry.cpp:242
>> +    json.appendLiteral("\n}\n" "}");
> 
> This can probably be combined:
> 
>     - json.appendLiteral("\n}\n" "}");
>     + json.appendLiteral("\n}\n}");

Thanks, I will do that. Or maybe break it into three lines they way I did with the code above!

[Darin Adler]

Created attachment 387480 [details]
Patch

[Darin Adler]

Created attachment 387495 [details]
Patch

[Darin Adler]

Created attachment 387496 [details]
Patch

[Darin Adler]

Created attachment 387538 [details]
Patch

[Darin Adler]

Sam, you should definitely review this one!

[Darin Adler]

Comment on attachment 387538 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387538&action=review

> Source/WebCore/platform/graphics/Color.cpp:335
> +    if (int thirdDigit = (alpha * 100 + 0x7F) / 0xFF % 10)

Just noticed that type here should be "unsigned" or "auto" rather than "int" I think. Not important either way.

[Sam Weinig]

Comment on attachment 387538 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387538&action=review

Very nice!

> Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:201
> +            CString scriptText = makeString(

I wonder if would make sense to have a makeUTF8CString function like this? Obviously, if tests were the only users, probably not, but I will keep an eye out for other cases.

[Darin Adler]

Comment on attachment 387538 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387538&action=review

>> Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:201
>> +            CString scriptText = makeString(
> 
> I wonder if would make sense to have a makeUTF8CString function like this? Obviously, if tests were the only users, probably not, but I will keep an eye out for other cases.

I had the *exact* same thought when coding this. Why make a UChar buffer only to narrow it to LChar/UTF-8 at the end? Can make the string the proper width every step of the way. And similarly, since this is only in a test, decided not to pursue it for now.

[Darin Adler]

Committed r254514: <https://trac.webkit.org/changeset/254514>

[Radar WebKit Bug Importer]

<rdar://problem/58569735>