B2G RIL: Should re-send the emergency callback mode event when entering the mode twice

RESOLVED FIXED in mozilla26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: aknow, Assigned: aknow)

Tracking

Trunk
mozilla26
All
Gonk (Firefox OS)
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Currently, we only sent the event when enter->exit or exit->enter. Consider the following scenario:

1. user dial an emergency call 911
2. receive unsolicited response for entering, pass the event to gaia
3. gaia starts its count down timer
4. after 4 mins, user re-dial the emergency call
5a. receive unsolicited response again for entering. mode not change. skip

Step 5a is wrong. We should still pass the event to gaia, and tell gaia to restart its 5 min timer.

Thus, We should always sent the event to gaia when received the unsolicited response from ril.
Posted patch Part 2: Update test case (obsolete) — Splinter Review
Attachment #800017 - Flags: review?(htsai)
Posted patch Part 2#2: Update test case (obsolete) — Splinter Review
Attachment #800017 - Attachment is obsolete: true
Attachment #800017 - Flags: review?(htsai)
Attachment #800019 - Flags: review?(htsai)
Comment on attachment 800019 [details] [diff] [review]
Part 2#2: Update test case

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

::: dom/system/gonk/tests/test_ril_worker_ecm.js
@@ +58,5 @@
>    let worker = workerHelper.worker;
>  
> +  // Do it twice. Should always send the event.
> +  for (let i = 0; i < 2; ++i) {
> +    worker.RIL._isInEmergencyCbMode = false;

I think what we'd like to see here is, even |worker.RIL._isInEmergencyCbMode == true| for the 2nd run, the notification message will still be sent out. The line #62 is supposed to be out of the for-loop, right?

@@ +77,5 @@
>  
>    run_next_test();
>  });
>  
> +

nit: Extra new line?
Attachment #800019 - Flags: review?(htsai)
Comment on attachment 800016 [details] [diff] [review]
Part 1: Always send out emergencyCbModeChange event when received from ril

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

Looks good, thank you.

::: dom/system/gonk/ril_worker.js
@@ +3790,2 @@
>  
> +    // Start a new timeout event when enter the mode.

nit: s/enter/entering
Attachment #800016 - Flags: review?(htsai) → review+
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #4)
> Comment on attachment 800019 [details] [diff] [review]
> Part 2#2: Update test case
> 
> Review of attachment 800019 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/tests/test_ril_worker_ecm.js
> @@ +58,5 @@
> >    let worker = workerHelper.worker;
> >  
> > +  // Do it twice. Should always send the event.
> > +  for (let i = 0; i < 2; ++i) {
> > +    worker.RIL._isInEmergencyCbMode = false;
> 
> I think what we'd like to see here is, even |worker.RIL._isInEmergencyCbMode
> == true| for the 2nd run, the notification message will still be sent out.
> The line #62 is supposed to be out of the for-loop, right?

You are right. How about simply remove the line #62?
Comment on attachment 800019 [details] [diff] [review]
Part 2#2: Update test case

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

r=me with the comment addressed. Thank you.

::: dom/system/gonk/tests/test_ril_worker_ecm.js
@@ +58,5 @@
>    let worker = workerHelper.worker;
>  
> +  // Do it twice. Should always send the event.
> +  for (let i = 0; i < 2; ++i) {
> +    worker.RIL._isInEmergencyCbMode = false;

Came to agreement on removing line #62!
Attachment #800019 - Flags: review+
Attachment #800019 - Attachment is obsolete: true
rebase
Attachment #800067 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/f7ac432b38b5
https://hg.mozilla.org/mozilla-central/rev/035ce0773762
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.