945469 - [B2G][Usage][Cost Control] Completing usage setup does not transition into usage graph page

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[gbennett]

Description:
Selecting the "Let's Go" button to complete Usage setup does not transition into the usage graph page but instead turns grey, but still completes the setup itself as if you look at the Usage bar dropdown everything is setup and you can update it to see a usage limit (as seen in the youtube video).

Repro Steps:
1) Updated Buri 1.3 mozRIL to BuildID: 20131202092621
2) Launch usage app
3) Go through usage setup

Actual:
Selecting "Let's Go" does not transition into the usage graph page.

Expected:
Selecting "Let's Go" transitions into the usage graph page.

Notes: As shown in the video (sorry about accidentally launching the video app in the video, no idea how that happened) one is able to get to the graph section by task manager closing the usage app and going back into it, but the usage graph area will be blank.

Repro frequency: 100%, 3/3
See attached: https://www.youtube.com/watch?v=o1_Z3uujyZo


.:Last Working Build:.
Environmental Variables:
Device: Buri 1.3 mozRIL
BuildID: 20131201040201
Gaia: f23aebebcd28c4d19347def77c4836c8baebc2ce
Gecko: 84a5a5800bd3
Version: 28.0a1
Firmware Version: 20131115

.:First Broken Build:.
Environmental Variables:
Device: Buri 1.3 mozRIL
BuildID: 20131202092621
Gaia: 8527840366eec3252d04e0dad64e7a40dbe3dc56
Gecko: 2581b84e0ca1
Version: 28.0a1
Firmware Version: 20131115

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

We found a slight work around by killing the app and reseting the cost control.  Having said that returning to the cost control after killing it leaves the graph white again.

Comment 2

[Jason Smith [:jsmith]]

There's nothing that landed in the cost control app & shared on 12/1/2013 in the Gaia regression range. I'm guessing this is a gecko regression.

Comment 3

[Jason Smith [:jsmith]]

QA Wanted - Can someone get a logcat when this bug reproduces?

Comment 4

