1206746 - Module `./adb-socket` is not found, can't connect to USB device in Nightly 43.0a1 (2015-09-21)

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[kdavis]

I am on OS X 10.10.5 and have a USB connected Flame-KK device that
shows up using

adb devices

I open the WebIDE in  Nightly 43.0a1 (2015-09-21) and the device
is not listed.

I subsequently close Nightly 43.0a1 (2015-09-21) and start an old
version of Firefox 37.0.2 and attempt the same, to connect to the
device using WebIDE. This works.

Using Nightly 43.0a1 (2015-09-21) and attempting to connect to the
device using WebIDE the browser console contains the stack trace....

Module `./adb-socket` is not found at resource://adbhelperatmozilla.org/adb-socket.js: @resource://adbhelperatmozilla.org/adb-client.js:13:23
@resource://adbhelperatmozilla.org/adb-running-checker.js:16:16
adb_start@resource://adbhelperatmozilla.org/adb.js:104:5
onADBStart@resource://adbhelperatmozilla.org/main.js:41:3
EventEmitter_emit@resource://gre/modules/devtools/event-emitter.js:147:11
enable@resource:///modules/devtools/webide/runtimes.js:304:5
_enableScanner@resource:///modules/devtools/webide/runtimes.js:169:5
enable@resource:///modules/devtools/webide/runtimes.js:164:7
exports.AppManager.init@resource:///modules/devtools/webide/app-manager.js:60:5
ProjectList@resource:///modules/devtools/webide/project-list.js:36:3
@chrome://webide/content/project-listing.js:11:19

Comment 1

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

I've heard that main culprit is TCPSocket changes to WebIDL (bug 885982).

Comment 2

[Alexandre Poirot [:ochameau]]

:gerard-majax already debugged this exception and the SDK loader is hidding the real exception here :/

This error is related to bug 885982.

https://github.com/mozilla/adbhelper/blob/master/adb-socket.js#L21-L25
  let scope = Cu.Sandbox(Services.scriptSecurityManager.getSystemPrincipal());
  scope.DOMError = Cu.import('resource://gre/modules/Services.jsm').DOMError;
  Services.scriptloader.loadSubScript("resource://gre/components/TCPSocket.js", scope);
  scope.TCPSocket.prototype.initWindowless = function () true;
  return new scope.TCPSocket();

We were fetching a TCPSocket instance from a Sandbox,
but it looks like, now, with the WebIDL, we have to set the related pref AND have a document instance.
That is quite unfortunate, especially the pref.

http://mxr.mozilla.org/mozilla-central/source/dom/webidl/Navigator.webidl#439

Josh, Do you think we can easily fetch a TCPSocket instance from chrome code (without setting the pref)?

Comment 3

[Josh Matthews [:jdm]]

Not as the code exists today. It may be possible to change the WebIDL files to use `Func=ShouldTCPSocketExist` instead of `Pref=` and `CheckAll=`, where ShouldTCPSocketExist would check the following:
- is the global using a system principal (something like http://mxr.mozilla.org/mozilla-central/source/dom/base/URL.cpp#161 and calling nsContentUtils::IsSystemPrincipal)
- is the preference enabled
- does CheckAnyPermissions return true (https://dxr.mozilla.org/mozilla-central/source/dom/bindings/BindingUtils.cpp?from=checkanypermissions#2427)

Andrew, you made a comment about socket.jsm in https://bugzilla.mozilla.org/show_bug.cgi?id=885982#c54 but I don't see that file anywhere.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

comm-central: https://dxr.mozilla.org/comm-central/source/chat/modules/socket.jsm

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Alexandre Poirot [:ochameau] from comment #2)
> That is quite unfortunate, especially the pref.

Note that bug 1079648 covers flipping the pref on Firefox and includes some minimal discussion of removing the preference in general.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 18

Here is a patch to use WebIDL's TCPSocket.
It requires a firefox build with bug 1207090's patch applied.
It also cleans up adb.js from duplicated code with adb-client.js.
(I made two distinct commits to help the review)

Comment 8

[Alexandre Poirot [:ochameau]]

I'm now waiting for bug 1207090's review before proceeding.

Comment 9

[Alexandre Poirot [:ochameau]]

Comment on attachment 8664769 [details] [review]
Pull request 18

I just pushed a new revision, with a bug fixed on release (was returning new TCPSocket() result instead of socket.open() in release-TCPSocket.jsm codepath!)

Platform patch re-landed, hopefully it will stick this time.
Asking review early in order to help releasing the addon ASAP.

Comment 10

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

Comment on attachment 8664769 [details] [review]
Pull request 18

Looks good to me! I'll test and release once the `TCPSocket` change lands in Firefox.

Comment 11

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

Merged: https://github.com/mozilla/adbhelper/commit/558d60630ec1d1ad0d7dab883b6fa74d611d7728

Preparing a new add-on release now.

Comment 12

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

ADB Helper 0.8.1 submitted to AMO, waiting on approval.

Comment 13

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

ADB Helper 0.8.1 has been released.  It may take a few hours for the update to be available due to CDN caching.

Comment 14

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

As a side note, only 44 will work for the moment.  43 still needs bug 1207090 to be uplifted.