[webkit-reviews] review denied: [Bug 100103] Web Inspector: Refactor Console's profiling feature to use InspectorProfilerAgent : [Attachment 170117] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 100103: Web Inspector: Refactor Console's profiling feature to use
InspectorProfilerAgent
https://bugs.webkit.org/show_bug.cgi?id=100103

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

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


> Source/WebCore/inspector/InspectorProfilerAgent.cpp:438
> +	   int resolvedLineNumber = 0;

Looks like copy-paste to me. Can we avoid it?

> Source/WebCore/inspector/InspectorProfilerAgent.h:85
> +    virtual void initiateProfiling(ErrorString* = 0, const String* title =
0, ScriptState* = 0, ScriptCallStack* = 0);

I would call them consoleProfile and consoleProfileEnd for clarity. They don't
belong to the protocol, so they should not have ErrorString and should be in a
separate group. We also don't pass strings by pointer.

> Source/WebCore/page/Console.cpp:275
> +    InspectorInstrumentation::startProfiling(page, title, state,
callStack.get());

I would call them consoleProfile and consoleProfileEnd for clarity.