1056625 - [Settings] crash when tapping on either SIM in SIM settings (of Messaging Settings) if the SIM pin is locked

Status: VERIFIED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Eric Chang [:ericcc] [:echang]]

### STR
1. SIM pin turn ON.
2. Reboot.
3. Skip SIM pin on boot.
4. 'Settings' -> 'Messaging Settings' -> 'SIM settings'.
5. Tap the one with SIM pin locked.

### Actual
Settings crashes

### Expected
No crash

### Version
Gaia      3584b2723412ed3299c6761f465885d80651c87e
Gecko     https://hg.mozilla.org/mozilla-central/rev/e7806c9c83f3
BuildID   20140820160201
Version   34.0a1 
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

### Last Good Build
Gaia      aa8aace12d65956dd9525da5dac66e0d3b28597f
Gecko     https://hg.mozilla.org/mozilla-central/rev/37ac55a26014
BuildID   20140817160201
Version   34.0a1
ro.build.version.incremental=eng.ericcc.20140815.103628
ro.build.date=Fri Aug 15 10:36:36 CST 2014

Comment 1

[Eric Chang [:ericcc] [:echang]]

[Blocking Requested - why for this release]: Crash

Comment 2

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

When calling `var req = mobileMessage.getSmscAddress(this.index);` in locked simcard, then it makes the app crash. 

Eric, do you know who should we ni? in Gecko for this part ?!

Comment 3

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

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #2)
> When calling `var req = mobileMessage.getSmscAddress(this.index);` in locked
> simcard, then it makes the app crash. 
> 
> Eric, do you know who should we ni? in Gecko for this part ?!

Our ril team will take a look. Get back to you if we have new findings :)

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Root cause found:
1. In nsIMobileMessageCallback.notifyGetSmscAddressFailed(in long error), an XXX_ERROR defined in nsIMobileMessageCallback is expected. [1]
2. However, in RadioInterfaceLayer, we pass the response.errorMsg directly without converting it into corresponding errorCode. [2]
3. Hence, a MOZ_CRASH will be risen while ConvertErrorCodeToErrorString() in MobileMessageCallback.cpp . [3]

A test case for failure scenario is required to cover this regression.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/interfaces/nsIMobileMessageCallback.idl#56
[2] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#4252
[3] http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/MobileMessageCallback.cpp#64

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Reply nsIMobileMessageCallback.NOT_FOUND_ERROR instead of response.errorMsg from ril_worker when failed to getSmscAddress().

a MOZ_CRASH at ConvertErrorCodeToErrorString() in MobileMessageCallback.cpp because an invalid errorCode was provided in RadioInterfaceLayer.
This patch is to reply nsIMobileMessageCallback.NOT_FOUND_ERROR when failure to getSmscAddress().

Hi Vicamo,

May I have your review for the quick fix of this crash?

Thanks!

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8477257 [details] [diff] [review]
Reply nsIMobileMessageCallback.NOT_FOUND_ERROR instead of response.errorMsg from ril_worker when failed to getSmscAddress().

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

Thank you for this quick fix!

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

update try srever result:
https://tbpl.mozilla.org/?tree=Try&rev=495eb6ce2a52

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/8bf563308159

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8bf563308159

Comment 10

[Eric Chang [:ericcc] [:echang]]

Gaia-Rev        b542080231bb9cdbdb282ab926965ad3f91f7460
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/d614ae398c03
Build-ID        20140923155123
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  27
FW-Date         Thu Sep  4 14:59:02 CST 2014
Bootloader      L1TC10011800