965305 - [Cost Control] Resetting wi-fi usage from Cost Control app does not always reset to 0 B

Status: RESOLVED FIXED

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

Description

[Bob Silverberg [:bsilverberg]]

This reproduces frequently in our automated test, and also intermittently manually.

STR:

1. Connect to wi-fi.
2. Open cost control and configure it to monitor wi-fi usage.
3. Open browser and visit a site (i.e., http://mozqa.com/data/firefox/layout/mozilla.html).
4. Disconnect wi-fi.
5. Open cost control and observe values. Wi-fi usage will be as expected (in the case of the test, approx. 2.8 MB).
6. Go to Cost Control Settings and reset wi-fi usage. Observe new value for wi-fi usage.

Expected result: View 0 B in step 6.
Actual result: View a small amount in step 6 (e.g., in the test, approx. 7.7 KB)

Tested on a Hamachi:
Gaia      b0aa74619cffb7a39ee0e7da5e9361a98e69f3f6
Gecko     http://hg.mozilla.org/releases/mozilla-aurora/rev/7d1173c4b173
BuildID   20140129004052
Version   28.0a2

Comment 2

[Jason Smith [:jsmith]]

We really shouldn't be breaking these type of things this late in the game.

Comment 3

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

It is probably not a Cost Control issue but an interface issue. Please, notice switching off Wi-Fi does not take place immediately. Anyway, asking Albert for back-end clarifications.

Albert, can you shed some light about this behavior?

Comment 4

[Peter Bylenga [:PBylenga]]

This issue has an average reproducibility rate of 20% or 1/5 attempts.

Regression window is 11-26-2013 to 11-27-2013.  On 11-26-2013 is the last build where we see bug 942060 which blocks reproducing the current issue.  On builds before the regression window found in 942060, I was not able to reproduce the issue.

Last build issue did not reproduce:
v1.3 Environmental Variables:
Device: Buri v1.3 MOZ
BuildID: 20131126052050
Gaia: 4ad796b196d468bdb231beba4392acbc90a74e96
Gecko: 99479edbee2a
Version: 28.0a1
Firmware Version: v1.2-device.cfg

First build issue reproduces:
v1.3 Environmental Variables:
Device: Buri v1.3 MOZ
BuildID: 20131127040203
Gaia: d4b9a3d271f0451b4d903a03c2b931b8cc092041
Gecko: 6ecf0c4dfcbe
Version: 28.0a1
Firmware Version: v1.2-device.cfg

Comment 5

[Peter Bylenga [:PBylenga]]

Attachment: ResetUsageAppWifi_log.txt

This log is from the 11-27-2013 nightly Buri v1.3 build I mentioned in the previous comment.

This log covers these STRs
1. After browser activity disable WiFi in Settings
2. Launch Usage App
3. Enabled wifi on graph
4. Navigate to Usage Settings
5. Tap Reset

Actual results: data flashes but remains the same
Expected results: data is reset to 0


It should be noted that on later builds the actual results I observed were closer to what reporter recorded in Comment 0.

Comment 6

[Albert [:albert]]

(In reply to Salvador de la Puente González [:salva] from comment #3)
> It is probably not a Cost Control issue but an interface issue. Please,
> notice switching off Wi-Fi does not take place immediately. Anyway, asking
> Albert for back-end clarifications.
> 
> Albert, can you shed some light about this behavior?

It is a back-end bug, because stats are not updated before doing the reset.

Comment 7

[[:AndreiH]]

This can be also reproduce on v1.4 aswell, build:

Gaia      ea4a1f0d94a995486ed219f47132949071ecc172                         
Gecko     https://hg.mozilla.org/mozilla-central/rev/262e73a6b7cd          
BuildID   20140206160203                                                   
Version   30.0a1

Comment 8

[Albert [:albert]]

Attachment: Patch

Patch to update stats before reset.

Comment 9

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8373320 [details] [diff] [review]
Patch

Review of attachment 8373320 [details] [diff] [review]:
-----------------------------------------------------------------

Wrong patch?

Comment 10

[Albert [:albert]]

Attachment: Patch

Correct patch

Comment 11

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8373855 [details] [diff] [review]
Patch

Review of attachment 8373855 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Just some nits.

::: dom/network/src/NetworkStatsService.jsm
@@ +473,5 @@
> +          return;
> +        }
> +
> +        self._db.clearInterfaceStats(network, function onDBCleared(aError, aResult) {
> +            self._updateCurrentAlarm(aNetId);

Please indent the codes with just 2 spaces.

@@ +498,5 @@
>  
> +      self.updateAllStats(function onUpdate(aResult, aMessage){
> +        if (!aResult) {
> +          mm.sendAsyncMessage("NetworkStats:ClearAll:Return",
> +                            { id: msg.id, error: aMessage, result: null });

Please properly align this.

Comment 12

[Albert [:albert]]

Attachment: Patch

Nits fixed

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Doesn't apply to b2g-inbound. Please rebase.

Comment 14

[Albert [:albert]]

This patch should be applied after the patch of bug 966244, which is marked as a dependency.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Sorry about that.

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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

You probably want to reconsider this bug's blocking status as well if you're doing all the other ones.

Comment 17

[Jason Smith [:jsmith]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #16)
> You probably want to reconsider this bug's blocking status as well if you're
> doing all the other ones.

Actually this one is a legitimate user facing regression, so this one would block. The ones where we don't have regression proof is where we should renom.

Comment 18

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #17)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #16)
> > You probably want to reconsider this bug's blocking status as well if you're
> > doing all the other ones.
> 
> Actually this one is a legitimate user facing regression, so this one would
> block. The ones where we don't have regression proof is where we should
> renom.

Although I do the dependencies you are linking to here, so the real problem here is that might be the wrong way to fix the bug.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Yeah, the point is that this bug as-patched depends on the other ones that were backed out and put up for reconsideration.

Comment 20

[Jason Smith [:jsmith]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=99479edbee2a&tochange=6ecf0c4dfcbe

Comment 21

[Albert [:albert]]

Attachment: Patch for 1.3

Rebase for 1.3

Removed dependencies from other bugs that had been backed out.

Comment 22

[Albert [:albert]]

Request approval before uplifting.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2ed60f63b7a6

Comment 24

[Ryan VanderMeulen [:RyanVM]]

(In reply to Albert [:albert] from comment #22)
> Request approval before uplifting.

Please use the approval-mozilla-b2g28? flag on the patch.

Comment 25

[Albert [:albert]]

Comment on attachment 8375421 [details] [diff] [review]
Patch for 1.3

NOTE: This flag is now for security issues only. 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 #): 
User impact if declined: Costcontrol will report more usage than the real.
Testing completed: yes
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/83d4288720dd