[webkit-reviews] review denied: [Bug 74603] Web Inspector: Implement CSS selector profiler backend : [Attachment 119425] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 74603: Web Inspector: Implement CSS selector profiler backend
https://bugs.webkit.org/show_bug.cgi?id=74603

Attachment 119425: Patch
https://bugs.webkit.org/attachment.cgi?id=119425&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119425&action=review


> Source/WebCore/inspector/Inspector.json:1619
> +		   "type": "object",

(You should expand the properties here). I don't think you need a header here.
CPU / Heap profilers have it due to historical reasons. I think CSS profiles
are lightweight enough to be pushed into the front-end atomically.

> Source/WebCore/inspector/Inspector.json:1628
> +		       { "name": "hits", "type": "integer", "description":
"Number of times this selector was considered a candidate for matching against
DOM elements." },

hitCount ?

> Source/WebCore/inspector/Inspector.json:1629
> +		       { "name": "matches", "type": "integer", "description":
"Number of times this selector actually matched a DOM element." }

matchCount ?

> Source/WebCore/inspector/Inspector.json:1780
> +		   "name": "stopSelectorProfiler"

This one should return the profile body.

> Source/WebCore/inspector/Inspector.json:1783
> +		   "name": "getProfileHeaders",

... and you should not need this.

> Source/WebCore/inspector/Inspector.json:1789
> +		   "name": "getProfile",

and this

> Source/WebCore/inspector/Inspector.json:1798
> +		   "name": "removeProfile",

and this

> Source/WebCore/inspector/Inspector.json:1804
> +		   "name": "clearProfiles"

and this :)

> Source/WebCore/inspector/Inspector.json:1813
> +		   "name": "addProfileHeader",

and this

> Source/WebCore/inspector/Inspector.json:1819
> +		   "name": "setRecordingProfile",

and this