751550 - B2G Emergency Calls: Notify the DOM about wrong emergency numbers.

Status: RESOLVED DUPLICATE of bug 765231

(Core :: DOM: Device Interfaces, defect)

Comment 1

[José Antonio Olivera Ortega [:jaoo]]

Once bug 712944 has landed we have to send an error event to the DOM when using wrong emergency numbers when making an emergency call.

Comment 2

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v1

WIP patch. Just noticed emergengy calls do not work on ICS. It seems the DIAL_EMERGENCY_CALL request type is no longer available. I have to touch a bit our loved initRILQuirks() function. I'll file a bug for fixing the problem. Current WIP has also another problem I don't know the cause yet. Any feedback is also welcome ;). Find attached a log with some information.

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Error log

Error log contaning a |adb logcat| output during a test.

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 625661 [details] [diff] [review]
WIP v1

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

::: dom/system/gonk/ril_worker.js
@@ +1145,5 @@
> +        options.callIndex = -1;
> +        options.type = "callError";
> +        options.error = GECKO_CALL_ERROR_BAD_NUMBER;
> +        debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
> +        this.sendDOMMessage(options);

I would have a slight preference for morphing `options` rather than creating a new object. That way all the info that the main thread added to the message (e.g. the number) will be preserved.

Looks good otherwise!

Comment 5

[Dietrich Ayala (:dietrich)]

Not absolutely required for shipping.

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v2

I go back to work in Emergency calls. I hope we can solve this remaining piece asap. This patch addresses philikon's comments. This is a WIP patch because it doesn't work at all. I really want some feedback on it. In fact, emergency call works in otoro and galaxys2 (the latter needs to have [1] merged) but the functionality we want to implement here doesn't. Here I paste some traces from my tests.

otoro

I/Gecko   (  115): -*- RadioInterfaceLayer: Received 'RIL:Dial' message from content process
I/Gecko   (  115): -*- RadioInterfaceLayer: Dialing 11
I/Gecko   (  115): RIL Worker: Received DOM message {"type":"dial","number":"11"}
I/Gecko   (  115): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
I/Gecko   (  115): -*- RadioInterfaceLayer: Received message from worker: {"type":"callError","number":"11","callIndex":-1,"error":"BadNumberError"}
F/libc    (  115): Fatal signal 11 (SIGSEGV) at 0x00000080 (code=1)
I/DEBUG   (  100): debuggerd committing suicide to free the zombie!
I/DEBUG   (  414): debuggerd: Jun 12 2012 11:23:02

galaxys2

I/Gecko   ( 1844): RIL Worker: Received DOM message {"type":"dial","number":"123"}
I/Gecko   ( 1844): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
I/Gecko   ( 1844): -*- RadioInterfaceLayer: Received message from worker: {"type":"callError","number":"123","callIndex":-1,"error":"BadNumberError"}
F/libc    ( 1844): Fatal signal 11 (SIGSEGV) at 0x00dc9fb8 (code=2)

Is it something wrong here?

+        // The connection is not established yet.
+        options.callIndex = -1;
+        options.type = "callError";
+        options.error = GECKO_CALL_ERROR_BAD_NUMBER;
+        debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
+        this.sendDOMMessage(options);

[1] https://github.com/mozilla-b2g/android-device-galaxys2/pull/11

