1032095 - Usage message with sound

Status: VERIFIED FIXED

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

Description

[Loli (:lolimartinezcr)]

Flame
Platform versión: 32.0a2
Build ID: 20140630073320
Git commit: c0c8ad18

Reproducible: Not 100%

Pre-requisites: 
Prepaid with billing support SIM card

STR:
1. Tap usage application.
2. Tap "update" button

Actual result:
When user receive sms with balance it is with *sound*.

Expected reuslt
When user receive sms with balance it is without *sound*.

Comment 1

[Julien Wajsberg [:julienw]]

Loli, when you write "Reproducible: Not 100%", does it mean "near 100%" or less than this?

Comment 2

[Julien Wajsberg [:julienw]]

Also, it would be nice to know if the SMS app is launched in background or not. Can you please try both these cases?

Comment 3

[Loli (:lolimartinezcr)]

(In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #1)
> Loli, when you write "Reproducible: Not 100%", does it mean "near 100%" or
> less than this?

Sorry, when i write "Not 100%" it mean less tan this. Not always is reproducible. Marina have seen this bug with me.

Comment 4

[Loli (:lolimartinezcr)]

(In reply to Loli from comment #3)
> (In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #1)
> > Loli, when you write "Reproducible: Not 100%", does it mean "near 100%" or
> > less than this?
> 
> Sorry, when i write "Not 100%" it mean less tan this. Not always is
> reproducible. Marina have seen this bug with me.

Also, SMS app is launched in background

Comment 5

[Julien Wajsberg [:julienw]]

> Sorry, when i write "Not 100%" it mean less tan this. Not always is reproducible. Marina have seen this bug with me.

I meant: does it happen 25%, 50%, 75%, 99%?

When you have the sound, you still don't have a notification, right ?
Then the only code that does this is [1] but we should not run it if the number is made silent by Cost Control.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/activity_handler.js#L387-L388

Comment 6

[Loli (:lolimartinezcr)]

(In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #5)
> > Sorry, when i write "Not 100%" it mean less tan this. Not always is reproducible. Marina have seen this bug with me.
> 
> I meant: does it happen 25%, 50%, 75%, 99%?
> 
> When you have the sound, you still don't have a notification, right ?
> Then the only code that does this is [1] but we should not run it if the
> number is made silent by Cost Control.
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/activity_handler.
> js#L387-L388

does it happen 25%

When i have the sound when I have received a notification in utility try.

Comment 7

[Julien Wajsberg [:julienw]]

Ok, so you have a real notification in that case ?

(sorry, I try to be as complete as possible)

Comment 8

[Loli (:lolimartinezcr)]

Also Tested and it bug is reproducible:
Hamachi
2.1
Gecko-deb9478
Gaia-c086196

Comment 9

[Jason Smith [:jsmith]]

Loli - Can you provide a video so that we can understand the sound used here?

Comment 10

[Marina Rodríguez [:mai]]

I cannot reproduce it with:
Device: flame
OS: 2.1
Platform versión: 33.0a1
Build ID: 20140703063604
Git commit: f456c886

Comment 12

[Marina Rodríguez [:mai]]

Hi, 
At last, I've can reproduce the bug. This bug is not related with the silent sms feature. I found that we're receive two balance response messages. Then the first is silent but the second it isn't. This behavior is not correct.

I think the origin of the problem is related with the automatic balance request feature of costcontrol, This feature consist on the costcontrol app performs an automatic balance request in background when the time elapsed since the last request is greater than that reflected in the minimun_delay config var. I have to check why the update button is not locked when the background request is working. 

Regards

Comment 13

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: 2.0+ per comment 12.

Comment 14

[Julien Wajsberg [:julienw]]

Another solution is to keep track of the various requests and to not "unsilent" the phone number until all requests were answered.

Comment 15

[Marina Rodríguez [:mai]]

Attachment: patch v1.0

Hi Salva,
would you mind reviewing this patch?

On this patch, I added a flag to not allow send a balance request if another is being sended. And updating the property lastBalanceRequest when the balance timeout alarms is not added (because the request is sended correctly)

Regards

Comment 16

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

Comment on attachment 8452953 [details] [review]
patch v1.0

The bug is related with a mistake I made consisting on combining the already-existing in-progress lock with the alarm ID used for the timeout. Let's use the already-existing lock instead. It is the setting called `waitingForBalance` (which now stores the alarm id for timeout too).

Comment 17

[Marina Rodríguez [:mai]]

Comment on attachment 8452953 [details] [review]
patch v1.0

Updated the PR.
Would you mind reviewing the patch?
Regards

Comment 18

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

Comment on attachment 8452953 [details] [review]
patch v1.0

I'm not sure about the solution here. Could you review my comments on GitHub?

Comment 19

[Marina Rodríguez [:mai]]

Comment on attachment 8452953 [details] [review]
patch v1.0

Salva, would you review the patch again?
Regards

Comment 20

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

Comment on attachment 8452953 [details] [review]
patch v1.0

As agreed offline, please comment the problems here and open a follow-up to discuss further solutions.

Change the name of the flag by `isSendingBalanceRequest`. Thank you for the patch, Mai!

Comment 21

[Marina Rodríguez [:mai]]

Hi,
this patch provides a solution for the most common of the cases. When the user ask manually for a balance request when anothe request is sent in background.

Although the app has a lock to prevent this kind of situations, the non-transactional nature of AsyncStorage produces that the lock does not work fine when two balance request are asked at the same time. Besides the costcontrol app has the handicap of its architecture, the app hast two UI (app and widget) that complicates find a easy solution like a queue or a request counter (as Julien suggestion on comment 14), because this solution would also have the same problem of the current lock implemented at  asyncStorage level.

Currently on gaia dev list is being arguing how to proceed with this kind of problems, you can follow the progress through the following link:
https://groups.google.com/d/msg/mozilla.dev.gaia/g5_eOE7YHWw/s8dH40HC8lsJ (Non-transactional nature of AsyncStorage)

This bug still could be reproduced (although the probability is low), if the user interacts with the balance section of usage app at the same time that slide down the widget bar, and the minimun delay has been passed (currently the minimun delay is 3 hours in all the config of the countries). This kind of scenario only could be reproduced with the interaction of the user, and when both ui are involved.

Regards

Comment 22

[Marina Rodríguez [:mai]]

Master: af72c579ace4cbe1cdd7fd6e50abcb0a56cb0ae2

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/90fea7606b6b8768ebaa23f87b700fecd1dde497

Comment 24

[Joshua Mitchell (Inactive)]

issue is resolved, do not believe QA-wanted tag is still needed.

Comment 25

[Loli (:lolimartinezcr)]

tested and working
Flame
2.0
Gecko-5287d8d
Gaia-8cb1a94

2.1
Gecko 64bhb3bd
Gaia 89fd655