[Webkit-unassigned] [Bug 89692] Web Inspector: show worker started and finished on timeline

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=89692





--- Comment #23 from Konrad Piascik <kpiascik at rim.com>  2012-07-27 11:12:53 PST ---
(From update of attachment 154987)
View in context: https://bugs.webkit.org/attachment.cgi?id=154987&action=review

You have workerId, url and workerId, url used repeatedly in different places.  Choose an order for the two parameters for consistency.  I prefer workerId, url but I leave it to you

> Source/WebCore/inspector/InspectorInstrumentation.cpp:972
> +    int id = 0;

can a worker ever have id 0?
it's better to set it to -1

> Source/WebCore/inspector/InspectorInstrumentation.cpp:977
> +    if (id && timelineAgent)

check for id > 0 if you set id = -1 as initial value.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:996
> +    int id = 0;

ditto

> Source/WebCore/inspector/InspectorInstrumentation.cpp:1001
> +    if (id && timelineAgent)

ditto

> Source/WebCore/inspector/InspectorWorkerAgent.cpp:86
> +    static int s_nextId;

I don't like the variable being public.  I'd rather see a static getter and static setter.

> Source/WebCore/inspector/InspectorWorkerAgent.cpp:-206
> -            return;

why did you remove the early return?  You should "return id;" here as well.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.