Closed Bug 1255266 Opened 5 years ago Closed 4 years ago

WebIDE won't work for B2G on non-debug build when 'initializing USB devtools' error happens

Categories

(Firefox OS Graveyard :: Developer Tools, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
Tracking Status
firefox48 --- fixed

People

(Reporter: selee, Assigned: schien)

Details

Attachments

(1 file, 2 obsolete files)

When the following log is shown by adb logcat, the device can not been connected to WebIDE.

I/GeckoDump(  707): Error while initializing USB devtools: [Exception... "Illegal value"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://devtools/shared/Loader.jsm :: <TOP_LEVEL> :: line 75"  data: no]
I/GeckoDump(  707): @undefined:75:NaN
I/GeckoDump(  707): @resource://devtools/shared/Loader.jsm:75:10
I/GeckoDump(  707): XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:198:21
I/GeckoDump(  707): @resource://devtools/server/actors/utils/TabSources.js:13:13
I/GeckoDump(  707): @resource://devtools/server/actors/webbrowser.js:16:22
I/GeckoDump(  707): DebuggerServer.registerModule@resource://devtools/server/main.js:327:17
I/GeckoDump(  707): DebuggerServer.addBrowserActors@resource://devtools/server/main.js:381:5
I/GeckoDump(  707): RemoteDebugger.initServer@chrome://b2g/content/devtools/debugger.js:188:5
I/GeckoDump(  707): USBRemoteDebugger.start@chrome://b2g/content/devtools/debugger.js:251:5
I/GeckoDump(  707): @chrome://b2g/content/devtools/debugger.js:369:35


We found WebIDE via USB still worked with this Gecko commit:
Gecko: c2a8d865ec9da9f8df75cc7853d92f6936d3dedb
Hello S.C.,

We had a quick talk about this issue last week. Could you help to give your suggestion here? Thank you!
Flags: needinfo?(schien)
Looks related to :ochameau's recent loader changes.
Flags: needinfo?(poirot.alex)
We only observe this error on Nexus Player, flame-kk works fine. I'll try setup the Nexus Player environment tomorrow to get more information.
Flags: needinfo?(schien)
Summary: WebIDE won't work for B2G when 'initializing USB devtools' error happens → WebIDE won't work for B2G on Nexus Player when 'initializing USB devtools' error happens
Hum. I'm not sure.
It fails with "URL" lazy getter which has been introduced recently in bug 1247192.
Could you try to revert e47fd356ef3da318fefa38dfc890c546035a16af to see if that fixes this issue?
Flags: needinfo?(poirot.alex)
It fails here:
http://mxr.mozilla.org/mozilla-central/source/devtools/shared/Loader.jsm#75
XPCOMUtils.defineLazyGetter(loaderModules, "URL", () => {
  return Cu.Sandbox(this, {wantGlobalProperties: ["URL"]}).URL;
});
When TabSources.js does:
  require("URL");

But it's not clear why would this lazygetter fail and especially fail just on one device?!!
I discovered that this bug happened on non-debug build but not device-related. flame-kk and nexues player will both success while adding |export B2G_DEBUG=1| in .userconfig, and fail if B2G_DEBUG=0.

I found the error is in side the construction of Sandbox. The native object doesn't have nsIPrincipal interface so it failed at https://dxr.mozilla.org/mozilla-central/source/js/xpconnect/src/Sandbox.cpp#1261
Summary: WebIDE won't work for B2G on Nexus Player when 'initializing USB devtools' error happens → WebIDE won't work for B2G on non-debug build when 'initializing USB devtools' error happens
Sorry my bad, the error in Sandbox.cpp is this line: https://dxr.mozilla.org/mozilla-central/source/js/xpconnect/src/Sandbox.cpp#1254 (GetNativeOfWrapper returns a nullptr).
BTW, I tried print the "this" object in Loader.jsm (see comment #5) and it shows "[object FakeBackstagePass]". Not sure if it is the expected object.
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #8)
> BTW, I tried print the "this" object in Loader.jsm (see comment #5) and it
> shows "[object FakeBackstagePass]". Not sure if it is the expected object.

Ok, on debug build we get different "this" object. It prints out "[object BackstagePass @ 0xec0438e0 (native @ 0xec043740)]".

The behavior difference for debug build and non-debug build is in [1], which was landed by bug 1073094 long time ago.

I tried disable reuse-loader-global on non-debug build and I can get WebIDE connected. @khuey are you aware of any recent change that could affect reuse-loader-global behavior?

[1] https://dxr.mozilla.org/mozilla-central/source/js/xpconnect/loader/mozJSComponentLoader.cpp#301
Flags: needinfo?(khuey)
Workaround by disabling reuse-loader-global.
http://hg.mozilla.org/mozilla-central/annotate/d6ee82b9a741/devtools/shared/Loader.jsm#l39

On line 65 'this' is the FakeBackstagePass, so getting the principal fails as per comment 7.  Maybe it needs to be more like line 43's Sandbox?
Flags: needinfo?(khuey)
Attached patch fix-sandbox-principal.patch (obsolete) — Splinter Review
Thanks @khuey, I can connect to WebIDE with this patch. Need further test.
Attachment #8730597 - Attachment is obsolete: true
You probably want the system principal instead of the null principal there.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #13)
> You probably want the system principal instead of the null principal there.

Both system principal and null principal can work for solving this issue. I thought we should pick the minimal privilege, no?

Might need more background knowledge for the JS module loading mechanism in order to understand my impact of this patch. :)
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #14)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #13)
> > You probably want the system principal instead of the null principal there.
> 
> Both system principal and null principal can work for solving this issue. I
> thought we should pick the minimal privilege, no?
> 
> Might need more background knowledge for the JS module loading mechanism in
> order to understand my impact of this patch. :)

All of the modules loaded by the actual loader infrastructure in Loader.jsm (aside from these few special modules being tweaked in this bug) are loaded using the system principal by default[1].  Loader.jsm is effectively a "loader for privileged DevTools code", so it seems simplest to continue supplying the system principal where an explicit principal is needed.

I am also unsure if null vs. system principal has an actual material / behavioral difference for the specific cases under discussion, but I would say just go with system for consistency in this case, since everything else that wants to use these also does so.

[1]: https://dxr.mozilla.org/mozilla-central/rev/5e14887312d4523ab59c3f6c6c94a679cf42b496/addon-sdk/source/lib/toolkit/loader.js#234
Attachment #8730670 - Attachment is obsolete: true
Comment on attachment 8731109 [details]
MozReview Request: Bug 1255266 - create sandbox with system principal. r=khuey, jryans.

https://reviewboard.mozilla.org/r/40361/#review37045

I detest mozreview, but for you I'll make an exception. r=me
Attachment #8731109 - Flags: review?(khuey) → review+
Assignee: nobody → schien
https://hg.mozilla.org/mozilla-central/rev/57a4d8b83db2
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.