Bug 148341 – Debugger's VM should never be null

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

Debugger's VM should never be null

[Geoffrey Garen]

Created attachment 259680 [details]
Patch

[Saam Barati]

Comment on attachment 259680 [details]
Patch

LGTM

[Joseph Pecoraro]

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]

Committed r188803: <http://trac.webkit.org/changeset/188803>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 148347

[Geoffrey Garen]

Committed r188841: <http://trac.webkit.org/changeset/188841>

[Csaba Osztrogonác]

(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]

(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