[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 #30 from Pavel Feldman <pfeldman at chromium.org>  2012-07-27 12:42:30 PST ---
(From update of attachment 154987)
View in context: https://bugs.webkit.org/attachment.cgi?id=154987&action=review

>>> Source/WebCore/ChangeLog:8
>>> +        Test: inspector/timeline/timeline-web-workers.html
>> 
>> Please explain what happens and why.
>> 
>> I'd also be interested in learning more about the use cases. Is this a user request? There is a lot happening in the browser, not everything deserves to be on the timeline. Sorry for bringing it up so late, but I am sure you have some good answers to this. We do need to justify the code we are adding.
> 
> It is a user request

I'd like to challenge it. Is is mentioned in any public forum or a bug tracker?

>>> Source/WebCore/inspector/InspectorWorkerAgent.cpp:59
>>> +        , m_id(id)
>> 
>> Please roll this back.
> 
> there will be instances where the browser don't have workerfrontend

I see what you mean now.

>>> 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.
> 
> Please don't make random fields public.

I would move it into the InspectorWorkerAgent then.

> Source/WebCore/inspector/InspectorWorkerAgent.cpp:194
> +    m_dedicatedWorkers.set(workerContextProxy, std::make_pair(++WorkerFrontendChannel::s_nextId, url.string()));

It is cleaner to generate an id first and then to use it in two methods below.

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