951958 - B2G RIL: set/get preferred network type error message is not set accordingly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Jessica Jong [:jessica] (inactive)]

RILContentHelper.handleSimpleRequest() relies on errorMsg to fire request error, we should set the errorMsg accordingly in ril_worker.

Comment 1

[Jessica Jong [:jessica] (inactive)]

Attachment: proposed patch, v1.

Comment 2

[Jessica Jong [:jessica] (inactive)]

Attachment: proposed patch, v2.

Comment 3

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8349815 [details] [diff] [review]
proposed patch, v2.

Since emulator's modem is multimode, we are not able to make modem return error while setting preferred network type, so no test cases for this yet. :(

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

Comment 4

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

Comment on attachment 8349815 [details] [diff] [review]
proposed patch, v2.

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

::: dom/system/gonk/ril_worker.js
@@ +5882,5 @@
>  
>    options.success = (options.rilRequestError == ERROR_SUCCESS);
> +  if (!options.success) {
> +    options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError];
> +  }

Seems redundant to check both options.rilRequestError and options.success.

How about this:

if (options.rilRequestError) {
    options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError];
}

@@ +5890,5 @@
>    options.success = (options.rilRequestError == ERROR_SUCCESS);
> +  if (!options.success) {
> +    options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError];
> +    this.sendChromeMessage(options);
> +    return;

ditto:

Comment 5

[Jessica Jong [:jessica] (inactive)]

Attachment: proposed patch, v3.

Hsinyi, I am keeping the option.success as discussed, as it is used in RadioInterfaceLayer. Please check if it's okay this way. Thanks.

Comment 6

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

Comment on attachment 8349913 [details] [diff] [review]
proposed patch, v3.

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

Thank you.

Comment 7

[Jessica Jong [:jessica] (inactive)]

Thank you, Hsinyi.

Try for patch v3: https://tbpl.mozilla.org/?tree=Try&rev=72efb06e0504

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1136327bc10f

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1136327bc10f