1438962 - Add diagnostic log to Browser Toolbox startup

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P3)

Description

[Brian Grinstead [:bgrins]]

I can't reproduce this locally, but worked with Nate (cc'ed) who could to narrow this down to the call to `yield DebuggerClient.socketConnect` here: https://searchfox.org/mozilla-central/rev/74b7ffee403c7ffd05b8b476c411cbf11d134eb9/devtools/client/framework/toolbox-process-window.js#47. We suspect this has something to do with no prompt showing up despite `devtools.debugger.prompt-connection` being true.

Still need to do further debugging but I suspect the issue can be reproduced locally by returning a promise that doesn't resolve inside that function. A couple other things we could do while we are here:

1) Convert toolbox-process-window to use async/await instead of Task
2) Provide some feedback in the UI on where in the connection process the Browser Toolbox is to make it easier for us to debug problems like this (i.e. show a message before each step in the connect function)

Comment 1

[drJeckyll]

I have same problem under Linux - just an empty window.

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to drJeckyll from comment #1)
> I have same problem under Linux - just an empty window.

Do you see a prompt asking for permission to connect when you open the Browser Toolbox? And what is the `devtools.debugger.prompt-connection` pref set to?

Comment 3

[drJeckyll]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> (In reply to drJeckyll from comment #1)
> > I have same problem under Linux - just an empty window.
> 
> Do you see a prompt asking for permission to connect when you open the
> Browser Toolbox? 
No
> And what is the `devtools.debugger.prompt-connection` pref
> set to?
devtools.debugger.prompt-connection: false

It's Firefox 60.0a1 (2018-02-20) (64-bit) under Gentoo Linux

Comment 4

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

It doesn't seem to me that we have enough data about the actual issue here yet...  For now, I've focused on improving the connection status in hopes of getting more info.

Comment 5

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

Attachment: Bug 1438962 - Convert Browser Toolbox window to async / await.

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

Comment 6

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

Attachment: Bug 1438962 - Add diagnostics during Browser Toolbox startup.

Track connection status at various points during Browser Toolbox startup.  If
connecting takes a long time or triggers an error, the status panel is revealed
with the most recent message displayed.

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

Comment 7

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

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

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8952640 [details]
Bug 1438962 - Convert Browser Toolbox window to async / await.

https://reviewboard.mozilla.org/r/221886/#review227868

Thanks!

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8952641 [details]
Bug 1438962 - Add diagnostics during Browser Toolbox startup.

https://reviewboard.mozilla.org/r/221888/#review227874

This will definitely help. We can ask Nate to re-test once this hits Nightly and spin off a new bug based on what status messages we see.

::: commit-message-8cab6:1
(Diff revision 1)
> +Bug 1438962 - Add diagnostics during BT startup. r=bgrins

Nit: "BT" -> "Browser Toolbox"

::: devtools/client/framework/toolbox-process-window.js:34
(Diff revision 1)
>  
>  var gToolbox, gClient;
>  
> -var connect = async function () {
> -  window.removeEventListener("load", connect);
> +function updateStatusMessage(msg) {
> +  let statusMessage = document.getElementById("status-message");
> +  statusMessage.value = msg;

I could see it being useful to append the status messages instead of wiping it out at each step so we could go back and get the connection details or other info when debugging an issue from a bug report.

Comment 10

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

Comment on attachment 8952641 [details]
Bug 1438962 - Add diagnostics during Browser Toolbox startup.

https://reviewboard.mozilla.org/r/221888/#review227874

> I could see it being useful to append the status messages instead of wiping it out at each step so we could go back and get the connection details or other info when debugging an issue from a bug report.

Good call, changed it to append instead.

Comment 11

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

Comment on attachment 8952640 [details]
Bug 1438962 - Convert Browser Toolbox window to async / await.

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

Comment 12

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

Comment on attachment 8952641 [details]
Bug 1438962 - Add diagnostics during Browser Toolbox startup.

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38c6b0c07b2a
Convert Browser Toolbox window to async / await. r=bgrins
https://hg.mozilla.org/integration/autoland/rev/6fd6890d798f
Add diagnostics during Browser Toolbox startup. r=bgrins

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/38c6b0c07b2a
https://hg.mozilla.org/mozilla-central/rev/6fd6890d798f

Comment 15

[drJeckyll]

Today build - in browser toolbox I can see only: Connecting to localhost:39243, ws: false