1084955 - JavaScript errors in adb.js and missing AdbController

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[:gerard-majax]

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}]

Comment 1

[:gerard-majax]

Attachment: Remove redeclaration of Cu r=fabrice

Comment 2

[:gerard-majax]

Attachment: Remove redeclaration of Cu r=fabrice

The Cu const is already defined by settings.js when adb.js is included
by shell.html. We should not redefine it.

Comment 3

[:gerard-majax]

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.

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/eb9c0b46ccf5

Comment 5

[Dave Hylands [:dhylands]]

Looks like you found the cause - sorry about that.

Comment 6

[Tony Chung [:tchung]]

[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

Comment 7

[:gerard-majax]

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

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Bug 1072625 never landed on v2.1. Is it intended to at some point? Otherwise, why the blocking status? :)

Comment 10

[:gerard-majax]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Dave Hylands [:dhylands]]

Yeah and I don't see any reason to uplift bug 1072625. It doesn't fix anything, just a bit of code cleanup.

Comment 13

[Tony Chung [:tchung]]

: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.

Comment 14

[Dave Hylands [:dhylands]]

tchung: on what device? What build?

I'm not seeing adb dropping on the flame.

Comment 15

[Dave Hylands [: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?

Comment 16

[Tony Chung [: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

Comment 17

[Dave Hylands [:dhylands]]

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.

Comment 18

[Dave Hylands [:dhylands]]

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?

Comment 19

[Dave Hylands [:dhylands]]

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.

Comment 20

[Dave Hylands [:dhylands]]

I'm able to reproduce this locally as well, and it doesn't seem to be related to bug 1088494. Investigating....

Comment 21

[Dave Hylands [:dhylands]]

I opened bug 1089028, since that seems to be the root cause.

Comment 22

[sarsenyev]

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