[Jason Smith [:jsmith]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=84a5a5800bd3&tochange=2581b84e0ca1

Comment 5

[Jason Smith [:jsmith]]

Salva - Can you weigh in on what's causing the let's go button to disable?

Comment 6

[Jason Smith [:jsmith]]

Just realized that today's build was spun later, so it will include 12/2/2013 commits. The only gaia commit that landed in this regression range is:

https://github.com/mozilla-b2g/gaia/commit/6e2b106720adebd47b088305a2df9da3ae765484

Comment 7

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #6)
> Just realized that today's build was spun later, so it will include
> 12/2/2013 commits. The only gaia commit that landed in this regression range
> is:
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 6e2b106720adebd47b088305a2df9da3ae765484

cost control gaia commit specifically

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: logcat of usage app start until Let's go is pushed.txt

Comment 9

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: logcat of usage app start until Let's go is pushed.txt

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

in logcat : 
12-02 19:25:40.229: E/GeckoConsole(496): [JavaScript Warning: "Error in parsing value for 'background'.  Declaration dropped." {file: "app://costcontrol.gaiamobile.org/style/views/firsttime.css" line: 151 column: 88 source: "    background: url(../images/app/icons/checked.png) content-box right center no-repeat / 1.9rem;"}]

Comment 11

[Salvador de la Puente González [:salva]]

I can not reproduce. The referenced bug should not affect the start-up process and a CSS error does not block JS execution.

Comment 12

[Askeing Yen[:askeing][:fyen]]

(In reply to gbennett from comment #0)
> .:Last Working Build:.
> Environmental Variables:
> Device: Buri 1.3 mozRIL
> BuildID: 20131201040201
> Gaia: f23aebebcd28c4d19347def77c4836c8baebc2ce
> Gecko: 84a5a5800bd3
> Version: 28.0a1
> Firmware Version: 20131115
> 
> .:First Broken Build:.
> Environmental Variables:
> Device: Buri 1.3 mozRIL
> BuildID: 20131202092621
> Gaia: 8527840366eec3252d04e0dad64e7a40dbe3dc56
> Gecko: 2581b84e0ca1
> Version: 28.0a1
> Firmware Version: 20131115

Gaia:     8ba455758d557ff18e4ef41a9fc2e683d904c21c
Gecko:    http://hg.mozilla.org/mozilla-central/rev/2581b84e0ca1
Works for me.

Gaia:     6e2b106720adebd47b088305a2df9da3ae765484
Gecko:    http://hg.mozilla.org/mozilla-central/rev/2581b84e0ca1
Works for me, too.
(git checkout 6e2b106 && B2G_SYSTEM_APPS=1 make reset-gaia)

Comment 13

[Askeing Yen[:askeing][:fyen]]

Gaia: 8527840366eec3252d04e0dad64e7a40dbe3dc56 => Failed
Gaia: f6c046839673ba58db441830236a3c9b562275b6 => Failed
Gaia: b87c58c072f9d4fd56698af54d8786e11a18bcc0 => WFM
Gaia: 772261a7c0fe7d2a80a4c0ec887d669d90c93db7 => WFM
Gaia: 3a070d5a406a4ae1f3d3f5208bc23bef31cd9f83 => WFM
Gaia: 5e99c9981e18cb4d51a5b2419d23b3ed00f9655c => WFM

test f6c0468396 again, then failed again.
($ git checkout f6c0468396 && make reset-gaia)
Gaia:     f6c046839673ba58db441830236a3c9b562275b6
Gecko:    http://hg.mozilla.org/mozilla-central/rev/2581b84e0ca1

https://github.com/mozilla-b2g/gaia/commit/f6c046839673ba58db441830236a3c9b562275b6

Comment 14

[Askeing Yen[:askeing][:fyen]]

test b87c58c072f9d4fd56698af54d8786e11a18bcc0 again, and failed now.
I guess it might be the gecko issue.

Comment 15

[Jason Smith [:jsmith]]

(In reply to Salvador de la Puente González [:salva] from comment #11)
> I can not reproduce. The referenced bug should not affect the start-up
> process and a CSS error does not block JS execution.

Are you running the latest gecko changes available? I think Askeing is right here - this is likely a gecko regression. What I'm looking to know is what's failing at the let's go button of the usage setup. Once I know that, I can probably point out the relevant gecko engineer who would know how to fix this.

Comment 16

[Salvador de la Puente González [:salva]]

My Gecko build is from yesterday.

Comment 18

[Jason Smith [:jsmith]]

(In reply to Salvador de la Puente González [:salva] from comment #16)
> My Gecko build is from yesterday.

Looks like this still confirmed to be failing during today's testing. Can you double check? Are you testing on Buri?

Trying to figure out why you can't reproduce.

Comment 19

[Askeing Yen[:askeing][:fyen]]

Updated: try again today
$ git checkout 8ba455758d && B2G_SYSTEM_APPS=1 make reset-gaia
8ba455758d WFM. (10/10 passed)

$ git checkout 6e2b106720 && B2G_SYSTEM_APPS=1 make reset-gaia
6e2b106720 Failed. (1/10 passed, first time passed, then all failed)

today's build also failed.

It seems like the root cause is comment 6.

Comment 20

[Salvador de la Puente González [:salva]]

I got reproduce but under very specific conditions. We are checking with today's build.

Comment 21

[Marina Rodríguez [:mai]]

Attachment: proposal patch v1

please, could you review the code?

Comment 22

[Tony Nguyen]

I encountered a crash when attempting to open the Usage App for the first time after an FTU. I followed the same repro steps and tapped on the "Let's Go" button repeatedly. I was only able to repro the crash two times.

Signature: mozilla::dom::network::MobileConnection::Listener::NotifyVoiceChanged() 

Environmental Variables:
BuildID: 20131204040204
Gaia: 45564d6318cdab2fae2de0eb801308e2bcf4e472
Gecko: 9688476c1544
Version: 28.0a1

Comment 23

[Tony Nguyen]

The crash-state URL is: https://crash-stats.mozilla.com/report/index/f0994dc3-7e9b-463c-b20a-09d2e2131204

I was able to repro this crash for a third time.

Comment 24

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I don't think the crash is related to this bug.  I am working with Tony to write the bug separately and figure out where the crash really lies.

Comment 25

[Marina Rodríguez [:mai]]

Comment on attachment 8342331 [details]
proposal patch v1

Please Jose Manuel, Salva is on holidays, could you review my patch?

Comment 26

[Askeing Yen[:askeing][:fyen]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #24)
> I don't think the crash is related to this bug.  I am working with Tony to
> write the bug separately and figure out where the crash really lies.

Bug 946259 and Bug 946437 might be the same crash issue.

Comment 27

[Jose Manuel Cantera]

the proposed patch does not work properly

E/GeckoConsole(  806): [JavaScript Warning: "XUL box for p element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "app://costcontrol.gaiamobile.org/settings.html" line: 0}]
E/GeckoConsole(  806): [JavaScript Error: "ReferenceError: BalanceLowLimitView is not defined" {file: "app://costcontrol.gaiamobile.org/js/settings/settings.js" line: 216}]
E/GeckoConsole(  806): [JavaScript Error: "TypeError: model.axis.Y.get is not a function" {file: "app://costcontrol.gaiamobile.org/js/views/datausage.js" line: 391}]

Comment 28

[Marina Rodríguez [:mai]]

Comment on attachment 8342331 [details]
proposal patch v1

Change the async loading of some scripts in settings.html

Comment 29

[Jose Manuel Cantera]

Comment on attachment 8342331 [details]
proposal patch v1

at application start-up the proposed patch leaves the CostControl app in a 'black screen' state for 2 seconds. Thus, the patch is a clear r-

Comment 30

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Askeing, yes... I was trying to determine if Bug 946259 was the same crash or not by verifying the crash stack.  If we don't get the crash stack, we're guessing that it's the same.  I believe that we should confirm if it's the same by looking at the crash stack.

Also I am unsure that the crash has to do with this particular bug because this bug happened before the crash started occurring.  One way to tell is if the patch in bug 946437 fixes the crash and this bug still occurs.

Thanks.

Comment 31

[bhavana bajaj [:bajaj]]

NI on :mai to help with an eta here as this has been failing smoketest for a while as pointed by QA.
:mai 1.3 is going to be FC on Monday(12/9), we would recommend backing https://bugzilla.mozilla.org/show_bug.cgi?id=937554 this out in case this bug cannot be resolved in time.

Comment 32

[Marina Rodríguez [:mai]]

Comment on attachment 8342331 [details]
proposal patch v1

Please, could yo review the path?

Start-up time has been reduced.

Comment 33

[Salvador de la Puente González [:salva]]

Comment on attachment 8342331 [details]
proposal patch v1

Before granting you the r+, can you explain me what are the root causes of this bug?

Comment 34

[Salvador de la Puente González [:salva]]

Comment on attachment 8342331 [details]
proposal patch v1

The patch does not address the root cause. The root cause is an `updateUI()` call is triggered by the observer in [1]. This only should happen **after** application set-up. So the observer needs to be moved.

In addition, a related but yet unadvertised bug has been found.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/costcontrol/js/app.js#L169

Comment 35

[Salvador de la Puente González [:salva]]

This is indeed caused by the patch for the bug 944718 but that patch is correct in terms of what it is fixing. Jason, can you give us your opinion here?

What do you prefer, a backout for patch in bug 944718 or a specific path of this?

IMHO, in order to keep patches separated and fixing only what they were intended to fix, I would prefer to provide a specific patch here but I would like to hear your opinion first.

Thank you!

Comment 36

[Jason Smith [:jsmith]]

(In reply to Salvador de la Puente González [:salva] from comment #35)
> This is indeed caused by the patch for the bug 944718 but that patch is
> correct in terms of what it is fixing. Jason, can you give us your opinion
> here?
> 
> What do you prefer, a backout for patch in bug 944718 or a specific path of
> this?
> 
> IMHO, in order to keep patches separated and fixing only what they were
> intended to fix, I would prefer to provide a specific patch here but I would
> like to hear your opinion first.
> 
> Thank you!

I think we should backout bug 944718 for now. The main reasons for this is:

1. We've been blocked on this for a few days in the cost control app, which is blocking daily testing of that app & ui automation with that app
2. We need to kick off a 1.3 test run tomorrow, so not having the cost control app working at a basic level will greatly limit the depth of the testing executed

When we resolve the root cause problem in this bug, then we can reland the backed out patch in bug 944718 with the fix needed for this bug.

Comment 37

[Salvador de la Puente González [:salva]]

Ok, agree with you. Let's back-out the bug 944718, version for master due to version for 1.2 has not this problem.

Comment 38

[Jason Smith [:jsmith]]

Fixed via backout on master:

https://github.com/mozilla-b2g/gaia/commit/24823a14069fee495b34d693d79bcf02a1da8764

Comment 39

[Marcia Knous [:marcia]]

Verified fixed on Buri using:

Gaia   c952e2756c03eceb4de6a3eba15651741a62f9e8
SourceStamp df82be9d89a5
BuildID 20131210040206
Version 29.0a1

The backout has restored the Usage functionality and I can now see the graph and other associated information in the usage app.

Comment 40

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 24823a14069fee495b34d693d79bcf02a1da8764 to:
v1.3 already had this commit