[webkit-reviews] review denied: [Bug 37025] Web Inspector: GC Run event should be added to Timeline Panel : [Attachment 52555] [patch] Third iteration.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 37025: Web Inspector: GC Run event should be added to Timeline Panel
https://bugs.webkit.org/show_bug.cgi?id=37025

Attachment 52555: [patch] Third iteration.
https://bugs.webkit.org/attachment.cgi?id=52555&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Looks better, but bi-directional dependency is still there. Could you introduce
a ScriptGCHelperListener (as ScriptDebugServerListener in WebCore/inspector) to
resolve that?

Rest of the nits below. 


> +namespace WebCore {
> +class ScriptGCEventHelper
> +{

Ad blank line here.

> +public:
> +    ScriptGCEventHelper(InspectorTimelineAgent*){}
> +    static void getHeapSize(size_t&, size_t&) {}
> +};

Ad blank line here.

> +} // namespace WebCore

> +namespace WebCore {

Ad blank line here.

> +    static int s_helpersCounter;

I guess this one is no longer needed.

> -    : m_frontend(frontend)
> +    : m_frontend(frontend), m_gcEventHelper(this)

Move ", m_gcEventHelper" to the next line.