[webkit-reviews] review granted: [Bug 218348] REGRESSION (r254038): Simple.com money transfer UI is very laggy (multiple seconds per keypress) : [Attachment 412976] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 218348: REGRESSION (r254038): Simple.com money transfer UI is very laggy
(multiple seconds per keypress)
https://bugs.webkit.org/show_bug.cgi?id=218348

Attachment 412976: Patch

https://bugs.webkit.org/attachment.cgi?id=412976&action=review




--- Comment #38 from Darin Adler <darin at apple.com> ---
Comment on attachment 412976
  --> https://bugs.webkit.org/attachment.cgi?id=412976
Patch

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

> Source/JavaScriptCore/runtime/JSDateMath.cpp:119
> +	   return LocalTimeOffset { false, 0 };
> +    return LocalTimeOffset { !!dstOffset, rawOffset + dstOffset };

We don’t actually have to write LocalTimeOffset here, do we?

> JSTests/ChangeLog:20
> +	   * ChakraCore.yaml:
> +	   * ChakraCore/test/Date/DateCtr.baseline-jsc: Added. The time before
America/Los_Angeles timezone is effective should be handled as UTC-0752.
> +	   * complex.yaml:
> +	   * complex/timezone-offset-before-america-los-angeles-is-defined.js:
Added for UTC-0752.
> +	   (shouldBe):
> +	   * microbenchmarks/local-date-constructor.js: Added.
> +	   (test):
> +	   * mozilla/ecma/Date/15.9.5.16.js:
> +	   * mozilla/ecma/Date/15.9.5.18.js:
> +	   * mozilla/ecma/Date/15.9.5.22-1.js:
> +	   * mozilla/ecma/Date/15.9.5.22-2.js:
> +	   * mozilla/ecma/Date/15.9.5.35-1.js:
> +	   (getTestCases):

I wish this explained the test changes better. Could get rid of the silly
quoting of the function names, and add comments explaining the reasons these
are good changes to make.

> JSTests/mozilla/ecma/Date/15.9.5.16.js:-58
> -    addTestCase( TIME_YEAR_0 );

The comments don’t explain this. I think I understand it, but it would have
been good to explain it in the change log.

> JSTests/mozilla/ecma/Date/15.9.5.18.js:-59
> -    addTestCase( TIME_YEAR_0 );

Ditto.

> JSTests/mozilla/ecma/Date/15.9.5.22-1.js:-61
> -    addTestCase( TIME_YEAR_0 );

Ditto.

> JSTests/mozilla/ecma/Date/15.9.5.22-2.js:60
> -    addTestCase( TIME_YEAR_0 );
>  /*
> +    addTestCase( TIME_YEAR_0 );

Ditto.