Bug 175811 - Speedometer 2.0: Make it possible to change the iteration count via query string
Summary: Speedometer 2.0: Make it possible to change the iteration count via query string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 172339
  Show dependency tree / graph
 
Reported: 2017-08-21 22:06 PDT by Ryosuke Niwa
Modified: 2017-08-22 14:02 PDT (History)
8 users (show)

See Also:

Attachments
Patch (3.67 KB, patch)
2017-08-21 23:00 PDT, Ryosuke Niwa 		saam: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2017-08-21 22:06:47 PDT 
Make it possible to change the iteration count of Speedometer via query string.
Also reduce the default iteration count from 20 to 10.
Comment 1 Ryosuke Niwa 2017-08-21 23:00:02 PDT 
Created attachment 318738 [details]
Patch
Comment 2 Mathias Bynens 2017-08-22 04:32:30 PDT 
Comment on attachment 318738 [details]
Patch

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

> PerformanceTests/Speedometer/resources/main.js:154
> +        var parts = location.search.substring(1).split('&');

Shame Edge doesn’t support `URLSearchParams` yet — that would simplify this section greatly!

> PerformanceTests/Speedometer/resources/main.js:171
> +                    console.error('Invalid iteratin count: ' + value);

typo: iteration
Comment 3 Saam Barati 2017-08-22 05:00:49 PDT 
Comment on attachment 318738 [details]
Patch

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

> PerformanceTests/Speedometer/resources/main.js:160
> +            case 'unit':

Why have this parameter if only “ms” is allowed?

> PerformanceTests/Speedometer/resources/main.js:168
> +                if (!isNaN(value))

Should be isNaN(parsedValue)
Comment 4 Ryosuke Niwa 2017-08-22 13:58:42 PDT 
(In reply to Saam Barati from comment #3)
> Comment on attachment 318738 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318738&action=review
> 
> > PerformanceTests/Speedometer/resources/main.js:160
> > +            case 'unit':
> 
> Why have this parameter if only “ms” is allowed?

Just for clarity. ?unit=ms reads better than ?ms.

> > PerformanceTests/Speedometer/resources/main.js:168
> > +                if (!isNaN(value))
> 
> Should be isNaN(parsedValue)

Oops, fixed.
Comment 5 Ryosuke Niwa 2017-08-22 13:58:53 PDT 
(In reply to Mathias Bynens from comment #2)
> Comment on attachment 318738 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318738&action=review
> 
> > PerformanceTests/Speedometer/resources/main.js:154
> > +        var parts = location.search.substring(1).split('&');
> 
> Shame Edge doesn’t support `URLSearchParams` yet — that would simplify this
> section greatly!
> 
> > PerformanceTests/Speedometer/resources/main.js:171
> > +                    console.error('Invalid iteratin count: ' + value);
> 
> typo: iteration

Fixed.
Comment 6 Ryosuke Niwa 2017-08-22 14:00:51 PDT 
Committed r221044: <http://trac.webkit.org/changeset/221044>
Comment 7 Radar WebKit Bug Importer 2017-08-22 14:02:04 PDT 
<rdar://problem/34020863>