Bug 146866 – Web Inspector: Improve runtime of pseudo-element sidebar style ordering

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

Currently, the algorithm determining where the pseudo-element styles appear in the sidebar has a runtime of O(n^2).  This can be improved to O(n) by getting the most specific selector for each rule when that rule is created.

[Radar WebKit Bug Importer]

<rdar://problem/21778637>

[Devin Rousso]

Created attachment 256636 [details]
Patch

[Timothy Hatcher]

Comment on attachment 256636 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:83
> +        this._mostSpecificSelector = this._determineMostSpecificSelector();

This could be in the getter and called when !this._mostSpecificSelector. That way it is as lazy as possible.

> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:193
> +        if (!this._mostSpecificSelector)

This would need to use the getter if you make it lazy.

> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:196
> +        return this._mostSpecificSelector.isGreaterThan(otherSelector);

Ditto.

> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:211
> +            return;

This should be: return null;

[Devin Rousso]

Created attachment 256651 [details]
Patch

[Timothy Hatcher]

Comment on attachment 256651 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:199
> +            return;

return false; or return true;?

[Devin Rousso]

Comment on attachment 256651 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:199
>> +            return;
> 
> return false; or return true;?

This is meant to be "return false;".  Is it necessary to specify false (since undefined is falsy)?

[Timothy Hatcher]

Comment on attachment 256651 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Models/CSSRule.js:199
>>> +            return;
>> 
>> return false; or return true;?
> 
> This is meant to be "return false;".  Is it necessary to specify false (since undefined is falsy)?

The JIT is happier when types don't change in the flow. We also like to be explicit.

[Devin Rousso]

Created attachment 256667 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 256667 [details]
Patch

Clearing flags on attachment: 256667

Committed r186717: <http://trac.webkit.org/changeset/186717>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.