1095145 - Debugger::onNewScript doesn't need its 'compileAndGoGlobal' argument

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

The 'compileAndGoGlobal' argument to Debugger::onNewScript is redundant, and should be removed.

Looking at the assertions at the top of Debugger::onNewScript:

/* static */ void
Debugger::onNewScript(JSContext *cx, HandleScript script, GlobalObject *compileAndGoGlobal)
{
    MOZ_ASSERT_IF(script->compileAndGo(), compileAndGoGlobal);
    MOZ_ASSERT_IF(script->compileAndGo(), compileAndGoGlobal == &script->uninlinedGlobal());
    ...
    MOZ_ASSERT_IF(!script->compileAndGo(), !compileAndGoGlobal);

From these it clearly follows that compileAndGoGlobal is always equal to:

  script->compileAndGo() ? script->uninlinedGlobal() : nullptr

and hence need not be passed to onNewScript, nor need onNewScript pass it to slowPathOnNewScript.

Comment 2

[Jim Blandy :jimb]

Attachment: Remove compileAndGoGlobal argument to Debugger::onNewScript, and simplify accordingly.

This takes care of bug 1064079 as well.

Comment 3

[Jim Blandy :jimb]

Try push:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=07e23242d743

Comment 4

[Boris Zbarsky [:bzbarsky]]

Jim, is this still happening?

Comment 5

[Jim Blandy :jimb]

Yeah, just got dropped. Let me give it a new push...

Comment 6

[Jim Blandy :jimb]

Attachment: Remove compileAndGoGlobal argument to Debugger::onNewScript, and simplify accordingly.

Patch refreshed, new try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=91a0a64c7fdf

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 8554625 [details] [diff] [review]
Remove compileAndGoGlobal argument to Debugger::onNewScript, and simplify accordingly.

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

Very nice.

::: js/src/vm/Debugger.cpp
@@ -1312,5 @@
> -    for (Debugger **p = src->begin(); p != src->end(); p++) {
> -        Debugger *dbg = *p;
> -        Value v = ObjectValue(*dbg->toJSObject());
> -        if (dbg->observesScript(script) && dbg->observesNewScript() &&
> -            (wasEmpty || Find(dest->begin(), dest->end(), v) == dest->end()) &&

I wonder why we ever needed to deduplicate the list of debuggers...

Comment 8

[Jim Blandy :jimb]

I wouldn't call that try push "clean", but nothing there seemed remotely related to this patch.

Comment 9

[Jim Blandy :jimb]

https://hg.mozilla.org/integration/mozilla-inbound/rev/00ec63159c11

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/00ec63159c11