941012 - Firefox OS 1.2 Simulator is not connected

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Petruta Horea [:phorea], Desktop Test Engineering]

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

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Always use SDK loader to load DebuggerServer r=dcamp

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 3

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

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.

Comment 4

[Paul Rouget [:paul]]

Do we need to uplift that to beta?

Comment 5

[Alexandre Poirot [:ochameau]]

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.

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: 1.2 patch

I fixed the escodegen error and rebased it on 1.2.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: master patch - fixed escodegen ex

https://tbpl.mozilla.org/?tree=Try&rev=0d510eb10d3a

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: master patch - fixed ChromeWorker exception

https://tbpl.mozilla.org/?tree=Try&rev=22743421b007

Comment 9

[Alexandre Poirot [:ochameau]]

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]

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: 1.2 patch - with ChromeWorker fix

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

Comment 11

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

(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 12

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

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.

Comment 13

[Alexandre Poirot [:ochameau]]

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

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: 1.2 patch - fixed new escodegen issues

https://tbpl.mozilla.org/?tree=Try&rev=0ac077622b42

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/c50cca7d248a

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef84114446cc