1064491 - [Data Usage][Data Usage widget displaying incorrect values with the available data limit

Status: VERIFIED FIXED

Description

[Josh Schmitt [Joshs]]

Description:
The Cost Control widget is displaying the remaining data incorrectly with the Cost Control app

Repro Steps:
1) Update a Flame device to BuildID: 20140908040204
2) Connect to Cellular network
3) Open the Cost Control app
4) Set a limit/warning of 15MB
5) Browse a few websites
6) Pull down the notification center

Actual:
The 'available' data value is displayed incorrectly.

Expected: 
The 'available' data value is displayed correctly.

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140908040204
Gaia: c71fd5d8c9c7cb021c97e5e9fbb29f92b50a084d
Gecko: 892768985915
Version: 35.0a1 (2.2 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
  
Notes:
Repro frequency: 100%
See attached: screenshot, logcat

Comment 1

[Martin Shuman [:Marty]]

Created attachment 8485945 [details]
logcat_ Cost_Control.txt

Comment 2

[Martin Shuman [:Marty]]

Created attachment 8485946 [details]
Cost_Control_Screenshot.png

Comment 3

[Martin Shuman [:Marty]]

This issue DOES occur on Flame 2.1.
Notification tray widget displays 14.13 MB used when Usage app displays 11.57 MB used.

Environmental Variables:
Device: Flame 2.1
Build ID: 20140908000204
Gaia: a8e4d26555e5713ec6c72270cfd0cfabc096a0d3
Gecko: 746f24f9d21d
Version: 34.0a2 (2.1)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 4

[Albert [:albert]]

As talked offline with Marina, she told me that Costcontrol app changed the total usage query to the API. Before changes, costcontrol requested the total amount of traffic per interface and now it requests the amount of traffic per application and interface.

The NetworkStats API handle these two requests in a different way:
- Total traffic per interface obtains usage directly from the interface at Linux level (widget).
- Traffic per app and interface is obtained at gecko level from the protocols of necko (https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/). (costcontrol app)

On the other hand, alarms are handled by iptables through the netd at Linux level also.

Having that, we have to keep in mind that charges of Telco providers will be closer to the amount of data counted at Linux level than the amount counted at Gecko, which is lower.

So, we have to possible solutions:

1) Keep per app queries in costcontrol app in order to show the per app traffic but add the interface query to show the Wifi and Mobile traffic. The difference between totals can be added to 'System' app.

2) Investigate what is producing the leak in gecko in order to fix it.

IMHO we should go for solution 1 in order to fix the bug quickly, but we have to start working in option 2 in a followup to provide accurate results in the future. Option 2 will require more work and it can take long time.

Comment 5

[Peter Bylenga [:PBylenga]]

qawanted for 2.0 branch check.  I'd nominate this to block release for the lowest affected branch, this can affect a user financially.

Comment 7

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

Marshall, I think this is a regression from bug 1033549. Do you think we could make two separate queries? One for the total data usage and another for app statistics in order to keep app and widget synchronized?

Comment 8

[Maria Angeles Oteo (:oteo)]

[Blocking Requested - why for this release]: feature broken

Comment 9

[Maria Angeles Oteo (:oteo)]

Comms triage: Data lost, feature broken, regression

Comment 10

[Duane Dixon [:ddixon]]

Branch Check

Issue DOES occur in Flame 2.2, 2.1, and Open_C 2.2

Note: All Flame devices were tested with 512 MB  

Device: Flame Master
Build ID: 20140919111450
Gaia: 12d634c63ec8a41553a9f150c38260c992dc0020
Gecko: a084c4cfd8a1
Version: 35.0a1 (Master)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
-------------------------------------------------------------------
Device: Flame 2.1
Build ID: 20140922053548
Gaia: 689c4ad4d8c3e4aa95805a2e49ae6cf786a1ae91
Gecko: 185fc54d29c1
Version: 34.0a2 
Firmware Version: v180 
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
-------------------------------------------------------------------
Device: Open_C Master
Build ID: 20140922040649
Gaia: 3802009e1ab6c3ddfc3eb15522e3140a96b33336
Gecko: 5e704397529b
Version: 35.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
-------------------------------------------------------------------
-------------------------------------------------------------------
Issue DOES NOT occur in Flame 2.0

Device: Flame 2.0
Build ID: 20140922082143
Gaia: 0658006be8a00fdb5931ee15a0aa353a3ab231ba
Gecko: dc61f92b855e
Version: 32.0 (2.0)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 11

[Adam Alldredge [:AdamA][Inactive]]

B2G-Inbound regression window:

Last working:
Device: Flame 2.1
BuildID: 20140831192413
Gaia: 561f053a8eb8da05d6c8432f35e01db36b55d1fd
Gecko: c345da3eef13
Version: 34.0a1 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken:
Device: Flame 2.1
BuildID: 20140831221313
Gaia: 728132e3e6e438526b038e5a12f9014846d1c15a
Gecko: dc4e0ca31737
Version: 34.0a1 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Last working Gaia First Broken Gecko: Issue does NOT reproduce
Gaia: 561f053a8eb8da05d6c8432f35e01db36b55d1fd
Gecko: dc4e0ca31737

