712944 - B2G telephony: ensure error scenarios are covered

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Philipp von Weitershausen [:philikon]]

There are various error scenarios that may not be adequately covered by the API spec. They're definitely not covered by the current implementation. Things like: what if the radio isn't powered on, or there's no network, or we've maxed out the number of concurrent calls, etc. Also, should invalid numbers and dropped calls notify as errors somehow, or are these simply "disconnected" events on the respective call objects?

Comment 1

[Price Tseng]

Attachment: Define the ErrorHandler object in charge of handling RIL request errors

Hi Philipp:

After fixing Bug 713451, we build the notification mechanism of RIL request error so that we can handle the errors in RadioInterfaceLayer.
As shown in Attachment, I implement the ErrorHandler object which is responsible for handling RIL request error and notifying the APs.

I have the question:
How many errors we should handle in current status and what are them? Should we define these errors and how to handle them for more details? How much information should the notification messages take when they are sent to APs?

Thanks

Comment 2

[Philipp von Weitershausen [:philikon]]

Comment on attachment 604702 [details] [diff] [review]
Define the ErrorHandler object in charge of handling RIL request errors

Thanks for the patch, Price. I will have a look. Next time please make sure to flag me for review or feedback!

Comment 3

[Philipp von Weitershausen [:philikon]]

Comment on attachment 604702 [details] [diff] [review]
Define the ErrorHandler object in charge of handling RIL request errors

Thanks, Price! This is a great start.

As far as addressing the problems/questions outlined in comment 0, I think the best course of action is to propose a change to the WebTelephony API, e.g. by specifying a new "error" event (along with the "onerror" attribute) as well as when it will be sent. Note that in comment 0 I also raised the point that in some situations, just dropping the call (notifying "disconnected") might be enough, e.g. when there's no network -- I'm not sure, this should be discussed (and also backed with some analysis: what kind of information does rild give us?)

As far as the implementation is concerned, there will be a dom/telephony piece that adds the error events, and then a way for nsIRadioInterfaceLayer to notify the DOM of such errors. I don't think this will require a new object such as the ErrorHandler that you're introducing here. That said, you're on the the right track in terms of analyzing the "error" message from the RIL worker, but I get the feeling it might be better to first discriminate by the original request type -- message.type is probably a better value than message.rilRequestType. Or perhaps we should even do some of the error preprocessing in the RIL worker. Come to think of it, that's probably best.

Comment 4

[Price Tseng]

Attachment: The new error notification mechanism

Hi Philipp:

According to comment 3, I modify the error notification mechanism. Please refer to the attachment.

When the errors are detected in Buf::processParcel(), I keep track of the error in RIL::setRILLastReqestError(). 
Then in RIL::handleParcel(), the member function relative to the RIL request is invoked. 
Take the REQUEST_DIAL request for example, the RIL::REQUEST_DIAL() is called.
In RIL::REQUEST_DIAL(), check RIL::rilLastRequestError. And if the error occurs, send this message to RadioInterfaceLayer through RIL::sendDOMMessage().
The RadioInterfaceLayer object receives messages and then parses the errors in RadioInterfaceLayer::handleRequestError().
I add a function: notifyRequestError() for the nsIRILTelephonyCallback interface so that the WebTelephony DOM object can receive the error.

Comment 5

[Philipp von Weitershausen [:philikon]]

Comment on attachment 607121 [details] [diff] [review]
The new error notification mechanism

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

