Bug 182387 – StyleBench: Attribute selectors and other improvements

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Antti Koivisto]

We should test attribute selectors too.

[Antti Koivisto]

Created attachment 332879 [details]
patch

[Antti Koivisto]

Created attachment 333279 [details]
patch

[Joseph Pecoraro]

Comment on attachment 333279 [details]
patch

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

r=me

> PerformanceTests/StyleBench/resources/style-bench.js:203
> +        if (valueNum==0)
> +            return "";
> +        if (valueNum==1)
> +            return "val";

Normal style would be:

    if (valueNum === 0)
        return "";
    if (valueNum === 1)
        return "val";

Up to you if you want to converge on `==` or `===`.

> PerformanceTests/StyleBench/resources/style-bench.js:204
> +        return `val${ valueNum }`;

Style: Normally you don't use spaces inside the ${...}.

> PerformanceTests/StyleBench/resources/style-bench.js:233
> +        if (operator=='')

Ditto on style. Or here you could even just:

    if (!operator)

> PerformanceTests/StyleBench/resources/style-bench.js:266
> +            result +=  this.randomAttributeSelector();

Style: Accidental double space after the operator.

> PerformanceTests/StyleBench/resources/style-bench.js:493
> +                const count = this.random.number(this.configuration.elementMaximumAttributes) + 1;
> +                for (let i = 0; i < count; ++i)
> +                    element.setAttribute(this.randomAttributeName(), this.randomAttributeValue());

It is bad style to shadow both `i` and `count`. Had you used `var` you'd have run into issues! I'd recommend using different names just for clarity.

[Antti Koivisto]

https://trac.webkit.org/r228237

[Radar WebKit Bug Importer]

<rdar://problem/37321177>