WorkerScriptDebugServer is declared but not implemented in jsc, without it the worker inspector frontend is faulty
*** Bug 92652 has been marked as a duplicate of this bug. ***
The bug 95334 and 95341 are the existing defects that cause the worker inspector for JSC unable to work, so I put them in the independent bug record to make it easier to be reviewed. The patch for implementation of "WorkerScriptDebugServer" will be uploaded soon.
Created attachment 161450 [details] Patch
Comment on attachment 161450 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161450&action=review > Source/JavaScriptCore/parser/Parser.h:1039 > + if (debugger && !debugger->isWorkerDebugger() && !ParsedNode::scopeIsFunction) How does inspector get notified about parsed scripts in case of worker if is is skipped here? > Source/WebCore/bindings/js/ScriptDebugServer.cpp:447 > +void ScriptDebugServer::blockToWaitInstruction(bool& stop) I'd call it runEventLoopWhilePaused > Source/WebCore/bindings/js/ScriptDebugServer.h:117 > + virtual void blockToWaitInstruction(bool& stop); m_doneProcessingDebuggerEvents is a protected field, no need to pass a reference to it as an argument. > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:71 > + m_workerContext->script()->attachDebugger(this); This should happen only when first listener is added. We should probably simplify the API as I am not aware of any cases when we may have more than one listener. > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:81 > + m_workerContext->script()->detachDebugger(this); This should only occur when there are no listeners. > Source/WebCore/bindings/js/WorkerScriptDebugServer.h:48 > + bool isWorkerDebugger() { return true; } This method should be marked virtual.
(In reply to comment #4) > (From update of attachment 161450 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161450&action=review > > > Source/JavaScriptCore/parser/Parser.h:1039 > > + if (debugger && !debugger->isWorkerDebugger() && !ParsedNode::scopeIsFunction) > > How does inspector get notified about parsed scripts in case of worker if is is skipped here? The "JSC::WorkerScriptDebugServer::addListener" invokes "recompileAllJSFunctions" which will make "JSC::Debugger::sourceParsed" to be called. Actually, if we don't skip here, worker inspector will have a problem in some case(refer to the attachement). > > Source/WebCore/bindings/js/ScriptDebugServer.cpp:447 > > +void ScriptDebugServer::blockToWaitInstruction(bool& stop) > > I'd call it runEventLoopWhilePaused ok. > > Source/WebCore/bindings/js/ScriptDebugServer.h:117 > > + virtual void blockToWaitInstruction(bool& stop); > > m_doneProcessingDebuggerEvents is a protected field, no need to pass a reference to it as an argument. ok, thx. > > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:71 > > + m_workerContext->script()->attachDebugger(this); > > This should happen only when first listener is added. We should probably simplify the API as I am not aware of any cases when we may have more than one listener. yes, but JSC really needs interface "addListener" to inform of recompilation. > > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:81 > > + m_workerContext->script()->detachDebugger(this); > > This should only occur when there are no listeners. ok > > Source/WebCore/bindings/js/WorkerScriptDebugServer.h:48 > > + bool isWorkerDebugger() { return true; } > > This method should be marked virtual. ok.
Created attachment 165974 [details] The test case why we need to skip "sourceParsed" in "Parser.h"
Created attachment 165975 [details] Snapshot of of the worker inspector problem caused by "sourceParsed" in "Parser.h"
Created attachment 165978 [details] Patch
However, the subject of this bug is confused, so I open a new bug#99801 to make it clear that what I'm going to do. So this bug should be closed.
Comment on attachment 165978 [details] Patch Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.
*** This bug has been marked as a duplicate of bug 164136 ***