Bug 198918

Summary:

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

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

WebCore Misc.

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aboxhall, apinheiro, benjamin, calvaris, cdumez, cfleizach, cgarcia, cmarcelo, dbates, dino, dmazzoni, eric.carlson, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gyuyoung.kim, hi, jamesr, jcraig, jdiggs, jer.noble, joepeck, keith_miller, kondapallykalyan, luiz, macpherson, mark.lam, menard, msaboff, pdr, philipj, rniwa, saam, sabouhallawa, samuel_white, sam, schenney, sergio, simon.fraser, tonikitoo, tzagallo, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Archive of layout-test-results from ews100 for mac-highsierra	none
Archive of layout-test-results from ews122 for ios-simulator-wk2	none
Archive of layout-test-results from ews105 for mac-highsierra-wk2	none
Archive of layout-test-results from ews215 for win-future	none
Archive of layout-test-results from ews116 for mac-highsierra	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	sam: review+

Description Darin Adler 2019-06-17 09:21:23 PDT

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

Comment 1 Darin Adler 2019-06-17 09:21:41 PDT Comment hidden (obsolete)

Created attachment 372248 [details]
Patch

Comment 2 Darin Adler 2019-06-17 09:22:24 PDT

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

Comment 3 EWS Watchlist 2019-06-17 09:24:22 PDT Comment hidden (obsolete)

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.

Comment 4 Darin Adler 2019-06-18 20:47:39 PDT Comment hidden (obsolete)

Created attachment 372424 [details]
Patch

Comment 5 EWS Watchlist 2019-06-18 21:57:27 PDT Comment hidden (obsolete)

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.

Comment 6 EWS Watchlist 2019-06-18 21:57:28 PDT Comment hidden (obsolete)

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

Comment 7 EWS Watchlist 2019-06-18 22:06:33 PDT Comment hidden (obsolete)

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.

Comment 8 EWS Watchlist 2019-06-18 22:06:34 PDT Comment hidden (obsolete)

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

Comment 9 EWS Watchlist 2019-06-18 22:06:45 PDT Comment hidden (obsolete)

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.

Comment 10 EWS Watchlist 2019-06-18 22:06:46 PDT Comment hidden (obsolete)

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

Comment 11 EWS Watchlist 2019-06-18 22:13:30 PDT Comment hidden (obsolete)

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.

Comment 12 EWS Watchlist 2019-06-18 22:13:32 PDT Comment hidden (obsolete)

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

Comment 13 EWS Watchlist 2019-06-18 22:25:22 PDT Comment hidden (obsolete)

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.

Comment 14 EWS Watchlist 2019-06-18 22:25:24 PDT Comment hidden (obsolete)

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

Comment 15 Darin Adler 2020-01-01 13:18:37 PST Comment hidden (obsolete)

Created attachment 386589 [details]
Patch

Comment 16 Darin Adler 2020-01-01 18:31:52 PST Comment hidden (obsolete)

Created attachment 386592 [details]
Patch

Comment 17 Darin Adler 2020-01-03 09:49:18 PST Comment hidden (obsolete)

Created attachment 386689 [details]
Patch

Comment 18 Darin Adler 2020-01-03 09:59:05 PST Comment hidden (obsolete)

Created attachment 386693 [details]
Patch

Comment 19 Darin Adler 2020-01-03 10:43:48 PST Comment hidden (obsolete)

Created attachment 386697 [details]
Patch

Comment 20 Darin Adler 2020-01-03 11:54:09 PST Comment hidden (obsolete)

Created attachment 386700 [details]
Patch

Comment 21 Darin Adler 2020-01-03 15:06:15 PST Comment hidden (obsolete)

Created attachment 386719 [details]
Patch

Comment 22 Darin Adler 2020-01-03 18:02:35 PST Comment hidden (obsolete)

Created attachment 386738 [details]
Patch

Comment 23 Darin Adler 2020-01-03 19:21:48 PST Comment hidden (obsolete)

Created attachment 386747 [details]
Patch

Comment 24 Joseph Pecoraro 2020-01-06 14:13:38 PST

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}");

Comment 25 Darin Adler 2020-01-09 10:18:42 PST

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!

Comment 26 Darin Adler 2020-01-12 11:21:36 PST Comment hidden (obsolete)

Created attachment 387480 [details]
Patch

Comment 27 Darin Adler 2020-01-12 15:30:31 PST Comment hidden (obsolete)

Created attachment 387495 [details]
Patch

Comment 28 Darin Adler 2020-01-12 16:37:08 PST Comment hidden (obsolete)

Created attachment 387496 [details]
Patch

Comment 29 Darin Adler 2020-01-13 09:13:32 PST

Created attachment 387538 [details]
Patch

Comment 30 Darin Adler 2020-01-13 19:29:45 PST

Sam, you should definitely review this one!

Comment 31 Darin Adler 2020-01-14 08:52:14 PST

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.

Comment 32 Sam Weinig 2020-01-14 08:53:36 PST

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.

Comment 33 Darin Adler 2020-01-14 09:02:50 PST

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.

Comment 34 Darin Adler 2020-01-14 09:16:10 PST

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

Comment 35 Radar WebKit Bug Importer 2020-01-14 09:17:25 PST

<rdar://problem/58569735>