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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Szu-Yu Chen [:aknow] (inactive)]

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.

Comment 1

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1: Always send out emergencyCbModeChange event when received from ril

Comment 2

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Update test case

Comment 3

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2#2: Update test case

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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?

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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

Comment 6

[Szu-Yu Chen [:aknow] (inactive)]

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

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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!

Comment 8

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 1: Always send out emergencyCbModeChange event when received from ril. r=hsinyi

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final] Part 2: Update test case. r=hsinyi

Comment 10

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final#2] Part 1: Always send out emergencyCbModeChange event when received from ril. r=hsinyi

rebase

Comment 11

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [Final#2] Part 2: Update test case. r=hsinyi

rebase

Comment 12

[Szu-Yu Chen [:aknow] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=f5f85b5729b4

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/f7ac432b38b5
https://hg.mozilla.org/integration/b2g-inbound/rev/035ce0773762

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f7ac432b38b5
https://hg.mozilla.org/mozilla-central/rev/035ce0773762