1492724 - [remote-dbg-next] Investigate connection issues with new ADB Extension when ADB Helper is still installed

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

Reported initially by :ochameau, we are not sure if this is a reproducible bug or just USB debugging being unstable.

Steps should be:
- setup a profile with ADB Helper installed (for instance by updating to the last revision before WebIDE switched to the new extension: https://hg.mozilla.org/mozilla-central/rev/ab213805a838 and doing some android debugging)
- update to latest central
- start web ide
- install new ADB extension
- try to debug USB device

Only occurred once, needs more investigations. Should be mitigated by Bug 1492374 which will remove adbhelper before installing the new extension.

Comment 1

[Alexandre Poirot [:ochameau]]

I just reproduced it again, when testing bug 's patch:
JavaScript error: resource://devtools/shared/base-loader.js -> resource://devtools/shared/adb/adb.js, line 103: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIProcess.init]

I do have "ADB binary provider" installed at version 0.0.2.

After some investigation, it may be a race.

In resource://devtools/shared/adb/adb.js
In start function:
  const adbFile = await this.adbFilePromise;
  process.init(adbFile);

`adbFile` is null
`adbFilePromise` is a cached promise (i.e. this._adbFilePromise)
And it looks like, when this promise is created and cached, `getFileBinary()` returned null.
But when I call this function from the debugger, while breaking in start function, it returns a valid file.

So I imagine start is being called too early for getFileBinary to succeed.

Comment 2

[Alexandre Poirot [:ochameau]]

Note that restarting firefox, fixes the issue. So it looks like it only fails the first time the extension is installed.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I will look into this tomorrow.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

I could reproduce this locally.  The error happened at the top of extractFiles(), here https://hg.mozilla.org/mozilla-central/file/ebeba937ca2a/devtools/shared/adb/adb-binary.js#l95 .

To reproduce this, we don't need to setup the old ADB helper, I just delete the profile directory for mach run, in my case that's obj-firefox/tmp/profile-default.  What I don't quit understand is that even if I delete the profile directory, the new extension there when I try ./mach run again.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> What I don't quit understand is that even
> if I delete the profile directory, the new extension there when I try ./mach
> run again.

This is wrong.  The new extension is installed when I open the WebIDE window, so I guess it's bit early to try to start adb?

Comment 6

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7efcdd0f51214497de9a1abd657effa68becb4e6
This should work, but I have no idea how to write a test for this.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1492724 - Add ADBScanner once after the extension is installed or is ready. r=jdescottes

And remove the ADBScanner once the extension is uninstalled.

When RuntimeScanners.add is called for ADBScanner, RuntimeScanner calls
ADBScanner.enable(), thus ADB.start() is called.  Meanwhile WebIDE installs the
devtools adb extension if the extension hasn't installed yet.  So if the
ADB.start() is called before the installation finishes, ADB.start() fails.

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 9010858 [details]
Bug 1492724 - Add ADBScanner once after the extension is installed or is ready. r=jdescottes

Julian Descottes [:jdescottes][:julian] has approved the revision.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41a10675b7e7
Add ADBScanner once after the extension is installed or is ready. r=jdescottes

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/41a10675b7e7