[webkit-reviews] review denied: [Bug 37025] Web Inspector: GC Run event should be added to Timeline Panel : [Attachment 52621] [patch] Fifth 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 52621: [patch] Fifth iteration.
https://bugs.webkit.org/attachment.cgi?id=52621&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
r- for iterating over original and for poor 'helper' name.

ScriptGCEventHelper is no longer a good name since you can't add listeners to
the helper.

> +public:
> +    static void addEventListener(ScriptGCEventListener*){}
> +    static void removeEventListener(ScriptGCEventListener*){}
> +    static void getHeapSize(size_t&, size_t&){}

should be ") { }"

> +    double endTime = WTF::currentTimeMS();
> +    size_t collectedBytes = s_usedHeapSize - getUsedHeapSize();
> +    for (GCEventListeners::iterator i = s_eventListeners.begin(); i !=
s_eventListeners.end(); ++i)
> +	   (*i)->didGCRun(s_startTime, endTime, collectedBytes);

You should iterate over copy in case client wants to unregister listener after
the first event or something similar.

> +    typedef Vector< ScriptGCEventListener* > GCEventListeners;

Vector<ScriptGCEventListener*>

> +    virtual void didGCRun(double startTime, double endTime, size_t
collectedBytes) = 0;

didGC?