[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 #6 from Hanna <hanma at rim.com>  2012-07-10 13:06:24 PST ---
(From update of attachment 151496)
View in context: https://bugs.webkit.org/attachment.cgi?id=151496&action=review

fixing now

>> Source/WebCore/inspector/InspectorInstrumentation.cpp:982
>> +        timelineAgent->didCreateWorker(url, isSharedWorker);
> 
> You should pass the worker id as well so that we could navigate between worker records.

sure, except the id is the memory address

>> Source/WebCore/inspector/InspectorInstrumentation.cpp:1009
>> +    if (InspectorTimelineAgent* timelineAgent = instrumentingAgents->inspectorTimelineAgent())
> 
> Why aren't you using didDestroyWorker? It belongs to the legacy workers inspection (fake workers), but should still work.

didDestroyWorker is not called on our platform

>> Source/WebCore/inspector/InspectorInstrumentation.h:1267
>> +        didCreateWorkerImpl(instrumentingAgents, id, context->url().string(), isSharedWorker);
> 
> Why did this change? We don't make any calls from these inline public methods, all processing should be in impl.

turned out that the url passed in is different from context->url().string(), in order to match up with the worker terminated I changed it, but if this messes up other things (ie inspectorAgent->didCreateWorker(id, url, isSharedWorker);) I'll add a new method then pass in context->url().string()

>> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:523
>> +        if (this.type !== recordTypes.WorkerCreated && this.type !== recordTypes.WorkerTerminated) {
> 
> We already have atomic events with 0 duration that we report. Fixing them altogether should be handled in a separate patch.

got it

>> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:586
>> +            case recordTypes.WorkerTerminated:
> 
> ditto. Did you run this change?

it is supposed to fall through, since workercreated has both isSharedWorker and url, but I couldn't pass the isSharedWorker info to workerterminated easily

>> LayoutTests/inspector/timeline/timeline-web-workers.html:25
>> +    var sharedWorker = createSharedWorker();
> 
> Shared workers instantiation is browser-specific. Please make sure tests pass at least on Mac ports and for Chromium.

this method that I used is copied directly from LayoutTests/fast/workers for both sharedWorker and dedicatedWorker, should that be fine?

>> LayoutTests/inspector/timeline/timeline-web-workers.html:29
>> +    dedicatedWorker.postMessage("close");
> 
> Worker message processing is asynchronous to the inspector message interchange. By the time you leave this method and stop timeline, the worker is not necessarily stopped.

will change right away

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