Bug 175815

Summary: REGRESSION(r216718): Speedometer 2.0: Fix vanilla JS examples
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: addyo, buildbot, cdumez, ggaren, joepeck, keith_miller, lforschler, mathias, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 172339    
Attachments:
Description Flags
Fixes the bug saam: review+

Description Ryosuke Niwa 2017-08-22 00:50:53 PDT
Vanilla JS & ES2015 test cases got broken again in https://trac.webkit.org/changeset/217107

We can't use new Date().getTime() to generate an unique ID.
Comment 1 Ryosuke Niwa 2017-08-22 00:56:49 PDT
Created attachment 318744 [details]
Fixes the bug
Comment 2 Ryosuke Niwa 2017-08-22 00:57:52 PDT
Comment on attachment 318744 [details]
Fixes the bug

bundle.app* files are generated from npm run build. See changes in
PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js
PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015/src/store.js
for the actual code/test change.
Comment 3 Mathias Bynens 2017-08-22 01:11:14 PDT
Comment on attachment 318744 [details]
Fixes the bug

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

> PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js:5
> +var uniqueID = 0;

`vanilla-examples/es2015/src/store.js` has the same line but uses `let` instead of `var` and starts at `1` instead of `0`. Is there a reason for these subtle differences? If so, we may want to add a comment saying so.
Comment 4 Mathias Bynens 2017-08-22 01:40:21 PDT
How did https://trac.webkit.org/changeset/220043/webkit(In reply to Ryosuke Niwa from comment #0)
> Vanilla JS & ES2015 test cases got broken again in
> https://trac.webkit.org/changeset/217107

That doesn’t seem like the right link.

This bug’s title refers to R220043 but that patch didn’t touch any JavaScript: https://trac.webkit.org/changeset/220043

Did you mean R216718 instead? https://trac.webkit.org/changeset/216718
Comment 5 Ryosuke Niwa 2017-08-22 13:54:46 PDT
(In reply to Mathias Bynens from comment #3)
> Comment on attachment 318744 [details]
> Fixes the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318744&action=review
> 
> > PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js:5
> > +var uniqueID = 0;
> 
> `vanilla-examples/es2015/src/store.js` has the same line but uses `let`
> instead of `var` and starts at `1` instead of `0`. Is there a reason for
> these subtle differences? If so, we may want to add a comment saying so.

It looked like all variable declarations in this file was using `var` so I used that to be consistent.

(In reply to Mathias Bynens from comment #4)
> How did https://trac.webkit.org/changeset/220043/webkit(In reply to Ryosuke
> Niwa from comment #0)
> > Vanilla JS & ES2015 test cases got broken again in
> > https://trac.webkit.org/changeset/217107
> 
> That doesn’t seem like the right link.
> 
> This bug’s title refers to R220043 but that patch didn’t touch any
> JavaScript: https://trac.webkit.org/changeset/220043
> 
> Did you mean R216718 instead? https://trac.webkit.org/changeset/216718

Oops, fixed.
Comment 6 Saam Barati 2017-08-22 16:04:08 PDT
Comment on attachment 318744 [details]
Fixes the bug

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

>>> PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js:5
>>> +var uniqueID = 0;
>> 
>> `vanilla-examples/es2015/src/store.js` has the same line but uses `let` instead of `var` and starts at `1` instead of `0`. Is there a reason for these subtle differences? If so, we may want to add a comment saying so.
> 
> It looked like all variable declarations in this file was using `var` so I used that to be consistent.
> 
> (In reply to Mathias Bynens from comment #4)

As Mathias asked, is there a reason to use 0 instead of 1 or vice versa?
Comment 7 Ryosuke Niwa 2017-08-22 16:06:16 PDT
Oh oops, clearly, I need to start the value at 1.
Comment 8 Ryosuke Niwa 2017-08-22 16:11:07 PDT
Committed r221054: <http://trac.webkit.org/changeset/221054>
Comment 9 Radar WebKit Bug Importer 2017-08-22 16:11:42 PDT
<rdar://problem/34023999>