Hi Price! Thanks for your updated patch. It doesn't look like you have addressed much of my comments/requests in comment 3, though. I suggest to start by sketching out an error event with failure causes for the WebTelephony API. Then we can work our way backwards to the RIL.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +466,5 @@
> +  handleRequestError: function handleRequestError(message) {
> +	debug("RIL request error: " + JSON.stringify(message));
> +	switch (message.rilRequestType) {
> +      case RIL.REQUEST_DIAL:
> +      case RIL.REQUEST_HANGUP:

Inconsistent indentation here (never ever use tabs!)

::: dom/system/gonk/nsIRadioInterfaceLayer.idl
@@ +87,5 @@
> +   *
> +   * @return true to continue enumeration or false to cancel.
> +   */
> +  void notifyRequestError(in unsigned short rilRequestType,
> +                          in unsigned short rilRequestError);

This is too simplistic. The Telephony, SMS, or 3G DOM implementations don't know anything about RIL parcels and their error codes. We should provide more meaningful error notifications that are tailored to those cases. This bug is about telephony, so let's focus on that.

Also, changing an interface (IDL) requires you to update the UUID with newly generated one.

::: dom/system/gonk/ril_worker.js
@@ +658,5 @@
> +   */  
> +  rilLastRequestError: null,
> +  setRILLastReqestError: function setRILLastReqestError(rilRequestError) {
> +    this.rilLastRequestError = rilRequestError;
> +  },

This isn't Java, we don't need setters just to update a property.

But I don't understand the need for this property. In RIL.processParcel() we pass the `options` object to the handler, so the handlers already have access to `options.rilRequestError`.

@@ +1661,5 @@
> +    this.sendDOMMessage({type: "error",
> +                         rilRequestType: 10,
> +	                     rilRequestError: this.rilLastRequestError});	
> +    return;
> +  }

This is not enough. We should use REQUEST_LAST_CALL_FAIL_CAUSE to find out *why* the call failed and then notify the main thread with that information.

Comment 6

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer

Hi Philipp:

According to our former discussion, I modify the error notification mechanism as follow:

For example, when user dials the phone and the error occurs, |RIL::REQUEST_DIAL| is invoked to get the "Last Call Fail Cause".
|RIL::REQUEST_LAST_CALL_FAIL_CAUSE| tries to translate rilLastCallFailCause into a string and then sends to UI thread.
The UI thread sends the error string to DOM through |nsIRILTelephonyCallback::notifyError|

Comment 7

[Philipp von Weitershausen [:philikon]]

Comment on attachment 613888 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer

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

Slowly getting there! Please see below for my comments. Some of them we already discussed in person a couple of days ago.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +290,5 @@
>          this.radioState.msisdn = message.msisdn;
>          break;
> +      case "telephony-failed":
> +        this.notifyError(message);
> +        break;

`callError` would be a more descriptive message name and `handleCallError()` would follow the existing naming conventions for methods. Please also add this code next to the call-related stanzas and not just at the end.

::: dom/system/gonk/nsIRadioInterfaceLayer.idl
@@ +85,5 @@
> +   * @param errorDescription
> +   *        Error from RIL.
> +   */
> +  void notifyError(in long callIndex,
> +                   in AString errorDescription);

`errorDescription` is not a good name for this parameter because it's an identifier, not really a description (see also my comments regarding your error constants in ril_const.js). Let's just call parameter `error`.

::: dom/system/gonk/ril_consts.js
@@ +1391,5 @@
> +const CALL_FAIL_STRING_CALL_BARRED = "Call Barred";
> +const CALL_FAIL_STRING_FDN_BLOCKED = "FDN Blocked";
> +const CALL_FAIL_STRING_IMSI_UNKNOWN_IN_VLR = "IMSI Unknown In VLR";
> +const CALL_FAIL_STRING_IMEI_NOT_ACCEPTED = "IMEI Not Accepted";
> +const CALL_FAIL_STRING_ERROR_UNSPECIFIED = "Error Unspecified";

These should be named GECKO_CALL_ERROR_* and placed next to the other GECKO_* constants.

Also the string values should follow the camelCase convention that we have in the DOM, e.g.: "badNumber", "busy", "congested", "barred", "unspecified", "imeiNotAccepted", etc. These values should be enumerated and explained in the nsIDOMTelephonyCall interface documentation (in the code and on https://wiki.mozilla.org/WebAPI/WebTelephony).

Speaking of nsIDOMTelephonyCall, you need to add the `onerror` handler to it.

::: dom/system/gonk/ril_worker.js
@@ +1845,5 @@
> +  if (options.rilRequestError) {
> +    options.callIndex = -1; // The connection is not established yet.
> +    this.getFailCauseCode(options);
> +    return;
> +  }

Great! I wonder: should we also call this.getFailCauseCode() in other scenarios, e.g. when a call disconnects/drops?

@@ +1877,5 @@
>  };
>  RIL[REQUEST_CONFERENCE] = null;
>  RIL[REQUEST_UDUB] = null;
> +RIL[REQUEST_LAST_CALL_FAIL_CAUSE] = function REQUEST_LAST_CALL_FAIL_CAUSE(length, options) {
> +  if (DEBUG) debug("RIL::REQUEST_LAST_CALL_FAIL_CAUSE(), options: " + JSON.stringify(options));

This seems pretty useless to me, please remove it. (Also, `::` is not a valid operator in Javascript!)

@@ +1883,5 @@
> +  if (length) {
> +    num = Buf.readUint32();
> +  }
> +  if (!num)
> +    return;

Nit: always need braces.

@@ +1886,5 @@
> +  if (!num)
> +    return;
> +
> +  var rilLastCallFailCause = Buf.readUint32();
> +  var rilLastCallFailCauseDescription = null;

Nit: always use `let`. Also, your variable names are pretty long and unwieldy. `failCause` and `description` would work just as well -- but like I said above, this isn't really a `description`, it's the Gecko/DOM error string, so you probably want to call it that.

@@ +1888,5 @@
> +
> +  var rilLastCallFailCause = Buf.readUint32();
> +  var rilLastCallFailCauseDescription = null;
> +  switch (rilLastCallFailCause) {
> +    case CALL_FAIL_UNOBTAINABLE_NUMBER:

Ugh... Please just use a mapping! For instance:

  const RIL_CALL_FAILCAUSE_TO_GECKO_CALL_ERROR = {};
  RIL_CALL_FAILCAUSE_TO_GECKO_CALL_ERROR[CALL_FAIL_UNOBTAINABLE_NUMBER] = GECKO_CALL_ERROR_BADNUMBER;
  etc.

You can define this in ril_const.js and then just look it up here.

@@ +1922,5 @@
> +    default:
> +      if (DEBUG) debug("Unknown Last Call Fail Cause");
> +      break;
> +  }
> +  this.sendDOMMessage({type: "telephony-failed",

"callError" would be a more descriptive message name.

::: dom/telephony/Telephony.cpp
@@ +499,5 @@
>    return NS_OK;
>  }
>  
> +NS_IMETHODIMP
> +Telephony::NotifyError(PRInt32 aCallIndex, const nsAString& aErrorDescription)

This whole method is a mess in terms of coding style. Please familiarize yourself with https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide and follow it in all code changes.

In particular, please fix:
* blocks always need curly braces, including 1-line blocks.
* always indent by two spaces
* always add a space on both sides of operators

@@ +516,5 @@
> +  }
> +
> +  // Error occurs, change the call state of a specified call object to DISCONNECTED.
> +  if (index != -1)
> +    mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);

Also set `call.error` and notify the `error` event!

Comment 8

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer : V2

Fix some incorrect coding styles

Comment 9

[Price Tseng]

(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> Comment on attachment 613888 [details] [diff] [review]
> Error handling for the telephony part of RadioInterfaceLayer
> 
> Review of attachment 613888 [details] [diff] [review]:
> -----------------------------------------------------------------
> Speaking of nsIDOMTelephonyCall, you need to add the `onerror` handler to it.
> 
> @@ +516,5 @@
> > +  }
> > +
> > +  // Error occurs, change the call state of a specified call object to DISCONNECTED.
> > +  if (index != -1)
> > +    mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> 
> Also set `call.error` and notify the `error` event!

According to the comment 4 in Bug 717462, I think I continue to implement the error handling mechanism in DOM.
How do you think?

Comment 10

[Philipp von Weitershausen [:philikon]]

(In reply to Price Tseng from comment #9)
> (In reply to Philipp von Weitershausen [:philikon] from comment #7)
> > @@ +516,5 @@
> > > +  }
> > > +
> > > +  // Error occurs, change the call state of a specified call object to DISCONNECTED.
> > > +  if (index != -1)
> > > +    mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> > 
> > Also set `call.error` and notify the `error` event!
> 
> According to the comment 4 in Bug 717462, I think I continue to implement
> the error handling mechanism in DOM.

Yeah, that's fine. I don't mind doing it in a different patch/bug.

Comment 11

[Philipp von Weitershausen [:philikon]]

Comment on attachment 614292 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer : V2

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

Excellent. Just one or two more nits and a little bit of bikeshedding about the error strings.

::: dom/system/gonk/ril_consts.js
@@ +1363,5 @@
>  const GECKO_CARDSTATE_NETWORK_LOCKED = "network_locked";
>  const GECKO_CARDSTATE_NOT_READY      = null;
>  const GECKO_CARDSTATE_READY          = "ready";
>  
> +const GECKO_CALL_ERROR_UNOBTAINABLE_NUMBER = "unobtainableNumber";

Doesn't this just mean that it's a bad number? "badNumber" is so much shorter and much harder to mistype ;)

@@ +1364,5 @@
>  const GECKO_CARDSTATE_NOT_READY      = null;
>  const GECKO_CARDSTATE_READY          = "ready";
>  
> +const GECKO_CALL_ERROR_UNOBTAINABLE_NUMBER = "unobtainableNumber";
> +const GECKO_CALL_ERROR_NORMAL              = "normal";

I still don't know what a "normal" error means. I'm not sure this string makes a lot of sense. Please find out what it means so we can provide a better string here.

@@ +1367,5 @@
> +const GECKO_CALL_ERROR_UNOBTAINABLE_NUMBER = "unobtainableNumber";
> +const GECKO_CALL_ERROR_NORMAL              = "normal";
> +const GECKO_CALL_ERROR_BUSY                = "busy";
> +const GECKO_CALL_ERROR_CONGESTION          = "congestion";
> +const GECKO_CALL_ERROR_ACM_LIMIT_EXCEEDED  = "acmLimitExceeded";

I believe the "ACM limit" is a credit-related value determined by the carrier. It woudl be good to find out what it means, perhaps we can define a better string here.

@@ +1368,5 @@
> +const GECKO_CALL_ERROR_NORMAL              = "normal";
> +const GECKO_CALL_ERROR_BUSY                = "busy";
> +const GECKO_CALL_ERROR_CONGESTION          = "congestion";
> +const GECKO_CALL_ERROR_ACM_LIMIT_EXCEEDED  = "acmLimitExceeded";
> +const GECKO_CALL_ERROR_CALL_BARRED         = "callBarred";

Could just be "barred"

@@ +1371,5 @@
> +const GECKO_CALL_ERROR_ACM_LIMIT_EXCEEDED  = "acmLimitExceeded";
> +const GECKO_CALL_ERROR_CALL_BARRED         = "callBarred";
> +const GECKO_CALL_ERROR_FDN_BLOCKED         = "fdnBlocked";
> +const GECKO_CALL_ERROR_IMSI_UNKNOWN_IN_VLR = "imsiUnknownInVLR";
> +const GECKO_CALL_ERROR_IMEI_NOT_ACCEPTED   = "imeiNotAccepted";

IMSI and IMEI are GSM specific terms. We should de-GSM-ify those strings, e.g. "subscriberUnknown" and "deviceNotAccepted"

@@ +1372,5 @@
> +const GECKO_CALL_ERROR_CALL_BARRED         = "callBarred";
> +const GECKO_CALL_ERROR_FDN_BLOCKED         = "fdnBlocked";
> +const GECKO_CALL_ERROR_IMSI_UNKNOWN_IN_VLR = "imsiUnknownInVLR";
> +const GECKO_CALL_ERROR_IMEI_NOT_ACCEPTED   = "imeiNotAccepted";
> +const GECKO_CALL_ERROR_ERROR_UNSPECIFIED   = "errorUnspecified";

Could just be "unspecified"

::: dom/system/gonk/ril_worker.js
@@ +1888,5 @@
> +
> +  let failCause = Buf.readUint32();
> +  this.sendDOMMessage({type: "callError",
> +                       callIndex: options.callIndex,
> +                       error: RIL_CALL_FAILCAUSE_TO_GECKO_CALL_ERROR[failCause]});

A pattern that we're using in a bunch of other places is to just reuse the `options` object, particularly when it already contains information that we want to pass along anyway (like the `callIndex` here.) So for instance:

  options.type = "callError";
  options.error = RIL_CALL_FAILCAUSE_TO_GECKO_CALL_ERROR[failCause]
  this.sendDOMMessage(options);

::: dom/telephony/Telephony.cpp
@@ +504,5 @@
> +                        const nsAString& aError)
> +{
> +  PRInt32 index = -1;
> +  PRInt32 length = mCalls.Length();
> +  if (aCallIndex == -1) { // The connection is not established yet, remove the latest call object

Please put the commen ton its own line.

Comment 12

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer : V3

Modify the string of Last Call Fail Cause and incorrect coding style

Comment 13

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer : V4

Modify the string of Last Call Fail Cause and incorrect coding style

Comment 14

[Philipp von Weitershausen [:philikon]]

Comment on attachment 615265 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer : V4

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

Looking good! Just a few nits and a question for bent (see below).

::: dom/system/gonk/ril_consts.js
@@ +1372,5 @@
> +const GECKO_CALL_ERROR_CALL_BARRED         = "barred";
> +const GECKO_CALL_ERROR_FDN_BLOCKED         = "fdnBlocked";
> +const GECKO_CALL_ERROR_IMSI_UNKNOWN_IN_VLR = "subscriberUnknown";
> +const GECKO_CALL_ERROR_IMEI_NOT_ACCEPTED   = "deviceNotAccepted";
> +const GECKO_CALL_ERROR_ERROR_UNSPECIFIED   = "unspecified";

Following the existing convention, please call these GECKO_CALL_ERROR_BAD_NUMBER, GECKO_CALL_ERROR_NORMAL_CALL_CLEARING, etc. since they are web API constants and not RIL constants.

::: dom/telephony/Telephony.cpp
@@ +500,5 @@
>  }
>  
> +NS_IMETHODIMP
> +Telephony::NotifyError(PRInt32 aCallIndex,
> +                        const nsAString& aError)

nit: align arguments.

@@ +508,5 @@
> +
> +  // The connection is not established yet, remove the latest call object
> +  if (aCallIndex == -1) {
> +    if (length > 0) {
> +      index = length -1;

nit: spaces around operator.

@@ +518,5 @@
> +    index = aCallIndex;
> +  }
> +  if (index != -1) {
> +    mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> +  }

I just realized, we might need to first go through the DISCONNECTING state before we can go to DISCONNECTED. I think we can just do

  mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTING);
  mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);

