Closed Bug 608763 Opened 9 years ago Closed 9 years ago

JSD breakpoint does not work

Categories

(Core :: JavaScript Engine, defect)

1.9.2 Branch
x86
Windows 2000
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: Honza, Assigned: sayrer)

References

Details

(Keywords: relnote, Whiteboard: [firebug-p1] fixed-in-tracemonkey)

Attachments

(2 files)

Attached file Test XPI
Registered breakpoint hook (jsd.breakpointHook) is not executed.
Tested with: http://hg.mozilla.org/mozilla-central/rev/f4ea6992e1c6

STR:
1) Install the attached extension
2) Load www.google.com
3) Open the Tools -> error console
4) You should see a log coming from the breakpoint handler, but it's not there.
Something like: "onBreakpoint: <script-name> <line>"

Note that a breakpoint is set at the first line of every created script (see the XPI for more details):

onScriptCreated: function(script)
{
    script.setBreakpoint(0);
}

It works in 3.6.12

You want to set following prefs:
javascript.options.showInConsole = true;
devtools.errorconsole.enabled = true;

Honza
blocking2.0: --- → ?
Whiteboard: [firebug-p1]
Thanks for the clear and simple STR!

My current understanding is that this has to block b7.
Assignee: general → sayrer
Keywords: relnote
Punting to b8
blocking2.0: ? → beta8+
Can we get someone to confirm this behavior?  Andreas, can you reproduce this?
blocking2.0: beta8+ → ?
Damon asked QA to confirm this on the trunk. Using Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101101 Firefox/4.0b8pre, when I follow the instructions in Comment 0 I do not see anything being logged to the console, except for this:

Error: debugger: false
Source File: chrome://simpledebugger/content/debugger.js
Line: 141

I tested using 3.6.12 and I see the logging in the console.
(In reply to comment #4)
> Damon asked QA to confirm this on the trunk. Using Mozilla/5.0 (Windows NT 5.1;
> rv:2.0b8pre) Gecko/20101101 Firefox/4.0b8pre, when I follow the instructions in
> Comment 0 I do not see anything being logged to the console, except for this:
> 
> Error: debugger: false
> Source File: chrome://simpledebugger/content/debugger.js
> Line: 141

You should also see the following:
Error: debugger activated: true
Source File: chrome://simpledebugger/content/debugger.js
Line: 141

Note that the example xpi is using new API: "jsd.asyncOn" (coming from bug 595243).

Honza
Yes, in my testing of yesterday's build using 3.6.12 I did see what you describe in the bottom portion of Comment 5 - Error: debugger activated: true.

(In reply to comment #5)
> (In reply to comment #4)
> > Damon asked QA to confirm this on the trunk. Using Mozilla/5.0 (Windows NT 5.1;
> > rv:2.0b8pre) Gecko/20101101 Firefox/4.0b8pre, when I follow the instructions in
> > Comment 0 I do not see anything being logged to the console, except for this:
> > 
> > Error: debugger: false
> > Source File: chrome://simpledebugger/content/debugger.js
> > Line: 141
> 
> You should also see the following:
> Error: debugger activated: true
> Source File: chrome://simpledebugger/content/debugger.js
> Line: 141
> 
> Note that the example xpi is using new API: "jsd.asyncOn" (coming from bug
> 595243).
> 
> Honza
(In reply to comment #2)
> Punting to b8

b8+
blocking2.0: ? → beta8+
so far, I have enough data with this patch to show that all relevant contexts are being recompiled with debugMode, and that setBreakpoint is indeed being called on new scripts. Digging deeper in SetBreakpoint.
I spoke with Honza on the phone earlier today and he told me that the impact of this bug is that the script tab in Firebug isn't going to be populated properly because Firebug internally sets a breakpoint at the beginning of each script for the purpose of gathering information.

I don't think that view of why this bug is important is expressed in the description or comments above.
Hold up b7 for a bit.  This may be worse than we thought.  dvander and sayre
are looking into this.  Will update as soon as we know more.

b7+
blocking2.0: beta8+ → beta7+
The bug is straightforward, I think. I naively assumed that setting debugMode in the compartment constructor would work... but I think there must be other creation sites I need hit. SetBreakpoint isn't working because JS_SetTrap is finding a cx that is not in debugMode.
JSD makes its own "jsdc->dumbContext" JSContext... this might not be the best idea. That thing gets created in ActivateDebugger, so the JSRuntime needs to be in debug mode before we call that. 

However, this short term fix seems to fix the problem with setting breakpoints.
Comment on attachment 487730 [details] [diff] [review]
Put the runtime in debugMode prior to calling ActivateDebugger

r=gal over my shoulder
Attachment #487730 - Flags: review+
Depends on: 609141
Drive-by: someone hacked compartment awareness in but didn't reuse a local that caches a memory value:

JSBool
jsd_IsValueNative(JSDContext* jsdc, JSDValue* jsdval)
{
    JSContext* cx = jsdc->dumbContext;
    jsval val = jsdval->val;
    JSFunction* fun;
    JSExceptionState* exceptionState;
    JSCrossCompartmentCall *call = NULL;

    if(jsd_IsValueFunction(jsdc, jsdval))
    {
        JSBool ok = JS_FALSE;
        JS_BeginRequest(cx);
        call = JS_EnterCrossCompartmentCall(jsdc->dumbContext, // SHOULD BE cx!
                                            JSVAL_TO_OBJECT(val));
        if(!call) {
            JS_EndRequest(cx);

            return JS_FALSE;
        }
 . . .

Bigger non-nit: going by my very old memories, jsdc->dumbContext is what the in-process debugger runs on, not the JS scripts being debugged. Do we really need it to have debugMode enabled in its compartment, or is there some other dependency at work here (either on that compartment, or on the dumbContext but for the wrong reason)?

/be
http://hg.mozilla.org/tracemonkey/rev/1d0e006769b0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
blocking2.0: beta7+ → ---
OS: Windows Vista → Windows 2000
Resolution: FIXED → ---
Whiteboard: [firebug-p1] → [firebug-p1] fixed-in-tracemonkey
I noticed that the blocking2.0 flag is not set. Is the fix still planned for b7?
If not, it means that b7 will not support JS debugging in Firebug.

Honza
blocking2.0: --- → ?
I think Rob accidentally reset the flag when he set fixed-in-tracemonkey.  Was beta7+
blocking2.0: ? → beta7+
Depends on: 609363
(In reply to comment #17)
> I think Rob accidentally reset the flag when he set fixed-in-tracemonkey.  Was
> beta7+

so can Rob correctly set the flag this time and post the "correct" status of this bug as soon as possible?
He already did, the status is beta7+ as shown above.
http://hg.mozilla.org/mozilla-central/rev/a9def7f8c835
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Tested with Firebug, works with the last nightly.
Great work Robert, thanks!

To reply on a comment in bug 595743:
https://bugzilla.mozilla.org/show_bug.cgi?id=595743#c1
> we should reevaluate Firebug bugs after something like my patch in bug 595243
> has been applied and used.

So here are the next things:
- Single stepping is not implemented (related to: Bug 595743)
- Bug 549617 regression

Do you want me to create a new bug report for the single-stepping?
Honza
(In reply to comment #21)
> Do you want me to create a new bug report for the single-stepping?

Please create new bugs for any remaining issues.
Bug 609663 - JSD doesn't support single stepping
Bug 609668 - Regression of Bug 549617: JSD doesn't see variables in flat closure

Honza
You need to log in before you can comment on or make changes to this bug.