Closed Bug 952029 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
1.3 C2/1.4 S2(17jan)

People

(Reporter: mai, Assigned: albert)

Details

Attachments

(1 file, 2 obsolete files)

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.
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.
Attached patch Patch (obsolete) — Splinter Review
The patch check if the network is valid querying the database if it is not found in the array of networks.
Attachment #8358337 - Flags: review?(gene.lian)
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
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.
Attachment #8358337 - Flags: review?(gene.lian)
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #8358337 - Attachment is obsolete: true
Attachment #8359066 - Flags: review?(gene.lian)
(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 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.
Attachment #8359066 - Flags: review?(gene.lian) → review+
Attached patch Patch v3Splinter Review
Changes from comment 6.
Attachment #8359066 - Attachment is obsolete: true
Attachment #8359180 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/baecd9b9d75c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
You need to log in before you can comment on or make changes to this bug.