Asking bent for feedback on that.


This reminds me of a question I asked in comment 7:

> I wonder: should we also call this.getFailCauseCode() in other scenarios, e.g.
> when a call disconnects/drops?

The question is: if a call is dropped mid-stream, can we also get the fail cause? Should we notify the error? If so, we should make sure we don't notify DISCONNECTED twice! -- We can discuss this in a follow up bug. Please file this and investigate. Thanks!

Comment 15

[Price Tseng]

> @@ +518,5 @@
> > +    index = aCallIndex;
> > +  }
> > +  if (index != -1) {
> > +    mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> > +  }
> 
> I just realized, we might need to first go through the DISCONNECTING state
> before we can go to DISCONNECTED. I think we can just do
> 
>  
> mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTING);
>  
> mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> 
> Asking bent for feedback on that.
I discuss this with Hsin-Yi, and her response is: Before the call object is released, the call state changes to DISCONNECTED. But not every scenario makes the call state change from DISCONNECTING to DISCONNECTED. Only when the user hangs up the phone, the phone tries to stop the connection so the call state switches to DISCONNETING. Then when the connection doesn’t exist, the call state changes to DISCONNECTED. However, when errors occur or the other hangs up the phone, the call state doesn’t move to DISCONNECTING before DISCONNECTED.

