Open Bug 1535470 Opened 5 years ago Updated 2 years ago

[remote-dbg-next] make `connect` the default page for runtimes

Categories

(DevTools :: about:debugging, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: ogasidlo, Unassigned, Mentored)

References

(Blocks 1 open bug)

Details

As we changed the order of our pages in the sidebar from this-firefox to connect, Daisuke suggested for https://phabricator.services.mozilla.com/D18588 to change the default page to connect when we disconnect from the remote runtime.

After changing the route in App.js to connect in
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/components/App.js#99

and

https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/components/App.js#120

and fixing 3 tests related to the default page path, another test failed and surfaced an issue that is not related to my patch.

https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_persist_connection.js

terminal log

 1:34.46 FAIL Test timed out - 
 1:34.61 GECKO(2745) console.warn: "Error while detaching the thread front: 'detach' request packet to 'server1.conn0.child1/context22' can't be sent as the connection is closed."
 1:34.77 INFO Removing tab.
 1:34.77 INFO Waiting for event: 'TabClose' on [object XULElement].
 1:34.79 INFO Got event: 'TabClose' on [object XULElement].
 1:34.81 GECKO(2745) console.log: MOCKED METHOD removeUSBRuntimesObserver
 1:34.81 INFO Tab removed and finished closing
 1:34.88 GECKO(2745) MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
 1:34.88 GECKO(2745) MEMORY STAT | vsize 6837MB | residentFast 661MB | heapAllocated 300MB

console errors

Warning: Failed prop type: Invalid prop `usbRuntimes[0].extra.connectionParameters` supplied to `App`.
Warning: Failed prop type: Invalid prop `usbRuntimes[0].extra.connectionParameters` supplied to `Sidebar`.

I walked through the code and my path lead me to following issues:

https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/modules/client-wrapper.js#135

Where we try to list all addons, which we do not have on the connect page.

This is where it fails:
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/actions/debug-targets.js#160

https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/actions/debug-targets.js#196

https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/actions/debug-targets.js#140

We do not have a runtime defined.

Daisuke, I'd love to discuss this a little bit more.

Flags: needinfo?(dakatsuka)

Thanks Ola.
We are showing connect as initial page when open about:debugging-new.
Thus, I am thinking connect page is default page already.
So, I thought, when disconnect the runtime in bug 1505128, better redirect to connect page.
Rather, we might should redirect to / in disconnecting case at least. Then, this of App.js handles to render default page.
What do you think?

Flags: needinfo?(dakatsuka)

Daisuke and I just had a chat.

Here we need to use dispatch(Actions.selectPage(PAGE_TYPES.CONNECT))

To make this work, we also need to modify selectPage()
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/actions/ui.js#34

As selectPage() requires a runtimeId, we can not provide e.g. in the case the device was disconnected (see bug 1505128).
So, every time we call selectPage() without an existing or valid runtimeId, we should redirect to the connect page aka default page.

  • Please note:
    We might also have touch the functions in debug-targets.js mentioned in the first comment as they also just check if we are not on the this-firefox page. This worked well, when the default page was this-firefox. But since we changed this, we need to also check for a runtime at all.

Related console.error message to that is:

[ACTION FAILED] REQUEST_EXTENSIONS_FAILURE: Connection closed, pending request to root, type listAddons failed
Summary: [remote-dbg-next] default page should not render extension, tab and worker list per default → [remote-dbg-next] make `connect` the default page
Summary: [remote-dbg-next] make `connect` the default page → [remote-dbg-next] make `connect` the default page for runtimes
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.