1031881 - Most members of JSDebugHooks should be deleted

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

Now that JSD is gone, all but one member of JSDebugHooks are unused. They should be removed, along with the code that supports them.

Comment 1

[Jim Blandy :jimb]

Attachment: Remove unused elements of JSDebugHooks, and their supporting code.

Here's a patch that removes everything but JSDebugHooks::debuggerHandler and its closure pointer. I think I've chased down all the stuff that exists only to support those hooks.

A lovely diffstat:

 b/js/public/OldDebugAPI.h                                  |  109 --------
 b/js/src/builtin/Eval.cpp                                  |    2 
 b/js/src/frontend/BytecodeCompiler.cpp                     |   23 -
 b/js/src/frontend/BytecodeCompiler.h                       |    8 
 b/js/src/frontend/BytecodeEmitter.cpp                      |    2 
 b/js/src/jit/BaselineFrame.cpp                             |    5 
 b/js/src/jit/BaselineFrame.h                               |   18 -
 b/js/src/jit/BaselineJIT.cpp                               |   14 -
 b/js/src/jit/VMFunctions.cpp                               |    7 
 b/js/src/jsapi-tests/testDebugger.cpp                      |  171 -------------
 b/js/src/jsapi-tests/testSourcePolicy.cpp                  |   23 -
 b/js/src/jscntxt.cpp                                       |   25 -
 b/js/src/jsfun.cpp                                         |   21 -
 b/js/src/jsscript.cpp                                      |   29 --
 b/js/src/jsscript.h                                        |   13 
 b/js/src/shell/js.cpp                                      |   24 -
 b/js/src/vm/HelperThreads.cpp                              |   33 --
 b/js/src/vm/Interpreter.cpp                                |   10 
 b/js/src/vm/OldDebugAPI.cpp                                |  124 ---------
 b/js/src/vm/Stack-inl.h                                    |   32 --
 b/js/src/vm/Stack.cpp                                      |    1 
 b/js/src/vm/Stack.h                                        |   37 --
 js/src/jit-test/tests/jaeger/bug563000/test-throwhook-1.js |   19 -
 js/src/jit-test/tests/jaeger/bug563000/test-throwhook-2.js |   14 -
 24 files changed, 13 insertions(+), 751 deletions(-)

Comment 2

[Jim Blandy :jimb]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=754e64d79e9c

Comment 3

[Jim Blandy :jimb]

Attachment: Remove unused elements of JSDebugHooks, and their supporting code.

Rats, that patch dropped calls to clearTraps. Here's another attempt.

Try: https://tbpl.mozilla.org/?tree=Try&rev=a26a22287052

Perhaps I should go to bed...

Comment 4

[Jim Blandy :jimb]

Try push looks good.

Comment 5

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8447721 [details] [diff] [review]
Remove unused elements of JSDebugHooks, and their supporting code.

Review of attachment 8447721 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay, I somehow didn't notice this review request. And it was therapeutic reading all of the removed code.

r=me with the redundant stepModeEnabled stuff removed. Also, I'll give this an r+ if you remove the redundant stepModeEnabled stuff.

::: js/src/frontend/BytecodeCompiler.cpp
@@ -180,5 @@
> -frontend::MaybeCallSourceHandler(JSContext *cx, const ReadOnlyCompileOptions &options,
> -                                 SourceBufferHolder &srcBuf)
> -{
> -    JSSourceHandler listener = cx->runtime()->debugHooks.sourceHandler;
> -    void *listenerData = cx->runtime()->debugHooks.sourceHandlerData;

Oh, so debugHooks.sourceHandler is kinda like onBeforeCompileSource or whatever it's called? Huh.

::: js/src/jit/VMFunctions.cpp
@@ +891,3 @@
>  
> +    if (script->stepModeEnabled()) {
> +        if (script->stepModeEnabled())

Uh... you never can be too sure?

::: js/src/vm/Interpreter.cpp
@@ +1524,3 @@
>              RootedValue rval(cx);
>              JSTrapStatus status = JSTRAP_CONTINUE;
> +            if (script->stepModeEnabled())

That darn step mode. Keeps flapping back and forth.

Comment 7

[Jim Blandy :jimb]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a19c530f3d0

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9a19c530f3d0