[Webkit-unassigned] [Bug 99801] Web Inspector: [JSC] implement WorkerScriptDebugServer

[bugzilla-daemon at webkit.org]

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


Yury Semikhatsky <yurys at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #169578|review?                     |review-
               Flag|                            |




--- Comment #2 from Yury Semikhatsky <yurys at chromium.org>  2012-10-19 02:46:40 PST ---
(From update of attachment 169578)
View in context: https://bugs.webkit.org/attachment.cgi?id=169578&action=review

Inspector part of the change looks good to me except a few minor things. I am pretty sure we should be able to treat page and worker debuggers similarly in Source/JavaScriptCore/debugger but I'm not an expert in JSC so you need someone working on JSC to review that part.

> Source/JavaScriptCore/parser/Parser.h:998
> +    if (debugger && !debugger->isWorkerDebugger() && !ParsedNode::scopeIsFunction)

I still don't understand why we should treat parse events differently in case of workers. We need someone familiar with JSC code to look at this part.

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:370
> +        dispatchDidParseSource(*listeners, sourceProvider, isWorkerDebugger() ? false : isContentScript(exec));

Please make isContentScript a virtual method on ScriptDebugServer and provide appropriate implementations for Page/WorkerScriptDebug instead of introducing isWorkerDebugger.

> Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:58
> +    // If JavaScript stack is not empty postpone recompilation.

It seems that we can safely assume the JS stack is empty if recompileAllJSFunctions is invoked by Timer but this is a different issue.

> Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:73
> +    recompileAllJSFunctions(0);

recompileAllJSFunctionsSoon

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