Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 Reproducible on latest Beta Fx 26 beta 6 (20131118212339), latest Aurora (20131120004002), latest Nightly (20131120030202). Steps to reproduce: 1. Open App Manager 2. Start the Firefox 1.2 OS Simulator Actual Results: "Connecting to.." is displayed on the bottom bar for a few seconds, the Simulator window is opened then in the console "disconnected" is shown. The string on the bottom bar returns to "Not Connected. No device found. Plug a device or Start Simulator". The window remains open. Expected Results: The Simulator is connected, on the console the "connected" string is displayed. Notes: - We didn't encounter this issue using Firefox OS 1.3 Simulator - It is not reproducible when using the app manager for the first time - Intermittently the issue reproduces also on MAC OS X and Ubuntu 13.10

More or less a revert of bug 911127, but fixed against b2g. It end up fixing this issue, where DebuggerServer fails to load on 1.2, except on first run (!!). The loadSubScript code is quite complex and end up having same issue than when using SDK loader on non-first run. I haven't looked at precisely why, but, on b2g, main.js sandbox has a b2g-jsm like behavior. Its global object is different than `this` in top level scope. So that, when we do `var DebuggerServer = ...` in top level scope, it isn't set on `this`. (i.e. this.DebuggerServer is undefined) That's why we had the sdk loader exception, even with: '({components : Components}) = require("chrome"); It won't overload this.Components, so that transport.js will still use sdk loader Components.utils. We can fix that by doing: Object.definedProperty(this, "Components", {get: function () require("chrome").components}); (defineProperty, as SDK loader se definedProperties to set Components.) Then, the bad part of this patch is that we have to do similar trick for all symbols used by scripts loaded with loadSubScript (DebuggerServer, dumpn, SourceMap*,...). I haven't exported escodegen as I haven't seen any usage?? (ideally, all dependencies would explicitely load what it uses!) https://tbpl.mozilla.org/?tree=Try&rev=4a579d1d35e3

Comment on attachment 8336446 [details] [diff] [review] Always use SDK loader to load DebuggerServer r=dcamp Review of attachment 8336446 [details] [diff] [review]: ----------------------------------------------------------------- Now that I've looked at the patch, this change is clearer than when we talked earlier, so I am fine with reviewing this. The try results suggest to me that you probably do need to export escodegen, so take a look at that. Also, please test the original use case that we changed this to support, namely if you start the browser debugger, ensure you are able to debug an actor. r+ assuming you look into those things.

Do we need to uplift that to beta?

Yes we will need uplift. I volontary kept this patch short and simple. But be carefull, I only tested that on b2g-simulator master, that's why I haven't asked for the review yet.

I fixed the escodegen error and rebased it on 1.2.

Comment on attachment 8338451 [details] [diff] [review] master patch - fixed ChromeWorker exception Review of attachment 8338451 [details] [diff] [review]: ----------------------------------------------------------------- My tree doesn't want to compile for b2g on device, could you give a try to this patch and bless it again? I added two additional fixes after your review. 1) rebases the patch against last master that doesn't uses escodegen anymore (and fixed escodegen in 1.2 patch) 2) Injects ChromeWorker in all devtools sdk modules (I need to update 1.2 patch accordingly) [tbpl is now green]

And TBPL run: https://tbpl.mozilla.org/?tree=Try&rev=620dd0838e24

(In reply to Alexandre Poirot (:ochameau) from comment #10) > Created attachment 8338516 [details] [diff] [review] > 1.2 patch - with ChromeWorker fix > > And TBPL run: https://tbpl.mozilla.org/?tree=Try&rev=620dd0838e24 Seems like this one still has some escodegen issues.

Comment on attachment 8338451 [details] [diff] [review] master patch - fixed ChromeWorker exception Review of attachment 8338451 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. I've tested this locally on my Keon with 1.3 (2013-11-26), and it seems to work well.

Checking needed for attachment 8338451 [details] [diff] [review] on master.

This was backed out and re-landed while investigating a test failure. https://hg.mozilla.org/integration/fx-team/rev/ef84114446cc