Closed Bug 883342 Opened 11 years ago Closed 9 years ago

Breakpoint not triggering when closing/re-opening window

Categories

(DevTools :: Debugger, defect, P2)

x86
macOS
defect

Tracking

(Not tracked)

VERIFIED WORKSFORME

People

(Reporter: Fallen, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [chrome-debug])

Attachments

(1 file)

STR: 

1. Start browser debugger 
2. Open preferences window 
3. Set a breakpoint in one of the functions called (i.e in privacy.js) 
4. Close prefs window 
5. Open prefs window 
6. Trigger the breakpoint by clicking on things in the UI


Results: 

Execution is not halted at breakpoint.

Exepected:

Execution is halted at breakpoint.


I'm not sure if this is the same as bug 799070, but I think it would be good if the breakpoint is remembered in some form.
Are you using an optimized build? Because I'm hitting an assertion in debug builds.
Depends on: 875284
Whiteboard: [chrome-debug]
Yes, I'm using the standard nightly builds.
This happens with other windows as well:
1. Start browser debugger.
2. Open View Source.
3. Set a breakpoint in viewSource.js, in ViewSourceGotoLine().
4. In the View Source window, select menu Edit -> Go To Line to trigger the breakpoint. That works.
5. Close and reopen View Source, and select menu Edit -> Go To Line.

Expected:
Breakpoint triggered again.

Actual result:
Breakpoint is ignored.

Workaround:
Clear and set the breakpoint again.
Just tried to reproduce this and now the files mentioned above aren't even there in the script list. I don't know why, but viewSource.js for instance doesn't look like it could be GC'd while the view-source window is still open.
Priority: -- → P2
Bug 916845 blocks the browser debugger from working at all.
Depends on: 916845
This appears to happen in a very reproducible way when debugging certified apps. For example, when you attempt to debug pdfjs there are no scrips visible whatsoever, on browser (for 1.2 - I realise this is changing) many scripts load but browser.js does not.
This also happens when debugging conversation windows in the social api.
This makes debugging SocialAPI stuff particularly frustrating, as the chat windows tend to come and go.

I haven't verified this in the last little while, but at one point, after a bunch of experimentation, it appeared to me that the breakpoint did actually fire, but UI did not display it as having fired (ie no blue dot, viewport may or may not have scrolled to the right line), so it _appeared_ to not have fired.

Whether that observation is still the case when seeing the behavior described here (or is actually a separate bug), I'm not sure, but this is something to be on the lookout for.

In any case, fixing this bug would be a significant improvement in making SocialAPI code debuggable.
This also makes it impossible to debug startup/open code in browser windows, which is very frustrating for frontend dev in general.

Panos, do we know where the problem lies here? Is the underlying JSD2 API supposed to "just work" for pre-existing breakpoints if the same JS file is loaded into a new global?
Flags: needinfo?(past)
I haven't found the time to investigate this yet, but it seems that the root cause of the problem is that in some cases, like the view-source window for instance, we don't get onNewScript notifications from the debugger API about viewSource.js. It may be that we aren't adding the right global as a debuggee, but that seems hard to believe, since we use addAllGlobalsAsDebuggees() initially and then addDebuggee() on every global that appears through onNewGlobal notifications.

If we open the view-source window before opening the browser toolbox, then we can set and trigger breakpoints normally (which implies that addAllGlobalsAsDebuggees works fine), but not if view-source is closed and reopened. My best guess is that it's a platform issue, possibly somewhere around the code that calls onNewScript and/or onNewGlobal.

To answer your specific question, yes, the debugger server keeps a breakpoint store and anytime it sees a new script, it sets any stored breakpoints it has for that script. But in this case it doesn't seem to be notified when those scripts are loaded.
Flags: needinfo?(past)
So I spent today poking at this.

We get an onNewGlobal call as expected.

We don't get onNewScript calls for JS files loaded in the XUL window, AFAICT, but we *do* get them for e.g. XBL documents (toolbar.xml, popup.xml, etc.).

So I tried to poke at why. As best I can tell:

loading the XUL doc

http://mxr.mozilla.org/mozilla-central/source/content/xul/document/src/XULDocument.cpp#3046

calls into LoadScript:

XULDocument::LoadScript(nsXULPrototypeScript* aScriptProto, bool* aBlock)

which gets a scriptobject (presumably it's there already because it was already loaded in another window?) and calls ExecuteScript:

http://mxr.mozilla.org/mozilla-central/source/content/xul/document/src/XULDocument.cpp#3687

which calls

JS::CloneAndExecuteScript(cx, global, aScriptObject)

http://mxr.mozilla.org/mozilla-central/source/content/xul/document/src/XULDocument.cpp#3395

which, skipping a few more things called CloneScriptSomethingOrOther, ends up in:

js::CloneFunctionAndScript(JSContext *cx, HandleObject enclosingScope, HandleFunction srcFun)

at http://mxr.mozilla.org/mozilla-central/source/js/src/jsfun.cpp#482

calling the oldstyle (JSD1) new script hook (which isn't present), but not the new style one, and so we don't hear about the new script.

Jim, does that seem at all plausible?
Flags: needinfo?(jimb)
Huh, actually, thinking about it more, those are function scripts, so I guess it's expected there's no JSD2 new script notifications for them - but it seems like CloneScript ( http://mxr.mozilla.org/mozilla-central/source/js/src/jsscript.cpp#2866 ) also doesn't fire onNewScript - unlike CloneFunctionScript ( http://mxr.mozilla.org/mozilla-central/source/js/src/jsscript.cpp#3069 ). I don't know why.
Attached patch thing I triedSplinter Review
So here's what I tried. In XCode, I see all the right stuff being called now, for toplevel XUL-loaded urls... but my Cu.reportError in onNewScript is not firing. I don't understand why. I followed the onnewscript code all the way into fireNewScript up until it hits the Invoke call to actually call the callback (AFAICT?), and I can't make heads nor tails of what happens next. AFAICT the handleUncaughtException case is never hit there, so I'm at a loss as to why it's still not calling the onNewScript handler on the devtools side (NB: not sure that would immediately fix this issue, but it seems like we'll need to deal with this part first...).
I'll take a look at this this week.
Assignee: nobody → ejpbruel
Flags: needinfo?(jimb)
Summary: Breakpoint not triggered if window closed and re-opened → Breakpoint not hitting when window closed and re-opened
Summary: Breakpoint not hitting when window closed and re-opened → Breakpoint not triggering when closing/re-opening window
I'm not actively working on this at the moment, so unassigning myself, but I seem to recall that Alex had similar problems recently with missing onNewScript calls, which were caused by CloneScript not firing onNewScript notification. Alex, could this bug have the same underlying cause? And if so, could I convince you to look into it?
Assignee: ejpbruel → nobody
Flags: needinfo?(poirot.alex)
Given comment 11, I think jim's fix in attachment 8518564 [details] [diff] [review] fixed that.
Gijs, could you confirm this bug is fixed?
Otherwise no, I'm the right guy to debug this, I think Gijs already pointed out what was wrong in comment 11 and it is now matter of someone with JS API/Debugger knowledges to provide the right patch.
Flags: needinfo?(poirot.alex) → needinfo?(gijskruitbosch+bugs)
WFM on current Nightly.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(gijskruitbosch+bugs)
Resolution: --- → WORKSFORME
Thanks for checking, one of our contributors tested this with Thunderbird and it works as expected.
Status: RESOLVED → VERIFIED
Depends on: 1212099
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: