956322 - Trying to download an MMS fails with 'Missing SIM card'

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Landry Breuil (:gaston)]

On a ZTE Open device - was running 1.1.0b04 firmware from ZTE (carrier is free.fr), got an MMS notification. on the 2/1. Note that i successfully downloaded an MMS on the 31/12.

Tried upgrading the device on the 4/1, fastboot seems broken so i only upgraded gecko/gaia from master/1.4-prerelease, if i try to download the pending MMS when i have data coverage, i get 'Missing SIM card' error message, which seems to be triggered by http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/MmsService.js#2271 or line 2313 just below.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: 2014-01-02-17-36-42.png

Comment 2

[Julien Wajsberg [:julienw]]

Hey Gene,

In the following line :

2275         serviceId = gRil.getClientIdByIccId(aMessageRecord.iccId);

I think the "iccId" property was not set because the message was received before the property existed. Therefore I think you need a fallback code here in case iccId is not set.

Requesting 1.3? because this is an issue from the DSDS work. Adding dataloss because MMS data can be lost if the user migrates from 1.2 to 1.3.

Here is an updated STR:
1. have a phone in v1.2
2. go to Settings > Messaging Settings
3. Set Auto Retrieve to "Off"
4. Send a MMS to this phone from another phone
5. upgrade the phone to v1.3
6. Try to download the MMS
=> attachment 8355565 [details]

QAWanted to test this updated STR and see if you can reproduce.

Comment 3

[Matthew Vaughan]

I am not able to reproduce this issue using the 01/02/14 1.2 and 1.3 builds. The MMS will eventually come through when I go from 1.2 to 1.3.

To be honest, I could probably not be getting a repro due to how I "update". I assume there must be another way to update in a way similar to what the end user will see (like some how changing the update channel to 1.3 on the 1.2 build?), and that you are doing it that way. If this is so, are there any directions or web pages that can help me understand how to update the build that way?

Removing QAWanted for now.

Comment 4

[Julien Wajsberg [:julienw]]

Matthew, how do you "update" exactly ? Are you wiping your data ?

To be fair I didn't really try this STR, I'm merely guessing. I can try to reproduce on monday (keeping my needinfo).

I would:
* flash a 1.2 build
* do my STR
* flash a new gecko and a new gaia without wiping my data
And see what happens.

Comment 5

[Matthew Vaughan]

Since this is how you would perform the STR(In reply to Julien Wajsberg [:julienw] (PTO until January 6th) from comment #4)
> Matthew, how do you "update" exactly ? Are you wiping your data ?

The way I updated the device during the STR is by simply flashing 1.3 over 1.2. I do not wipe my data after that since I believe it would replicate more of an end user scenario.
 
> I would:
> * flash a 1.2 build
> * do my STR
> * flash a new gecko and a new gaia without wiping my data
> And see what happens.

So it looks like I did perform the update the way you would, and I can say that I could not get this issue to reproduce after 5 attempts. I am not sure what the frequency of this issue is so I can try to repro it more if needed.

Comment 6

[Landry Breuil (:gaston)]

Fwiw, i was coming from 1.1 to 1.4 (yeah i know big jump) - and for me it's 100% reproducible on the pending mms i have - as discussed with :julienw on IRC, i tried the following:

adb pull /system/b2g/omni.ja
unzip omni.ja
edit components/MmsService.js in-place, changing those two lines 
2275       serviceId = gRil.getClientIdByIccId(aMessageRecord.iccId || 0);
2413       serviceId = gRil.getClientIdByIccId(iccId || 0);
rezip omni.ja2
adb shell stop b2g
adb push omni.ja2 /system/b2g/omni.ja
adb reboot

and the dreaded message is still shown when i try to download that MMS, with data available.

Should this bug block 854326 ?

Additional question, where are the debug() calls going ? in logcat ? How can i see them to narrow down the problem ?

Comment 7

[Landry Breuil (:gaston)]

Backtracking from the seen string in gaia-l10n: http://mxr.mozilla.org/gaia/search?string=MissingSimCard
Seems to match http://mxr.mozilla.org/gaia/source/apps/sms/js/dialog.js#169
leading to NoSimCardError, looked in m-c:
http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/MobileMessageCallback.cpp#97

The error message i'm seeing is definitely coming from one of those: http://mxr.mozilla.org/mozilla-central/ident?i=NO_SIM_CARD_ERROR

Comment 8

[Landry Breuil (:gaston)]

The pending MMS notification was received on the 2, and it's available until the 9 - of course i'd like to get it before :)

Comment 9

[Landry Breuil (:gaston)]

Comment on attachment 8355565 [details]
2014-01-02-17-36-42.png

