Summary: | Pass debuggerTaskMode as a parameter in WorkerScriptDebugServer constructor | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Seokju Kwon <seokju> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, apavlov, haraken, japhet, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Seokju Kwon
2012-12-15 01:36:33 PST
Created attachment 179582 [details]
Patch
yurys : What do you think about it? Comment on attachment 179582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179582&action=review > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:96 > + result = m_workerContext->thread()->runLoop().runInMode(m_workerContext, WorkerScriptDebugServer::debuggerTaskMode); We should leave only one of WorkerDebuggerAgent::debuggerTaskMode and WorkerScriptDebugServer::debuggerTaskMode and remove other one. It'd prefer leaving WorkerDebuggerAgent::debuggerTaskMode but WorkerScriptDebugServer should not depend on WorkerDebuggerAgent as dependency is in the opposite direction. To overcome this we way pass debuggerTaskMode as a parameter in WorkerScriptDebugServer constructor. Created attachment 179937 [details]
Patch
(In reply to comment #3) > (From update of attachment 179582 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179582&action=review > > > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:96 > > + result = m_workerContext->thread()->runLoop().runInMode(m_workerContext, WorkerScriptDebugServer::debuggerTaskMode); > > We should leave only one of WorkerDebuggerAgent::debuggerTaskMode and WorkerScriptDebugServer::debuggerTaskMode and remove other one. It'd prefer leaving WorkerDebuggerAgent::debuggerTaskMode but WorkerScriptDebugServer should not depend on WorkerDebuggerAgent as dependency is in the opposite direction. To overcome this we way pass debuggerTaskMode as a parameter in WorkerScriptDebugServer constructor. As you said, I have fixed them. Comment on attachment 179937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179937&action=review > Source/WebCore/bindings/js/WorkerScriptDebugServer.h:45 > + WorkerScriptDebugServer(WorkerContext*, const String); const String -> const String& > Source/WebCore/bindings/v8/WorkerScriptDebugServer.h:50 > + WorkerScriptDebugServer(WorkerContext*, const String); const String -> const String& (In reply to comment #6) > const String -> const String& Apart from that looks good to me. Created attachment 180118 [details]
Patch
Comment on attachment 180118 [details]
Patch
Fixed it. Thank you for your review.
Comment on attachment 180118 [details] Patch Clearing flags on attachment: 180118 Committed r138135: <http://trac.webkit.org/changeset/138135> All reviewed patches have been landed. Closing bug. |