First Broken Gaia Last working Gecko: Issue DOES reproduce
Gaia: 728132e3e6e438526b038e5a12f9014846d1c15a
Gecko: c345da3eef13

Pushlog:
https://github.com/mozilla-b2g/gaia/compare/561f053a8eb8da05d6c8432f35e01db36b55d1fd...728132e3e6e438526b038e5a12f9014846d1c15a

Caused by Bug 1033549

Comment 12

[Joshua Mitchell (Inactive)]

Caused by Bug 1033549? Can you take a look Marshall?

Comment 13

[Marshall Culpepper [:marshall_law]]

Salva's recommendation sounds like a good one, I'll look into it today and get a patch up soon.

Comment 14

[Hema Koka [:hema]]

Marshall,

Any update on this?

Thanks
Hema

Comment 15

[Marina Rodríguez [:mai]]

Hi,
Since the patch "Bug 1033549 - [User Story] Data tracking on a per app basis" was landed, we've detected a big regression on the graphic loading, I thought more than 500ms. IMHO, it's necessary the separation of the queries to prevent this annoying effect.
Regards

Comment 16

[Marshall Culpepper [:marshall_law]]

Created attachment 8500666 [details] [review]
defer per-app request - v1

This patch reverts the original loading behavior by only loading the total data usage for each interface.

If there is any mobile data (model.data.mobile.total > 0), then another set of requests are made for each app, and the UI is updated after the app is loaded.

Also added a fix for the 'null' entry in usage when just the top level info is queried from NetworkStatsDB.

Comment 17

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

Comment on attachment 8500666 [details] [review]
defer per-app request - v1

Marshall, your patch is almost working. Please check the comments on GitHub and notice the most important part:

There is a race condition between checking total mobile data field of the model and filling that field. It's all on GitHub. Please, review and address the issues and ask for my review again.

Thank you, Marshall.

Comment 18

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8500666 [details] [review]
defer per-app request - v1

Addressed comments on the pull request

Comment 19

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

Comment on attachment 8500666 [details] [review]
defer per-app request - v1

I did not see any the part covering the new calculation of by app ratios. If I've missed something, just let me know. Flag me when the patch is ready.

Comment 21

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

Comment on attachment 8500666 [details] [review]
defer per-app request - v1

Ok. Now the patch is working fine. The chart it's not only showing the same data amount as the widget but now it render a lot faster.

Thank you very much, Marshall. Good work!

Comment 22

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

Notice in the patch we are showing bars of breakdown in relation with the total amount for the chart / widget query. That query is counting data at an interface level and it's counting in a different way than by-app. We should address the problem of adding the difference (sometimes a very big difference) in an application to provide a consistent UX (because it's very easy for the user to sum all the applications together and see the total is below the total shown in the chart).

Comment 23

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

master: 5a15414d7da5d44658cfd822fd5b57efde6d2f2d

Comment 24

[Marshall Culpepper [:marshall_law]]

Created attachment 8502644 [details] [review]
v2.1 PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 25

[bhavana bajaj [:bajaj]]

(In reply to Marshall Culpepper [:marshall_law] from comment #24)
> Created attachment 8502644 [details] [review]
> v2.1 PR
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #):
> [User impact] if declined:
> [Testing completed]:
> [Risk to taking this patch] (and alternatives if risky):
> [String changes made]:

marshall, can you please fill in these details in order to consider you uplift request?

Comment 26

[Marshall Culpepper [:marshall_law]]

restores the correct overall data usage label in the 'Usage' app, and improves startup time of the app
[Bug caused by] (feature/regressing bug #): Bug 1033549
[User impact] if declined: incorrect usage will be displayed in the Usage app, and Usage app startup time will increase
[Testing completed]: Tested with a AT&T sim card on a Flame, and by Salva
[Risk to taking this patch] (and alternatives if risky): this patch basically defers the query for app usage metrics until after the chart is loaded, which could introduce race conditions, we've mitigated as much as possible by only updating parts of the model in each query though.
[String changes made]: None

Comment 27

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/062901ef93eada3a7ebfc37333dc00632307876c

Comment 28

[Fred Muyumba [:FredM]]

This issue is verified fixed on Flame 2.2 Master KK (319mb) (Full Flash) and Flame 2.1 KK (319mb) (Full Flash)

Environmental Variables:
Device: Flame 2.2 Master KK (319mb) (Full Flash)
BuildID: 20141017040208
Gaia: abef62c0623e5504a97b4fd411e879a67b285b52
Gecko: ae1dfa192faf
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 36.0a1 
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

-----------------------------------------------------------------------------------------------------

Environmental Variables:
Device: Flame 2.1 KK (319mb) (Full Flash)
BuildID: 20141017001201
Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98
Gecko: 2befa902ff5c
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Result: The 'available' data value IS displayed correctly.