[webkit-reviews] review denied: [Bug 90264] Web Inspector: added low-level instrumentation support for TimelineAgent : [Attachment 150628] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Sergey Rogulenko
<rogulenko at google.com>'s request for review:
Bug 90264: Web Inspector: added low-level instrumentation support for
TimelineAgent
https://bugs.webkit.org/show_bug.cgi?id=90264

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

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


> Source/WebCore/inspector/InspectorInstrumentation.cpp:1140
> +InspectorTimelineAgent* InspectorInstrumentation::timelineAgentInstance()

Nit: here and below: there is no need for "Instance" suffix.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:1153
> +    return instance;

You should return reference here.

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:513
> +    , m_lowlevelEventsEnabledStackMark(-1)

Can we get away with 0 as the default value?

> Source/WebCore/inspector/InspectorTimelineAgent.h:165
> +    void pushCurrentRecord(PassRefPtr<InspectorObject>, const String& type,
bool captureCallStack, Frame*, bool hasLowlevelDetails = false);

hasLowLevelDetails

> Source/WebCore/inspector/InspectorTimelineAgent.h:167
> +	   

Remove new chars