Closed Bug 1084955 Opened 5 years ago Closed 5 years ago

JavaScript errors in adb.js and missing AdbController

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, b2g-v2.1 unaffected, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S7 (24Oct)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- unaffected
b2g-v2.2 --- verified

People

(Reporter: gerard-majax, Assigned: gerard-majax)

References

Details

(Keywords: regression, Whiteboard: [systemsfe])

Attachments

(1 file, 1 obsolete file)

I fear this may be a regression from bug 1072625

On a Gecko synced and built now, I see those errors in logcat:

> W/GeckoConsole( 1744): [JavaScript Error: "TypeError: redeclaration of const Cu" {file: "chrome://b2g/content/devtools/adb.js" line: 11}]
> W/GeckoConsole( 1744): [JavaScript Error: "ReferenceError: AdbController is not defined" {file: "chrome://b2g/content/devtools/debugger.js" line: 251}]
Flags: needinfo?(dhylands)
Keywords: regression
The Cu const is already defined by settings.js when adb.js is included
by shell.html. We should not redefine it.
Attachment #8507349 - Attachment is obsolete: true
Comment on attachment 8507354 [details] [diff] [review]
Remove redeclaration of Cu r=fabrice

As far as I can tell, in shell.html the very first file to be included is settings.js which already defines the Cu const at http://hg.mozilla.org/mozilla-central/annotate/ae1dfa192faf/b2g/chrome/content/settings.js#l11.
Attachment #8507354 - Flags: review?(fabrice)
Attachment #8507354 - Flags: review?(fabrice) → review+
Keywords: checkin-needed
Assignee: nobody → lissyx+mozillians
Whiteboard: [systemsfe]
Target Milestone: --- → 2.1 S7 (24Oct)
Looks like you found the cause - sorry about that.
Flags: needinfo?(dhylands)
[Blocking Requested - why for this release]:

We are losing ADB access as soon as we flip the adb on setting, and thus need to hard reboot to pick up the changes.  Until then, adb can't be seen, and this is breaking our QA lab setup (since we reflash and reboot all the time).  Please uplift this patch asap to 2.1.

Thanks,
Tony
blocking-b2g: --- → 2.1?
Comment on attachment 8507354 [details] [diff] [review]
Remove redeclaration of Cu r=fabrice

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1072625
User impact if declined: no more adb for QA
Testing completed: on device, this has been fixing broken adb for all people applying
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Attachment #8507354 - Flags: approval-mozilla-b2g34?
Attachment #8507354 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
blocking-b2g: 2.1? → 2.1+
Bug 1072625 never landed on v2.1. Is it intended to at some point? Otherwise, why the blocking status? :)
Flags: needinfo?(lissyx+mozillians)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> Bug 1072625 never landed on v2.1. Is it intended to at some point?
> Otherwise, why the blocking status? :)

Tony asked for uplift in comment 6.
Flags: needinfo?(lissyx+mozillians)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> Bug 1072625 never landed on v2.1. Is it intended to at some point?
> Otherwise, why the blocking status? :)

There's literally nothing to uplift in this bug without bug 1072625 being present.
Flags: needinfo?(tchung)
Yeah and I don't see any reason to uplift bug 1072625. It doesn't fix anything, just a bit of code cleanup.
:dave, i guess i was under the impression from fabrice that this would fix adb detection, as its regressed upon setting.  if i'm incorrect, can you educate on what broke that and what would fix it?  something is causing our post device flashing to lose adb connectivity until it's physically rebooted, and that wont work for our automation setup.
Flags: needinfo?(tchung) → needinfo?(dhylands)
tchung: on what device? What build?

I'm not seeing adb dropping on the flame.
Flags: needinfo?(dhylands)
(In reply to Tony Chung [:tchung] from comment #6)
> [Blocking Requested - why for this release]:
> 
> We are losing ADB access as soon as we flip the adb on setting, and thus
> need to hard reboot to pick up the changes.  Until then, adb can't be seen,
> and this is breaking our QA lab setup (since we reflash and reboot all the
> time).  Please uplift this patch asap to 2.1.
> 
> Thanks,
> Tony

So I'm really confused by this. You saying you lose adb when you enable it?

If it was disabled, then how did you have an adb connection ithe first place (if you're running a user build).

Are you running eng builds? Why is adb disabled?

So you're seeing this on 2.1? Do you see the same behaviour on master?
Flags: needinfo?(tchung)
(In reply to Dave Hylands [:dhylands] from comment #15)
> (In reply to Tony Chung [:tchung] from comment #6)
> > [Blocking Requested - why for this release]:
> > 
> > We are losing ADB access as soon as we flip the adb on setting, and thus
> > need to hard reboot to pick up the changes.  Until then, adb can't be seen,
> > and this is breaking our QA lab setup (since we reflash and reboot all the
> > time).  Please uplift this patch asap to 2.1.
> > 
> > Thanks,
> > Tony
> 
> So I'm really confused by this. You saying you lose adb when you enable it?
> 
> If it was disabled, then how did you have an adb connection ithe first place
> (if you're running a user build).
> 
> Are you running eng builds? Why is adb disabled?
> 
> So you're seeing this on 2.1? Do you see the same behaviour on master?

Hi David,

okay it sounds like you werent aware of what we're seeing.  this was on 2.1, and here's what happens:

Manual Repro:
1) check that adb only is on, and adb devices finds the phone
2) flash.sh base v180
3) confirm adb should be still be active 
4) run flash_pvt.py and full flash the 2.1 production build
5) goto settings > developer, and select ADB and DevTools box
6) verify adb is lost after selecting.  i had to physically reboot the phone
Flags: needinfo?(tchung)
I discovered another issue with adb on the flame, which is in bug 1088494.

That particular bug affects full-flash user builds.

I don't understand your step 6 above. You said:

> 6) verify adb is lost after selecting.  i had to physically reboot the phone

You had to physically reboot the phone why? To verify adb was lost. Or to recover?

In step 4 you said you flashed the production build. Did you mean User build? I don't see anywhere in flash_pvt to choose production build.
Flags: needinfo?(tchung)
I did a full flash using ./flash_pvt.py (with no options).

I chose the following options:
Device: flame-kk
Branch: mozilla-b2g34_v2_1
Build: User
Flash: images

It chose this URL: https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-b2g34_v2_1-flame-kk/latest/flame-kk.zip

And adb seems to work properly.

Were you using the same URL?
I was finally able to reproduce the problem.

Performing a Lock/Unlock fixed it for me (I didn't have to reboot). I think that this may be duplicate of bug 1088494. Let me see if I can reproduce the problem with a local build.

The problem only seems to happen during the first session (FTU through first lock) after the flash_pvt.py.

If I go through a Lock/Unlock cycle after FTU but before enabling ADB for the first time, then everything seems to behave properly.
I'm able to reproduce this locally as well, and it doesn't seem to be related to bug 1088494. Investigating....
I opened bug 1089028, since that seems to be the root cause.
Verified the issue is fixed on 2.2 Flame
Followed by steps in Comment 16 a couple times. adb is didn't loose and able to detect the device

Device: Flame 2.2 Master KK
BuildID: 20141105160209
Gaia: 7918024c737c4570cacd784f267e28737ae05dea
Gecko: 2114ef80f6ae
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Attachment #8507354 - Flags: approval-mozilla-b2g34+
Flags: needinfo?(chung808)
You need to log in before you can comment on or make changes to this bug.