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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

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

Current NetworkStatistics API is missing data during the very first start-up of Gecko (after flashing).

Comment 2

[Alex Keybl [:akeybl]]

(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?

Comment 3

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

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?

Comment 4

[Albert [:albert]]

To fix this bug we just need to add a record with rx and tx = 0 when NetworkStats DB is created.

Comment 5

[Daniel Coloma:dcoloma]

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.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 7

[Albert [:albert]]

Attachment: Fix stats lost at startup adding initial records to indexeddb

Comment 8

[Albert [:albert]]

Attachment: Fix stats lost at startup adding initial records to indexedDB

Sorry, last patch was wrong.

Comment 9

[Gene Lian (I already quit Mozilla)]

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

Comment 10

[Albert [:albert]]

Attachment: Fix stats lost at startup adding initial records to indexedDB

Comment 11

[Albert [:albert]]

nits fixed

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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