Closed Bug 1528912 Opened 9 months ago Closed 8 months ago

[remote-dbg-next] Review the ADB lifecycle and related issues

Categories

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

enhancement

Tracking

(firefox68 fixed)

RESOLVED FIXED
Firefox 68
Tracking Status
firefox68 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

(Whiteboard: [remote-debugging-reserve])

Attachments

(1 file)

At the moment we are trying to stop the ADB process as soon as Firefox no longer needs it, if the ADB process was started by the current instance of Firefox. However this has several shortcomings:

  • if you have a single tab of about:debugging and reload it, ADB will be successively stopped and started, terminating all established connections with USB devices
  • if you opened about:devtools-toolbox for targets on USB devices and close about:debugging, the connections will be terminated, and about:devtools-toolbox tabs will go into error mode
  • if 2 Firefox instances are started and are both opening about:debugging, the second one will not be starting ADB because it is already started. If the user closes about:debugging in the first Firefox, it will kill all connections in the second firefox as well

There are probably other situations where stopping ADB can be an issue, this is not an exhaustive list. We could complexify the logic around stopping ADB to take care of those issues (wait for some time before killing the process, registering more listeners etc...) but I think it would make more sense to only stop ADB when we close Firefox. So I would like to propose this change here.

It will not completely solve the last presented scenario, if you close the first Firefox, ADB connections would still be stopped on the second Firefox. But I think this is an edge case, and we can compromise it by restarting ADB if we detect that the existing ADB is closing.

Heh, as I was reading the beginning of the description, I was thinking "maybe we should wait until Firefox is closed to stop ADB". I think that we're agreeing on it :)

One note though, we're not the only consumer of ADB. For example there are android utils that use ADB to pull/push files to devices (file explorers), and if you're an Android developer you might also want to use ADB to look at logfiles.

In my experience, these utilities only care about the ADB service being running. If it's not running, they'll try to start it... and forget about it. Maybe we want to do the same. What is the rationale behind wanting to stopping it, other than cleaning up after ourselves? Are there problems caused by not stopping the service?

I admit I don't have any good reason other than "I thought it would be better not to leave ADB running if we started it". Some of the existing code was trying to do that (although unsuccessfully), so I assumed that was the behavior we wanted.

From an about:debugging developer's perspective, never stopping ADB seems like the easiest option. I wonder if there is someone we could ask to know what is acceptable for Firefox when it comes to spawning other processes.

Priority: -- → P3

At the moment we are not sure who to ask for guidance on this topic, but as :ladybenko mentioned in our standup, we should prioritize improving the quality of life of our users. We would only start ADB when users start about:debugging with USB debugging enabled anyway, so regular users should not be impacted.

Blocks: 1514787

(In reply to Julian Descottes [:jdescottes] from comment #3)

At the moment we are not sure who to ask for guidance on this topic, but as :ladybenko mentioned in our standup, we should prioritize improving the quality of life of our users. We would only start ADB when users start about:debugging with USB debugging enabled anyway, so regular users should not be impacted.

I don't think it's too bad to leave adbd running after the first usage. If other apps on the system try to use adb and have a different version they'll typically just kill it and start their own (Android Studio).

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [remote-debugging-reserve]

It looks like we don't need to stop ADB. Most of the logic can stay, in order to start and stop the devices polling.

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bca50b46b48
Do not stop ADB when closing aboutdebugging or webide;r=daisuke
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.