1492700 - [remote-dbg-next] Fix the timing of enable/disable ADB

Status: RESOLVED FIXED

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

Description

[Daisuke Akatsuka (:daisuke)]

Currently, we enable/disable ADB when about:debugging page was opened/closed (bug 1405235).
However, if user is opening another about:debugging page or another application is using, we should not disable ADB.
The other hand, if another application made ADB disable, we should enable again.
So, we need to consider the timing of enable/disable ADB.

Comment 1

[Julian Descottes [:jdescottes]]

The status for this one changed a bit. Now when about:debugging starts, we attempt to start ADB if needed (when we start the scanner, if the ADB extension is installed). And when we stop about:debugging we just destroy the scanner but we don't try to shutdown ADB.

I think that for m1 the current behavior is ok, and we should not shutdown ADB automatically when closing about:debugging. ADB is a process, we should not assume about:debugging is the only consumer. However, maybe it could be interesting to let the user know that the ADB process is currently running and give them a way to stop it?

Comment 2

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 3

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 4

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 5

[Julian Descottes [:jdescottes]]

Technically we should implement a mechanism to stop ADB if the process was started by firefox and if no client uses it anymore. 

I think we should also discuss if the status of ADB should be visible somewhere in the UI. It might be interesting for troubleshooting purposes. I think we should handle this in a separate bug.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Fix ADB stop() in case ADB start() was called several times;r=daisuke

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Stop sending adb-addon updates from Devices.jsm;r=daisuke

Depends on D12758

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Move Devices.jsm to devtools/shared/adb folder;r=daisuke

Depends on D12759

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Convert Devices.jsm to adb-devices-registry module;r=daisuke

Depends on D12760

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - adb-device.js should export AdbDevice and not Device;r=daisuke

Depends on D12761

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Stop exposing shell() on adb-device because it only forwards to ADB;r=daisuke

Depends on D12762

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Fold FirefoxOnAndroidRuntime.detect in scanner.detectRuntimes;r=daisuke

Depends on D12763

Comment 13

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Merge Runtime and FirefoxForAndroidRuntime into AdbRuntime;r=daisuke

Depends on D12764

Comment 14

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Extract AdbRuntime out of adb-scanner.js;r=daisuke

Depends on D12765

Comment 15

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Migrate AdbRuntime to a class;r=daisuke

Depends on D12766

Comment 16

[Julian Descottes [:jdescottes]]

I am trying to cleanup a bit our ADB modules in the scope of this bug. The reason behind is that if we want to stop the ADB process when all the "devtools" consumers of ADB no longer want to get ADB updates, then it means we need a clear entry point for all the interaction with ADB. 

In practice, we need to have some global module that will keep track of the consumers, and that will stop the ADB process if the number of consumers is down to 0 _and_ if ADB was started by devtools.

At the moment, all the consumers of ADB are using the "addon-aware-adb-scanner". This means that we could quickly hack something on top of this module to achieve the feature of this bug. However some other modules are also relying on ADB: about:devtools-toolbox when connected to a remote USB runtime for instance. Those modules don't need to scan for ADB runtimes, they just to say that they "need" ADB.

So I would like to cleanup the adb folder so that we can have small public API:
- adb-addon.js 
- adb.js

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5f900b18101
Fix ADB stop() in case ADB start() was called several times;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/5199632a4ada
Stop sending adb-addon updates from Devices.jsm;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/7590eb03b4ff
Move Devices.jsm to devtools/shared/adb folder;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/363e8b2ecfab
Convert Devices.jsm to adb-devices-registry module;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/65825c93b47b
adb-device.js should export AdbDevice and not Device;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/d5e67b8fbde3
Stop exposing shell() on adb-device because it only forwards to ADB;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/5db9614af6a5
Fold FirefoxOnAndroidRuntime.detect in scanner.detectRuntimes;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/bbc5bcb72208
Merge Runtime and FirefoxForAndroidRuntime into AdbRuntime;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/21f0a93a65c2
Extract AdbRuntime out of adb-scanner.js;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/ad28397dfa43
Migrate AdbRuntime to a class;r=daisuke

Comment 18

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/f5f900b18101
https://hg.mozilla.org/mozilla-central/rev/5199632a4ada
https://hg.mozilla.org/mozilla-central/rev/7590eb03b4ff
https://hg.mozilla.org/mozilla-central/rev/363e8b2ecfab
https://hg.mozilla.org/mozilla-central/rev/65825c93b47b
https://hg.mozilla.org/mozilla-central/rev/d5e67b8fbde3
https://hg.mozilla.org/mozilla-central/rev/5db9614af6a5
https://hg.mozilla.org/mozilla-central/rev/bbc5bcb72208
https://hg.mozilla.org/mozilla-central/rev/21f0a93a65c2
https://hg.mozilla.org/mozilla-central/rev/ad28397dfa43

Comment 19

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Split adb.js in several files;r=daisuke

Comment 20

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Migrate adb/commands/track-devices to a Class;r=daisuke

Depends on D13473

Comment 21

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Remove unused observables in commands/track-devices;r=daisuke

Depends on D13474

Comment 22

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Rename adb.js to adb-process.js and switch to class;r=daisuke

Depends on D13475

Comment 23

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Introduce adb singleton to register adb consumers;r=daisuke

Depends on D13476

Comment 24

[Julian Descottes [:jdescottes]]

Attachment: Bug 1492700 - Add mochitest to check that ADB is stopped after closing about:debugging;r=daisuke

Depends on D13477

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d8c5a3fc14e
Split adb.js in several files;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/41384e20308d
Migrate adb/commands/track-devices to a Class;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/271eb6988c58
Remove unused observables in commands/track-devices;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/c804e90f0ce5
Rename adb.js to adb-process.js and switch to class;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/738cae90db60
Introduce adb singleton to register adb consumers;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/ec57d6145de7
Add mochitest to check that ADB is stopped after closing about:debugging;r=daisuke

Comment 26

[Julian Descottes [:jdescottes]]

Attachment: fix_eslint_failure.patch

Fix eslint failure.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2aa7b7e89e14
Fix eslint failure in test_adb.js;r=fix

Comment 28

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/9d8c5a3fc14e
https://hg.mozilla.org/mozilla-central/rev/41384e20308d
https://hg.mozilla.org/mozilla-central/rev/271eb6988c58
https://hg.mozilla.org/mozilla-central/rev/c804e90f0ce5
https://hg.mozilla.org/mozilla-central/rev/738cae90db60
https://hg.mozilla.org/mozilla-central/rev/ec57d6145de7
https://hg.mozilla.org/mozilla-central/rev/2aa7b7e89e14