Closed Bug 879790 Opened 7 years ago Closed 7 years ago

Some data usage can be lost during the very first start-up of Gecko

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:-)

RESOLVED FIXED
blocking-b2g -

People

(Reporter: salva, Assigned: albert)

References

Details

(Whiteboard: c=)

Attachments

(1 file, 2 obsolete files)

Current NetworkStatistics API is missing data during the very first start-up of Gecko (after flashing).
Duplicate of this bug: 879788
(In reply to Salvador de la Puente González [:salva] from comment #0)
> Current NetworkStatistics API is missing data during the very first start-up
> of Gecko (after flashing).

User impact?
As this is a very short period of time, we are talking about 200 - 600 bytes. Anyway solution should have very low risk according to Albert. Can you confirm this, Albert?
Flags: needinfo?(acperez)
To fix this bug we just need to add a record with rx and tx = 0 when NetworkStats DB is created.
Flags: needinfo?(acperez)
200-600 bytes is not worth the risk even if it is close to null, moving to leo triage but you can always request approval.
blocking-b2g: tef? → leo?
Depends on: 879793
With no stated user impact, and the relatively minimal loss here, this is not a blocker. Renominate if there's a change in that assessment otherwise this can ride trains.
blocking-b2g: leo? → -
Whiteboard: c=
Attachment #762032 - Flags: review?(gene.lian)
Sorry, last patch was wrong.
Attachment #762032 - Attachment is obsolete: true
Attachment #762032 - Flags: review?(gene.lian)
Attachment #762034 - Flags: review?(gene.lian)
Comment on attachment 762034 [details] [diff] [review]
Fix stats lost at startup adding initial records to indexedDB

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

Nice patch!

r=gene when the nits are fixed. Thanks!

::: dom/network/src/NetworkStatsDB.jsm
@@ +29,4 @@
>    if (DEBUG) {
>      debug("Constructor");
>    }
> +  this.connectionTypes = aConnectionTypes;

s/connectionTypes/_connectionTypes

since it seems it's a coding convention within this file to represent local members.

@@ +72,5 @@
> +        // For mobile stats, when the network interface comes up there is a delay
> +        // between the time when the change to up happens and when the notification
> +        // is the received. In this short interval some traffic is generated and it
> +        // is not registered because is the first sample. The initialization of the
> +        // database fix that 

Trailing space.

Although I'm native English speaker, I'd prefer rewriting the comment to be:

// There could be a time delay between the point when the network
// interface comes up and the point when the database is initialized.
// In this short interval some traffic data are generated but are not
// registered by the first sample. The initialization of the database
// should make up the missing sample.

Please feel free to correct this comment again. Hope I didn't misunderstand your thought.

@@ +74,5 @@
> +        // is the received. In this short interval some traffic is generated and it
> +        // is not registered because is the first sample. The initialization of the
> +        // database fix that 
> +        let stats = [];
> +        for (let connection in this.connectionTypes) {

s/connectionTypes/_connectionTypes

@@ +75,5 @@
> +        // is not registered because is the first sample. The initialization of the
> +        // database fix that 
> +        let stats = [];
> +        for (let connection in this.connectionTypes) {
> +          let connectionType = this.connectionTypes[connection].name;

s/connectionTypes/_connectionTypes

@@ +76,5 @@
> +        // database fix that 
> +        let stats = [];
> +        for (let connection in this.connectionTypes) {
> +          let connectionType = this.connectionTypes[connection].name;
> +          let timestamp = this.convertDate(new Date());

Please see bug 877607, comment #21. If you don't mind, I'd prefer renaming this to be:

  s/convertDate/normalizeDate
Attachment #762034 - Flags: review?(gene.lian) → review+
nits fixed
Keywords: checkin-needed
https://hg.mozilla.org/projects/birch/rev/cae98e3c82fb

BTW, this had a bunch of redundant changes due to bug 877607. Please try to avoid that in the future.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cae98e3c82fb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.