> 
> 
> This reminds me of a question I asked in comment 7:
> 
> > I wonder: should we also call this.getFailCauseCode() in other scenarios, e.g.
> > when a call disconnects/drops?
> 
> The question is: if a call is dropped mid-stream, can we also get the fail
> cause? Should we notify the error? If so, we should make sure we don't
> notify DISCONNECTED twice! -- We can discuss this in a follow up bug. Please
> file this and investigate. Thanks!
I already filed a bug (Bug746886) for further discussion.

Comment 16

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer : V5

Modify some errors according to comment 14

Comment 17

[Price Tseng]

(In reply to Philipp von Weitershausen [:philikon] from comment #14)
> Comment on attachment 615265 [details] [diff] [review]
> Error handling for the telephony part of RadioInterfaceLayer : V4
> 
> Review of attachment 615265 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ +518,5 @@
> > +    index = aCallIndex;
> > +  }
> > +  if (index != -1) {
> > +    mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> > +  }
> 
> I just realized, we might need to first go through the DISCONNECTING state
> before we can go to DISCONNECTED. I think we can just do
> 
>  
> mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTING);
>  
> mCalls[index]->ChangeState(nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED);
> 
> Asking bent for feedback on that.
> 
Hi Bent:
I move the source codes of state change to the TelephonyCall::NotifyError() function.