As for the screenshot, is it possible to remove/hide it ? there's a mobile number on it.. and the message is definitely identified as 'Missing SIM Card', so the pic has little value now.

Comment 10

[Landry Breuil (:gaston)]

I tried rolling back to 1.2 (taking a system.img from https://daylightpirates.org/b2g_inari_nightly_builds/b2g_inari_v1.2_2014-01-03_605ee4d.tar.gz, unyaffs2 -v --yaffs-ecclayout ~/b2g_inari_v1.2_2014-01-03_605ee4d/out/target/product/inari/system.img system, adb push system/b2g /system/b2g and same for userdata.img/local/webapps) but all i got was an internal error from MMS/SMS service failing to read its internal database. I suppose that was to be expected, but worth a try. Came back to 1.4, the SMS app can read its database (where are the SMS stored, btw ?) but i still get that error when trying to fetch the pending MMS

Comment 11

[Gene Lian (I already quit Mozilla)]

(In reply to Landry Breuil (:gaston) from comment #10)
> I tried rolling back to 1.2 but all i got was an internal error from MMS/SMS
> service failing to read its internal database. I suppose that was to be
> expected, but worth a try.

Just a quick response to this. In our current DB updating schema, we only allow upgrade but no downgrade.

Comment 12

[Gene Lian (I already quit Mozilla)]

(In reply to Julien Wajsberg [:julienw] (PTO until January 6th) from comment #2)
> Hey Gene,
> 
> In the following line :
> 
> 2275         serviceId = gRil.getClientIdByIccId(aMessageRecord.iccId);
> 
> I think the "iccId" property was not set because the message was received
> before the property existed. Therefore I think you need a fallback code here
> in case iccId is not set.

Nice catch! Julien. That's exactly the root cause. We intentionally did this just because we don't have knowledge about which SIM is supposed to use to download the MMS after the DSDS scenario is considered, so we just stop downloading the MMS which has been saved during the previous version. Maybe this check is too strict, though.

Comment 13

[Gene Lian (I already quit Mozilla)]

Attachment: Patch

Comment 14

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

Comment on attachment 8356009 [details] [diff] [review]
Patch

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

This is also the only thing we can do because WDP address info was not stored in db so there is no way to try matching 1.2 messages with your SIM card(s).

Comment 15

[Landry Breuil (:gaston)]

(In reply to Gene Lian [:gene] (PTO Dec. 25 ~ Jan. 5) from comment #11)
> (In reply to Landry Breuil (:gaston) from comment #10)
> > I tried rolling back to 1.2 but all i got was an internal error from MMS/SMS
> > service failing to read its internal database. I suppose that was to be
> > expected, but worth a try.
> 
> Just a quick response to this. In our current DB updating schema, we only
> allow upgrade but no downgrade.

I wasnt expecting downgrades to be supported, don't worry :)

I'll try att #8356009 and report back if i've been able to fetch the MMS.

Comment 16

[Julien Wajsberg [:julienw]]

(In reply to Gene Lian [:gene] (PTO Dec. 25 ~ Jan. 5) from comment #12)
> (In reply to Julien Wajsberg [:julienw] (PTO until January 6th) from comment
> #2)
> > Hey Gene,
> > 
> > In the following line :
> > 
> > 2275         serviceId = gRil.getClientIdByIccId(aMessageRecord.iccId);
> > 
> > I think the "iccId" property was not set because the message was received
> > before the property existed. Therefore I think you need a fallback code here
> > in case iccId is not set.
> 
> Nice catch! Julien. That's exactly the root cause. We intentionally did this
> just because we don't have knowledge about which SIM is supposed to use to
> download the MMS after the DSDS scenario is considered, so we just stop
> downloading the MMS which has been saved during the previous version. Maybe
> this check is too strict, though.

I think this is too strict because we have basically 2 cases:

* either the user uses a DSDS device: in that case, he will always have a software that sets "aMessageRecord.iccId"
* either the user uses a single sim device: in that case, he will always have only one SIM, even after an upgrade


That said, I tried the upgrade scenario 1.2 -> 1.3 using geeksphone's builds, and it's worse: I can't get the db to load :( Maybe this is due to a wrong flashing process, I'll try it again.

Comment 17

[Julien Wajsberg [:julienw]]

Gene, Vicamo, Landry also spotted line 2413 where there is a similar issue.

Comment 18

[Gene Lian (I already quit Mozilla)]

(In reply to Julien Wajsberg [:julienw] (PTO until January 6th) from comment #17)
> Gene, Vicamo, Landry also spotted line 2413 where there is a similar issue.

Wow! Thanks Julien!

Comment 19

[Gene Lian (I already quit Mozilla)]

Attachment: Patch, V2

Fixed another getClientIdByIccId(iccId) when sending read report.

Comment 20

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8356102 [details] [diff] [review]
Patch, V2

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

::: dom/mobilemessage/src/gonk/MmsService.js
@@ +2425,5 @@
> +        // during the previous version that didn't take the DSDS scenario
> +        // into consideration. Tentatively setting the service ID to be 0 by
> +        // default is better than nothing. Although it might use the wrong
> +        // SIM to download the desired MMS, eventually it would still fail to
> +        // download it due to the wrong MMSC and proxy settings, though.

Self-review. Should correct the comment as below:

// If the ICC ID isn't available, it means the MMS has been received
// during the previous version that didn't take the DSDS scenario
// into consideration. Tentatively setting the service ID to be 0 by
// default is better than nothing. Although it might use the wrong
// SIM to send the read report for the desired MMS, eventually it
// would still fail to send due to the wrong MMSC and proxy settings.

The rest is still the same.

Comment 21

[Gene Lian (I already quit Mozilla)]

(In reply to Julien Wajsberg [:julienw] (PTO until January 6th) from comment #16)
> I think this is too strict because we have basically 2 cases:
> 
> * either the user uses a DSDS device: in that case, he will always have a
> software that sets "aMessageRecord.iccId"

This is a bit wrong. If the user is using the DSDS device with the older DB version, then we still cannot set the ICC ID for it. Whether or not the ICC is set depends on the DB (Gecko) version, instead of the type of device.

> * either the user uses a single sim device: in that case, he will always
> have only one SIM, even after an upgrade

This case is correct.

Comment 22

[Landry Breuil (:gaston)]

Comment on attachment 8356102 [details] [diff] [review]
Patch, V2

Using this patch on my device (pulled omni.ja & unzip it, patched MmsService.js, stop b2g, rezip omni.ja & pushed it back, start b2g), i don't get the 'Missing SIM Card' error message anymore and it tries to fetch the MMS. In the meantime i received another MMS notification (this one received with gaia 1.4 - so should have the iccId in the database?) and the app also tries to fetch this MMS, so both cases look ok to me. (i don't have coverage right now so cant fully check)

Comment 23

[Julien Wajsberg [:julienw]]

(In reply to Gene Lian [:gene] (PTO Dec. 25 ~ Jan. 5) from comment #21)
> (In reply to Julien Wajsberg [:julienw] (PTO until January 6th) from comment
> #16)
> > I think this is too strict because we have basically 2 cases:
> > 
> > * either the user uses a DSDS device: in that case, he will always have a
> > software that sets "aMessageRecord.iccId"
> 
> This is a bit wrong. If the user is using the DSDS device with the older DB
> version, then we still cannot set the ICC ID for it. Whether or not the ICC
> is set depends on the DB (Gecko) version, instead of the type of device.

Yep, but I was meaning that we'll have no user ever using a DSDS device with a FxOS version that does not support DSDS, and thus with an older DB version :)

But maybe I'm wrong here?

Anyway, thanks for fixing the issue :)

Comment 24

[Julien Wajsberg [:julienw]]

Comment on attachment 8356102 [details] [diff] [review]
Patch, V2

(asking approval for a small patch because blockers is too painful to get set)

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): DSDS work
User impact if declined: migration issue between 1.2 and 1.3
Testing completed: yes, by the user that found the issue
Risk to taking this patch (and alternatives if risky): low, the patch consists of adding guards around null values
String or UUID changes made by this patch: none

Comment 25

[Landry Breuil (:gaston)]

Fwiw, i've been able to download my two pending MMS this morning, the one with the notification received on 1.1 (so without iccId) and the one with the notification received on master (with an iccId).

Comment 26

[Gene Lian (I already quit Mozilla)]

(In reply to Landry Breuil (:gaston) from comment #22)
> Comment on attachment 8356102 [details] [diff] [review]
> Patch, V2
> 
> Using this patch on my device (pulled omni.ja & unzip it, patched
> MmsService.js, stop b2g, rezip omni.ja & pushed it back, start b2g), i don't
> get the 'Missing SIM Card' error message anymore and it tries to fetch the
> MMS. In the meantime i received another MMS notification (this one received
> with gaia 1.4 - so should have the iccId in the database?) and the app also
> tries to fetch this MMS, so both cases look ok to me. (i don't have coverage
> right now so cant fully check)

Thank you for the detailed testing!

Comment 27

[Gene Lian (I already quit Mozilla)]

Attachment: Patch to land (r=vicamo f=gaston a=v1.3+,approval-mozilla-b2g28+)

Comment 28

[Gene Lian (I already quit Mozilla)]

https://hg.mozilla.org/integration/b2g-inbound/rev/53f566c166bf

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53f566c166bf

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6154327ecec0