[Webkit-unassigned] [Bug 134421] Enhance Speedometer automation testing

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=134421





--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org>  2014-09-03 08:33:49 PST ---
(From update of attachment 236947)
View in context: https://bugs.webkit.org/attachment.cgi?id=236947&action=review

> PerformanceTests/Speedometer/resources/main.js:153
> +function startBenchmark(selectedSuite) {
> +    var enabledSuites = Suites.filter(function (suite) { if (selectedSuite) return selectedSuite == suite.name; return !suite.disabled });

The right approach will be for this function to check the query string
since this is the function responsible for filtering suites.

> PerformanceTests/Speedometer/resources/main.js:201
> +function getQueryVariable(variable) {

WebKit's naming convention doesn't use "get" prefix unless we have an out argument.
I'd call this function queryStringValueForKey(key) if I were you.

> PerformanceTests/Speedometer/resources/main.js:204
> +    var vars = queryString.split("&");
> +    for (var i = 0; i < vars.length; i++) {

Each segment split by & are "pairs".
Also, we've been using single quotation marks for strings so please be consistent and use a single quotation here.

> PerformanceTests/Speedometer/resources/main.js:205
> +        var pair = vars[i].split("=");

Ditto.  s/"/'/g.

> PerformanceTests/Speedometer/resources/main.js:209
> +    return false;

It seems strange to return "false" when there's no value.
I think null or undefined will be better.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.