952029 - [NetworkStats API] clearStats method fails after restart if data connection is disabled

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Marina Rodríguez [:mai]]

STR.
1. Produce traffic with mobile network
2. Disable data connection on settings ('Cellular & Data')
3. Restart the device without activate mobile traffic
4. Launch the Usage app
5. Tap gear button to open settings page of Usage app.
6. Tap "Reset" button, then tap "Reset mobile usage" button or "Reset mobile & wifi data".

Actual:
Mobile usage not refresh the info

Expected:
mobile usage data reset.

If you enabledata connection on settings ('Cellular & Data'), then Usage control works fine and reset the mobile data.

Comment 1

[Albert [:albert]]

Clear stats function fails because the network is not enabled and is not loaded in memory, the solution is to check if the network is valid querying the database.

Comment 2

[Albert [:albert]]

Attachment: Patch

The patch check if the network is valid querying the database if it is not found in the array of networks.

Comment 3

[Gene Lian (I already quit Mozilla)]

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

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

::: dom/network/src/NetworkStatsService.jsm
@@ +431,4 @@
>  
>        // Check if network is valid but has not established a connection yet.
>        let rilNetworks = this.getRilNetworks();
> +      if (!rilNetworks[netId]) {

I don't know why you flip this check. Could you please explain more? Thanks!

@@ +443,5 @@
> +            return;
> +          }
> +
> +          mm.sendAsyncMessage("NetworkStats:Clear:Return",
> +                              { id: msg.id, error: "Invalid networkType", result: null });

I wonder you need to return the whole function clearInterfaceStats() here, since you already sent "NetworkStats:Clear:Return" back.

Comment 4

[Albert [:albert]]

Attachment: Patch v2

Comment 5

[Albert [:albert]]

(In reply to Gene Lian [:gene] (PTO Dec. 25 ~ Jan. 5) from comment #3)
> Comment on attachment 8358337 [details] [diff] [review]
> Patch
> 
> Review of attachment 8358337 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/network/src/NetworkStatsService.jsm
> @@ +431,4 @@
> >  
> >        // Check if network is valid but has not established a connection yet.
> >        let rilNetworks = this.getRilNetworks();
> > +      if (!rilNetworks[netId]) {
> 
> I don't know why you flip this check. Could you please explain more? Thanks!

Because we have to look for network in database if it is not in rilNetworks.

Comment 6

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8359066 [details] [diff] [review]
Patch v2

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

::: dom/network/src/NetworkStatsService.jsm
@@ +431,2 @@
>  
>        // Check if network is valid but has not established a connection yet.

This comment should be revised based on what you're doing.

@@ +432,5 @@
>        // Check if network is valid but has not established a connection yet.
>        let rilNetworks = this.getRilNetworks();
> +      if (!rilNetworks[netId]) {
> +
> +        // Check if the network is available in the DB.

Don't need the the blank line above this comment.

Comment 7

[Albert [:albert]]

Attachment: Patch v3

Changes from comment 6.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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