Comment 18

[Philipp von Weitershausen [:philikon]]

(In reply to Price Tseng from comment #15)
> I discuss this with Hsin-Yi, and her response is: Before the call object is
> released, the call state changes to DISCONNECTED. But not every scenario
> makes the call state change from DISCONNECTING to DISCONNECTED. Only when
> the user hangs up the phone, the phone tries to stop the connection so the
> call state switches to DISCONNETING. Then when the connection doesn’t exist,
> the call state changes to DISCONNECTED. However, when errors occur or the
> other hangs up the phone, the call state doesn’t move to DISCONNECTING
> before DISCONNECTED.

Hmm, you're right. In that case your patch is indeed consistent with the current behaviour. But I wonder if the current behaviour is a bug. We can investigate that in a follow-up, though.

Comment 19

[Philipp von Weitershausen [:philikon]]

Comment on attachment 616896 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer : V5

Pinging Ben Turner a little bit harder on this.

Comment 20

[Philipp von Weitershausen [:philikon]]

Comment on attachment 616896 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer : V5

Spoke w Johnny about the event dispatch. As suspected, this patch is fine.

Comment 21

[Philipp von Weitershausen [:philikon]]

Comment on attachment 616896 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer : V5

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

::: dom/system/gonk/ril_consts.js
@@ +1372,5 @@
> +const GECKO_CALL_ERROR_BARRED                 = "barred";
> +const GECKO_CALL_ERROR_FDN_BLOCKED            = "fdnBlocked";
> +const GECKO_CALL_ERROR_SUBSCRIBER_UNKNOWN     = "subscriberUnknown";
> +const GECKO_CALL_ERROR_DEVICE_NOT_ACCEPTED    = "deviceNotAccepted";
> +const GECKO_CALL_ERROR_UNSPECIFIED            = "unspecified";

As noted in bug 717462, we're going to use DOMError. So we should change these constants to match the DOMError naming convention, e.g. "BadNumberError", etc.

Price, could you make this change real quick? Then we can land these improvements! Thanks!

Comment 22

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer : V6

Change string to match the DOMError naming convention according to Comment 21

Comment 23

[Price Tseng]

Attachment: Error handling for the telephony part of RadioInterfaceLayer : V6

Change string to match the DOMError naming convention according to Comment 21

Comment 24

[Philipp von Weitershausen [:philikon]]

Comment on attachment 620600 [details] [diff] [review]
Error handling for the telephony part of RadioInterfaceLayer : V6

Thanks!

Comment 25

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6c43ace1393

Comment 26

[Ed Morley [:emorley]]

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

Comment 27

[Ed Morley [:emorley]]

Sorry, wrong url in comment 26, should be:
https://hg.mozilla.org/mozilla-central/rev/f6c43ace1393