1275942 - Merge startup debugging support from Positron branch

Status: RESOLVED FIXED

(DevTools :: General, enhancement)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Over in the Positron branch, we added support platform debugging (the equivalent of Browser Toolbox) to pause the platform when --jsdebugger is passed to aid in debugging startup issues.

This should be generally useful to Firefox developers as well.

This bug should include some kind of squashed version of the changes from:

https://github.com/mozilla/positron/pull/41
https://github.com/mozilla/positron/pull/43
https://github.com/mozilla/positron/pull/54

Comment 1

[Myk Melez [:myk] [@mykmelez]]

@jryans: This is another one of those Positron DevTools enhancements that it would be helpful to upstream.  Is it something you can tackle, or shall I take it on?

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I'll take it on, thanks for the reminder!

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1275942 - Add option to wait for debugger on startup.

Review commit: https://reviewboard.mozilla.org/r/116794/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116794/

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

I think this is an accurate rebase of the Positron code, but it doesn't seem as useful in Firefox so far...  I tried to test it out in various startup files, but command line handler runs too late in startup, so the startup modules have already loaded by the time we pause.

Is it useful for you in this state?

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

I tried it in Firefox, adding a debugger statement to the app-startup/profile-after-change observer in TelemetryStartup.js, and that observer got called before Firefox could pause to await the debugger connection.

I also tried it in the nsDefaultCLH.handle() method in nsDefaultCLH.js, and it broke into the debugger as expected (although the initial browser window also appeared and functioned, so I guess nsICommandLineHandler.handle's caller doesn't wait for it to return).  It was also effective at the top of browser.js, breaking into the debugger and stalling the browser window.

I suppose its utility depends on how early the startup code runs.  For some Firefox use cases, it'll work.  And it also works for my use case.  So I think it's a worthy enhancement.

NB: Chrome appears to have the equivalent, according to http://peter.sh/experiments/chromium-command-line-switches/#wait-for-debugger, except that it only waits for up to 60 seconds.  I don't think a timeout is particularly necessary, although I'm willing to entertain it.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Okay, cool.  It does work for me as well when I try in browser.js.

I think I'll rename the option to "wait-for-jsdebugger", that seems a bit more explicit about the purpose.  (It seems like Chrome's "wait-for-debugger" is for a native code debugger, like gdb / LLDB.)

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116794/diff/1-2/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2493b3fc1d777ebd750eed1be6dad740f8378fe

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116794/diff/2-3/

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116794/diff/3-4/

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=03b951e954ca822e8e3c182822ff087921b8809a

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

https://reviewboard.mozilla.org/r/116794/#review119216

Looks good! I don't really know much about hiddenDOMWindow to comment on this part, but the rest looks good to me.
Just one question: is there any reason to not make this the default behavior when using --jsdebugger? Should it even be an option?

::: devtools/client/devtools-startup.js:226
(Diff revision 4)
>  
>    /* eslint-disable max-len */
>    helpInfo: "  --jsconsole        Open the Browser Console.\n" +
>              "  --jsdebugger       Open the Browser Toolbox.\n" +
> +            "  --wait-for-jsdebugger Spin event loop until JS debugger connects.\n" +
> +            "                     Enables debugging (some) application startup code paths.\n" +

handleDebuggerFlag is only called if the --jsdebugger flag is on. This means that --wait-for-jsdebugger requires --jsdebugger to be on.
Either:
- mention it in the helpInfo for --wait-for-jsdebugger
- call handleDebuggerFlag as well if --wait-for-jsdebugger is on

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

https://reviewboard.mozilla.org/r/116794/#review119216

> handleDebuggerFlag is only called if the --jsdebugger flag is on. This means that --wait-for-jsdebugger requires --jsdebugger to be on.
> Either:
> - mention it in the helpInfo for --wait-for-jsdebugger
> - call handleDebuggerFlag as well if --wait-for-jsdebugger is on

I was worried that `--wait-for-jsdebugger` might slow down some workflows, but testing again here, I don't really notice a slow down.  So for now, I'll remove the option and have this always enabled with `--jsdebugger`.

Comment 14

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

https://reviewboard.mozilla.org/r/116794/#review119216

> I was worried that `--wait-for-jsdebugger` might slow down some workflows, but testing again here, I don't really notice a slow down.  So for now, I'll remove the option and have this always enabled with `--jsdebugger`.

Hmm, actually I've changed my mind.  I think it could be confusing for some people if they use `--jsdebugger` to access the inspector because this change will make it default to the hidden window instead of the first browser window.  (You can still switch to the first browser window in frames, but it's probably not obvious.)

So, to avoid disturbing workflows, I'll keep the extra option for this, and note in the help that you need `--jsdebugger` too.

Comment 15

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8843039 [details]
Bug 1275942 - Add option to wait for debugger on startup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116794/diff/4-5/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/783dcc2af468
Add option to wait for debugger on startup. r=jdescottes

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/783dcc2af468

Comment 18

[Julian Descottes [:jdescottes]]

Adding dev-doc-needed, we should add an entry to https://developer.mozilla.org/en-US/docs/Mozilla/Command_Line_Options describing the new "--wait-for-jsdebugger" command line option.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Posted to various mailing lists:

https://groups.google.com/d/topic/mozilla.dev.developer-tools/xIM8TxAcCjk/discussion