J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Description • 8 years ago

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

	Myk Melez [:myk] [@mykmelez]
	Comment 1 • 7 years ago

@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?

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 2 • 7 years ago

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

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 4 • 7 years ago

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?

	Myk Melez [:myk] [@mykmelez]
	Comment 5 • 7 years ago

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.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 6 • 7 years ago

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.)

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 8 • 7 years ago

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

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 11 • 7 years ago

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

	Julian Descottes [:jdescottes]
	Comment 12 • 7 years ago
mozreview-review

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

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 13 • 7 years ago
mozreview-review-reply

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`.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 14 • 7 years ago
mozreview-review-reply

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.

	Pulsebot
	Comment 16 • 7 years ago

Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/783dcc2af468
Add option to wait for debugger on startup. r=jdescottes

	Carsten Book [:Tomcat]
	Comment 17 • 7 years ago
bugherder

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

	Julian Descottes [:jdescottes]
	Comment 18 • 7 years ago

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.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 19 • 7 years ago

Posted to various mailing lists:

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