Bug 148341

Summary: Debugger's VM should never be null
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bburg, bfulgham, commit-queue, joepeck, ossy, peavo, timothy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 148347    
Bug Blocks:    
Attachments:
Description Flags
Patch joepeck: review+

Geoffrey Garen
Reported 2015-08-21 16:02:49 PDT
Debugger's VM should never be null
Attachments
Patch (13.23 KB, patch)
2015-08-21 16:05 PDT, Geoffrey Garen 		joepeck: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2015-08-21 16:05:24 PDT
Created attachment 259680 [details] Patch
Saam Barati
Comment 2 2015-08-21 16:07:40 PDT
Comment on attachment 259680 [details] Patch LGTM
Joseph Pecoraro
Comment 3 2015-08-21 16:19:37 PDT
Comment on attachment 259680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259680&action=review r=me > Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.cpp:31 > +#include "JSGlobalObject.h" I'm not sure this include is needed. There is no new code in this file. > Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.cpp:77 > void JSGlobalObjectScriptDebugServer::recompileAllJSFunctions() > { > - JSC::Debugger::recompileAllJSFunctions(&m_globalObject.vm()); > + JSC::Debugger::recompileAllJSFunctions(); > } Seems you can just remove this virtual method, it just calls its superclass. > Source/JavaScriptCore/inspector/ScriptDebugServer.h:121 > -#endif // ScriptDebugServer_h > +#endif // ScriptDebugSer*er_h Oops > Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp:87 > JSC::JSLockHolder lock(vm); In WorkerScriptDebugServer and PageScriptDebugServer these could both be simplified if they just used (m_vm) instead of re-looking up the vm, that would require an accessor on Debugger. Up to you.
Geoffrey Garen
Comment 4 2015-08-21 16:57:14 PDT
Committed r188803: <http://trac.webkit.org/changeset/188803>
WebKit Commit Bot
Comment 5 2015-08-21 17:49:31 PDT
Re-opened since this is blocked by bug 148347
Geoffrey Garen
Comment 6 2015-08-23 14:28:48 PDT
Committed r188841: <http://trac.webkit.org/changeset/188841>
Csaba Osztrogonác
Comment 7 2015-08-23 14:47:54 PDT
(In reply to comment #6) > Committed r188841: <http://trac.webkit.org/changeset/188841> It broke the WinCairo build: https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/48606
Csaba Osztrogonác
Comment 8 2015-08-24 02:21:33 PDT
(In reply to comment #7) > (In reply to comment #6) > > Committed r188841: <http://trac.webkit.org/changeset/188841> > > It broke the WinCairo build: > https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/48606 build log: c:\users\alex\documents\wincairobot\win-cairo-release\build\source\webcore\inspector\PageScriptDebugServer.cpp(57): error C2661: 'Inspector::ScriptDebugServer::ScriptDebugServer': no overloaded function takes 2 arguments c:\users\alex\documents\wincairobot\win-cairo-release\build\source\webcore\inspector\PageScriptDebugServer.cpp(90): error C3861: 'vm': identifier not found c:\users\alex\documents\wincairobot\win-cairo-release\build\source\webcore\inspector\PageScriptDebugServer.cpp(91): error C2660: 'JSC::Debugger::recompileAllJSFunctions': function does not take 0 arguments c:\users\alex\documents\wincairobot\win-cairo-release\build\source\webcore\inspector\PageScriptDebugServer.cpp(57): error C2661: 'Inspector::ScriptDebugServer::ScriptDebugServer': no overloaded function takes 2 arguments c:\users\alex\documents\wincairobot\win-cairo-release\build\source\webcore\inspector\PageScriptDebugServer.cpp(90): error C3861: 'vm': identifier not found c:\users\alex\documents\wincairobot\win-cairo-release\build\source\webcore\inspector\PageScriptDebugServer.cpp(91): error C2660: 'JSC::Debugger::recompileAllJSFunctions': function does not take 0 arguments
Note You need to log in before you can comment on or make changes to this bug.