Firefox OS 1.2 Simulator is not connected

RESOLVED FIXED in Firefox 28

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: petruta.rasa, Assigned: ochameau)

Tracking

Trunk
Firefox 28
x86_64
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [simulator][good first verify][needs-coverage])

Attachments

(2 attachments, 4 obsolete attachments)

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
Duplicate of this bug: 941011
Whiteboard: [simulator]
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
Assignee: nobody → poirot.alex
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.
Attachment #8336446 - Flags: review+
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.
Posted patch 1.2 patch (obsolete) — Splinter Review
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]
Attachment #8338451 - Flags: review?(jryans)
(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.
Attachment #8338451 - Flags: review?(jryans) → review+
Checking needed for attachment 8338451 [details] [diff] [review] on master.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/c50cca7d248a
Keywords: checkin-needed
Whiteboard: [simulator] → [simulator][fixed-in-fx-team]
This was backed out and re-landed while investigating a test failure.
https://hg.mozilla.org/integration/fx-team/rev/ef84114446cc
https://hg.mozilla.org/mozilla-central/rev/ef84114446cc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [simulator][fixed-in-fx-team] → [simulator]
Target Milestone: --- → Firefox 28
Depends on: 946813
Whiteboard: [simulator] → [simulator][good first verify]
Whiteboard: [simulator][good first verify] → [simulator][good first verify][needs-coverage]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.