[webkit-reviews] review denied: [Bug 74603] Web Inspector: Implement CSS selector profiler backend : [Attachment 119847] 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 119847: Patch
https://bugs.webkit.org/attachment.cgi?id=119847&action=review

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


> Source/WebCore/inspector/Inspector.json:1637
> +		       { "name": "title", "type": "string", "description":
"Profile title." },

You don't need a title for the profile

> Source/WebCore/inspector/Inspector.json:1638
> +		       { "name": "uid", "$ref": "SelectorProfileId",
"description": "Profile identifier (unique among CSS selector profiles.)" },

Neither you need the uid or typeId.

> Source/WebCore/inspector/InspectorCSSAgent.cpp:136
> +static const char userInitiatedSelectorProfiling[] =
"userInitiatedSelectorProfiling";

Rename to "selectorProfilerStarted"

> Source/WebCore/inspector/InspectorCSSAgent.cpp:140
> +static const char* const UserInitiatedProfileName =
"org.webkit.profiles.user-initiated";

ditto

> Source/WebCore/inspector/InspectorCSSAgent.h:193
> +    unsigned m_nextUserInitiatedSelectorProfileNumber;

You don't need this.

> Source/WebCore/inspector/InspectorCSSAgent.h:203
> +    SelectorProfilesMap m_selectorProfiles;

You don't need this.