Comment 7

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

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #6)
> Created attachment 632237 [details] [diff] [review]
> WIP v2
> 
> I go back to work in Emergency calls. I hope we can solve this remaining
> piece asap. This patch addresses philikon's comments. This is a WIP patch
> because it doesn't work at all. I really want some feedback on it. In fact,
> emergency call works in otoro and galaxys2 (the latter needs to have [1]
> merged) but the functionality we want to implement here doesn't. Here I
> paste some traces from my tests.
> 
> otoro
> 
> I/Gecko   (  115): -*- RadioInterfaceLayer: Received 'RIL:Dial' message from
> content process
> I/Gecko   (  115): -*- RadioInterfaceLayer: Dialing 11
> I/Gecko   (  115): RIL Worker: Received DOM message
> {"type":"dial","number":"11"}
> I/Gecko   (  115): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
> I/Gecko   (  115): -*- RadioInterfaceLayer: Received message from worker:
> {"type":"callError","number":"11","callIndex":-1,"error":"BadNumberError"}
> F/libc    (  115): Fatal signal 11 (SIGSEGV) at 0x00000080 (code=1)
> I/DEBUG   (  100): debuggerd committing suicide to free the zombie!
> I/DEBUG   (  414): debuggerd: Jun 12 2012 11:23:02
> 
> galaxys2
> 
> I/Gecko   ( 1844): RIL Worker: Received DOM message
> {"type":"dial","number":"123"}
> I/Gecko   ( 1844): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
> I/Gecko   ( 1844): -*- RadioInterfaceLayer: Received message from worker:
> {"type":"callError","number":"123","callIndex":-1,"error":"BadNumberError"}
> F/libc    ( 1844): Fatal signal 11 (SIGSEGV) at 0x00dc9fb8 (code=2)
> 
> Is it something wrong here?
> 
> +        // The connection is not established yet.
> +        options.callIndex = -1;
> +        options.type = "callError";
> +        options.error = GECKO_CALL_ERROR_BAD_NUMBER;
> +        debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
> +        this.sendDOMMessage(options);
> 
> [1] https://github.com/mozilla-b2g/android-device-galaxys2/pull/11

I wonder this is the same issue in Bug 764690.

Comment 8

[Philipp von Weitershausen [:philikon]]

Comment on attachment 632237 [details] [diff] [review]
WIP v2

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

Looks great! Please address the nits *and write tests for this*. Thanks!

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +11,5 @@
>  
>  var RIL = {};
>  Cu.import("resource://gre/modules/ril_consts.js", RIL);
>  
> +const DEBUG = true; // set to true to see debug messages

ahem.

::: dom/system/gonk/ril_worker.js
@@ +31,5 @@
>  
>  // We leave this as 'undefined' instead of setting it to 'false'. That
>  // way an outer scope can define it to 'true' (e.g. for testing purposes)
>  // without us overriding that here.
> +let DEBUG = true;

cough.

@@ +1447,5 @@
> +        // The connection is not established yet.
> +        options.callIndex = -1;
> +        options.type = "callError";
> +        options.error = GECKO_CALL_ERROR_BAD_NUMBER;
> +        debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");

All debug statements should be gated by an `if (DEBUG)`, but there's really no need for that here since in debug mode we print all messages between the threads.

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v3

Nits addressed. This functionality depends on bug 764690, thanks hsinyi!. I made a test for checking if everything is ok and what I see was:

I/Gecko   ( 1199): -*- RILContentHelper: Dialing 111
I/Gecko   ( 1199): -*- RadioInterfaceLayer: Received 'RIL:Dial' message from content process
I/Gecko   ( 1199): -*- RadioInterfaceLayer: Dialing 111
I/Gecko   ( 1199): RIL Worker: Received DOM message {"type":"dial","number":"111"}
I/Gecko   ( 1199): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
I/Gecko   ( 1199): -*- RadioInterfaceLayer: Received message from worker: {"type":"callError","number":"111","callIndex":-1,"error":"BadNumberError"}
I/Gecko   ( 1199): -*- RILContentHelper: Received message 'RIL:CallError': {"type":"callError","number":"111","callIndex":-1,"error":"BadNumberError"}
I/Gecko   ( 1199): -*- RILContentHelper: Requesting enumeration of calls for callback: [xpconnect wrapped nsIRILTelephonyCallback]
I/Gecko   ( 1199): -*- RILContentHelper: Registered telephony callback: [xpconnect wrapped nsIRILTelephonyCallback]

Should I see anything in the dailer app when the error occurs?

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Philipp von Weitershausen [:philikon] from comment #8)
> Comment on attachment 632237 [details] [diff] [review]
> WIP v2
> 
> Review of attachment 632237 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great! Please address the nits *and write tests for this*. Thanks!

Philipp, regarding your request about tests what about if I file a bug for adding tests for the whole emergency calls functionality?

Comment 11

[Philipp von Weitershausen [:philikon]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #10)
> Philipp, regarding your request about tests what about if I file a bug for
> adding tests for the whole emergency calls functionality?

Or just do it as part of this bug.

Comment 12

[Philipp von Weitershausen [:philikon]]

Comment on attachment 634118 [details] [diff] [review]
WIP v3

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

Great. Still needs tests.