The operator variant logic in charge of customizing APNs, voicemail, cellbroadcast, and some messaging settings must be adapted to the DSDS feature.

IMHO this bug must be part of v1.3. This bug is going to be a blocker for bug 928297 and bug 926372.

Sandip, what do you think?

Jose, can you please take this bug?

(In reply to Kevin Hu [:khu] from comment #2) > Sandip, what do you think? I do not want to add risk for v1.3 scope defined in user story Google doc. I am okay for this to move to 1.4. Are there any downsides of doing that?

(In reply to Sandip Kamat from comment #4) > (In reply to Kevin Hu [:khu] from comment #2) > > Sandip, what do you think? > > I do not want to add risk for v1.3 scope defined in user story Google doc. I > am okay for this to move to 1.4. Are there any downsides of doing that? Right now the APNs, voicemail, cellbroadcast, and some messaging settings are configured relying on the ICC card. If in a DSDS scenario the device has two different ICC card so how is it supposed to work in this new scenario? This bug is a must for v1.3. Data calls, voicemail service, both SMS and MMS services, cell broadcast services get affected since their settings won't be adapted if you guys decide to not take of care of this bug for v1.3. Actually the risk for v1.3 scope is to move it to 1.4.

See coment #1, this is a blocker for bug 928297.

Hsin-Yi, Arthur, Ken, I need you guys to confirm that this bug must be part of v1.3. There is a discussion about moving this bug to v1.4 and I would rise one more time this must to be done for v1.3. As you already know the `ril.data.apnSettings` setting is built by the system app on boot. Under this setting we store the array of APN settings for the ICC card on the device. Right now the operator variant logic builds an array of only one element as we only have one ICC card. This logic must change to build one more element with the APNs for this second ICC card (the APNs rely on the MCC and MNC codes in the ICC card). This one task that the operator variant logic perform, there are other ones related to voicemail, cellbroadcast and messaging settings. Could you guys share your thoughts please? Thanks. I hope this comment could help to understand how important this bug is and to set this bug as 1.3+.

This bugs is a customization. I agree that this bug is a critical bug for manufacturer and carrier. IMHO, this bug is a 1.3+ bug if it's also happened in single SIM project, Even this bug is a 1.3+ bug. But this bug isn't a blocker for any user story, I wonder if we have to fix this before FC date. Eventually, we still have to fix this bug . If you want to fix it early, it is fine for me to set this bug as 1.3+. However, the problem is when you expect to fix it. Do we have to fix this bug in current stage? Do we have resource for this bug now?

I agree with Ken on this. Thanks.

I agree with Ken. Jose, thank you for taking the effort. Let me know if you need help.

Triaged today. We(Francis,Ken,Enpei,Wesley,me) think that it's reasonable to put this in 1.3. We agreed that this one won't block the 1.3 feature implementation. Joe, this looks like a bug in Comms team. Is it in your radar? Thanks.

Blocked by Bug 814637, clearing target milestone till WebIccManager api lands

WIP branch at https://github.com/jaoo/gaia/tree/933203

This bug is supposed to block the user story of data call.

I put two sim cards (chunghwa & twm) into fugu. chunghwa is in the 1st slot and twm in the 2nd. From lockscreen and 'settings-->cellular & data-->carrier' I saw 'chunghwa' as expected. However, I saw 'twm' in 'settings-->cellular & data-->data settings' so that I always got the apn settings for 'twm' sim cards. Per discussion with Arthur, we thought this weird behaviour seems caused by followings: In icc_helper.js we don't check if iccIds[0] is coming from the 1st slot. iccIds[0] could be the card inserted into slot_2 but just getting ready first. Seems this issue is within the scope of this bug, so I commented here. :) Just let me know if I should file a new bug for this.

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #15) > I put two sim cards (chunghwa & twm) into fugu. chunghwa is in the 1st slot > and twm in the 2nd. From lockscreen and 'settings-->cellular & > data-->carrier' I saw 'chunghwa' as expected. However, I saw 'twm' in > 'settings-->cellular & data-->data settings' so that I always got the apn > settings for 'twm' sim cards. > > Per discussion with Arthur, we thought this weird behaviour seems caused by > followings: > In icc_helper.js we don't check if iccIds[0] is coming from the 1st slot. > iccIds[0] could be the card inserted into slot_2 but just getting ready > first. > > Seems this issue is within the scope of this bug, so I commented here. :) > Just let me know if I should file a new bug for this. This bug (and bug 928297) will take of it. The IccHelper object was useful for a single ICC card scenario but not for DSDS scenario. I am stop using it in all this new DSDS bugs.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #16) > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #15) > > I put two sim cards (chunghwa & twm) into fugu. chunghwa is in the 1st slot > > and twm in the 2nd. From lockscreen and 'settings-->cellular & > > data-->carrier' I saw 'chunghwa' as expected. However, I saw 'twm' in > > 'settings-->cellular & data-->data settings' so that I always got the apn > > settings for 'twm' sim cards. > > > > Per discussion with Arthur, we thought this weird behaviour seems caused by > > followings: > > In icc_helper.js we don't check if iccIds[0] is coming from the 1st slot. > > iccIds[0] could be the card inserted into slot_2 but just getting ready > > first. > > > > Seems this issue is within the scope of this bug, so I commented here. :) > > Just let me know if I should file a new bug for this. > > This bug (and bug 928297) will take of it. The IccHelper object was useful > for a single ICC card scenario but not for DSDS scenario. I am stop using it > in all this new DSDS bugs. Got it. Thank you very much for handling this. :)

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #13) > WIP branch at https://github.com/jaoo/gaia/tree/933203 Is it possible to have this before 12/6?

(In reply to Ken Chang from comment #18) > (In reply to José Antonio Olivera Ortega [:jaoo] from comment #13) > > WIP branch at https://github.com/jaoo/gaia/tree/933203 > Is it possible to have this before 12/6? Sure, I'm doing my best to request review for both bugs 928297 and this one today so the user could set the APNs for any of the ICC cards inserted in the multi ICC card device.

I'm having good progress with the WIP patch while testing on the fugu device. I'll request review ASAP. Data calls for multi ICC card devices could be already tested by applying the patch from bug 928297 (see https://bugzilla.mozilla.org/show_bug.cgi?id=928297#c17).

With this patch the device sets up on boot the APN settings for the ICC cards in the devices, for all of them. The `ril.data.apnSettings` settings is now an array containing the APNs for the ICC cards. We will handle only APNs settings in this bug so I'll change the title of the bug to make this clear. I'd like to remark that I've been able to set up the APN settings for the two ICC cards only a few times. The fugu device is pretty unstable and lot of times the second ICC card never gets ready so we are not able to set up the APN settings relying on the MCC and MNC in the ICC card. The most important thing is that the changes introduced in this bug don't broke the functionality on sindle ICC devices. Kaze, would you take a look at this patch please? Ping me on IRC if you have any doubt please.

I'd like to make clear that we will take care of APN settings here. The other settings e.g. cellbroadcast, messaging settings and voicemail settings are not critical right now. I'll file bugs for taking care of them in a multi ICC card environment. For example, DSDS support has not been added to the mozCellbroadcast API so it makes no sense to work on cellbroadcast settings for a multi ICC card scenario.

Comment on attachment 8340139 [details] [diff] [review] v2 Review of attachment 8340139 [details] [diff] [review]: ----------------------------------------------------------------- LGTM ::: apps/system/js/operator_variant/operator_variant.js @@ +101,5 @@ > + var self = this; > + var xhr = new XMLHttpRequest(); > + xhr.open('GET', OPERATOR_VARIANT_FILE, true); > + xhr.responseType = 'json'; > + xhr.onreadystatechange = function() { Nit: feel free to use xhr.onload instead. @@ +259,3 @@ > } > + > + // Change property mane 'type' by 'types'. Nit, typo: s/mane/name/ @@ +300,5 @@ > + > + var xhr = new XMLHttpRequest(); > + xhr.open('GET', WAP_UA_PROFILE_FILE, true); > + xhr.responseType = 'json'; > + xhr.onreadystatechange = function() { Nit: feel free to use xhr.onload = function() instead.

(In reply to Fabien Cazenave [:kaze] from comment #24) > Comment on attachment 8340139 [details] [diff] [review] > v2 > > Review of attachment 8340139 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM Thanks for the review Fabien! HisinYi, the patch from this bug got r+ but I would like to raise some concerns. The fugu device is pretty unstable at ICC card detection on boot. Sometimes the ICC card 1 gets detected, sometimes the ICC card 2 gets detected, sometimes both. This is not because of the patch as I've tested. Having said that I'd like you to help me to take a decision on what to do (I mean land it). The good thing is that this patch doesn't brake the functionality for single ICC card devices.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #25) > (In reply to Fabien Cazenave [:kaze] from comment #24) > > Comment on attachment 8340139 [details] [diff] [review] > > v2 > > > > Review of attachment 8340139 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > LGTM > > Thanks for the review Fabien! > > HisinYi, the patch from this bug got r+ but I would like to raise some > concerns. The fugu device is pretty unstable at ICC card detection on boot. > Sometimes the ICC card 1 gets detected, sometimes the ICC card 2 gets > detected, sometimes both. This is not because of the patch as I've tested. > Having said that I'd like you to help me to take a decision on what to do (I > mean land it). Hi José, Thanks for raising the concern. I know you understand that the system team is working hard to stabilize fugu hardware so I don't share more progress here. :) However, regarding the ICC detection problem, Edgar provided a patch on bug 944234 that should help. Maybe you'd like to apply to see if that helps you. Back to your question, I think this bug covers the situation of only one card, two cards and no sim card. So if the patch reflects the situation that the system is detecting (rather than the exact number of cards inserted physically), then I think the patch is ready to go. For example, when the system tells gaia there's only one even there are two inserted physically, what we are expecting is the patch could react for the case of only one card inserted and one slot keeping empty. Does that make sense to you? > The good thing is that this patch doesn't brake the functionality for single > ICC card devices. This is really good. :)

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #26) > However, regarding the ICC detection problem, Edgar provided a patch on bug > 944234 that should help. Maybe you'd like to apply to see if that helps you. Oh, great. That is the issue I see. I'll give a try. > Back to your question, I think this bug covers the situation of only one > card, two cards and no sim card. So if the patch reflects the situation that > the system is detecting (rather than the exact number of cards inserted > physically), then I think the patch is ready to go. For example, when the > system tells gaia there's only one even there are two inserted physically, > what we are expecting is the patch could react for the case of only one card > inserted and one slot keeping empty. > > Does that make sense to you? The code is able to react to whatever the RIL plumbing is reporting and doesn't broke the functionality for single ICC card devices so once the test with Edgar's patch is done I'll land this patch. If we found any issue once the fugu device gets more stable we will file follow-up bugs to fix them. Thanks for your feedback.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #27) > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #26) > > However, regarding the ICC detection problem, Edgar provided a patch on bug > > 944234 that should help. Maybe you'd like to apply to see if that helps you. The patch from bug 944234 saved my day. Now, if I apply that patch everything works as expected. On the down side I've noticed that unit test are failing and I need to fix them before landing this bug. The bad news is there are no mocks objects for the multi ICC card support for the mozIccManager/mozIcc objects and I need to create them :(. It will take more than expected.

Fabien, I've added the test changes. Now they pass so this will be ready to land. You already reviewed this bug but I feel more comfortable if you take a look at the tests part. Sorry for bothering you. Thanks! PS. I'll do the PR in parallel.

I see errors in Travis concerning mozSettings but they don’t seem to be related to this patch. I’d suggest to land this patch now if it helps working on bug 928297.

gaia/master: https://github.com/mozilla-b2g/gaia/commit/71e2fcdaf3a6b86d11645ab1163e037179f591a3

This commit caused Marionette webapi tests to crash, so has been backed out with https://github.com/mozilla-b2g/gaia/commit/f6a5c9991765bc91511e552e175cb91e10efae89. See bug 946178.

(In reply to Jonathan Griffin (:jgriffin) from comment #33) > This commit caused Marionette webapi tests to crash, so has been backed out > with > https://github.com/mozilla-b2g/gaia/commit/ > f6a5c9991765bc91511e552e175cb91e10efae89. See bug 946178. Sorry for the inconveniences, my apologies. How could I to shed some light here? I mean, in comment https://bugzilla.mozilla.org/show_bug.cgi?id=946178#c27 you mentioned you did local testing to figure out what the issue/the change causing the error is. How do you test this Marionette webapi tests to crash? Thanks!

If you have a local emulator build, you can run the webapi tests easily using ./mach marionette-webapi from the B2G directory. I verified that the tests crash when the emulator is built with 71e2fcdaf3a6b86d11645ab1163e037179f591a3, but they don't crash when the emulator is built with the previous gaia commit, f9901db71b8e5a70da1700ae82b66231bc000660.

Thanks Jonathan! Hsin-Yi, It seems this patch causes marionette webapi test to crash and It was backed out (see bug 946178 please). I'm trying to figure out what is going on but at a first glance I don't see how this patch is causing the failures, could you shed some light here please? Thanks.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #36) > Thanks Jonathan! > > Hsin-Yi, It seems this patch causes marionette webapi test to crash and It > was backed out (see bug 946178 please). I'm trying to figure out what is > going on but at a first glance I don't see how this patch is causing the > failures, could you shed some light here please? Thanks. José, I looked at the log but didn't see something obvious. :( Much more investigation is required on my side as well ... CC more people. Hopefully we get more clues.

I can reproduce this fail with the patch, below is the backtrace from gdb, Program received signal SIGSEGV, Segmentation fault. 0x4126cc34 in mozilla::dom::network::MobileConnection::Listener::NotifyRadioStateChanged (this=<value optimized out>) at /data/mercurial/mozilla-central/dom/network/src/MobileConnection.cpp:33 33 NS_FORWARD_SAFE_NSIMOBILECONNECTIONLISTENER(mMobileConnection) (gdb) bt #0 0x4126cc34 in mozilla::dom::network::MobileConnection::Listener::NotifyRadioStateChanged (this=<value optimized out>) at /data/mercurial/mozilla-central/dom/network/src/MobileConnection.cpp:33 #1 0x40c9c6d2 in NS_InvokeByIndex (that=<value optimized out>, methodIndex=<value optimized out>, paramCount=<value optimized out>, params=<value optimized out>) at /data/mercurial/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:164 #2 0x411ab690 in CallMethodHelper::Invoke (ccx=<value optimized out>, mode=<value optimized out>) at /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2567 #3 CallMethodHelper::Call (ccx=<value optimized out>, mode=<value optimized out>) at /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:1907 #4 XPCWrappedNative::CallMethod (ccx=<value optimized out>, mode=<value optimized out>) at /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:1873 #5 0x411aba2e in XPC_WN_CallMethod (cx=0x40220270, argc=0, vp=<value optimized out>) at /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNativeJSOps.cp---Type <return> to continue, or q <return> ---Type <return> to continue, or q <return> to quit---

(In reply to Edgar Chen [:edgar][:echen] from comment #38) > I can reproduce this fail with the patch, below is the backtrace from gdb, > > Program received signal SIGSEGV, Segmentation fault. > 0x4126cc34 in > mozilla::dom::network::MobileConnection::Listener::NotifyRadioStateChanged > (this=<value optimized out>) > at > /data/mercurial/mozilla-central/dom/network/src/MobileConnection.cpp:33 > 33 NS_FORWARD_SAFE_NSIMOBILECONNECTIONLISTENER(mMobileConnection) > (gdb) bt > #0 0x4126cc34 in > mozilla::dom::network::MobileConnection::Listener::NotifyRadioStateChanged > (this=<value optimized out>) > at > /data/mercurial/mozilla-central/dom/network/src/MobileConnection.cpp:33 > #1 0x40c9c6d2 in NS_InvokeByIndex (that=<value optimized out>, > methodIndex=<value optimized out>, paramCount=<value optimized out>, > params=<value optimized out>) > at > /data/mercurial/mozilla-central/xpcom/reflect/xptcall/src/md/unix/ > xptcinvoke_arm.cpp:164 > #2 0x411ab690 in CallMethodHelper::Invoke (ccx=<value optimized out>, > mode=<value optimized out>) > at > /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2567 > #3 CallMethodHelper::Call (ccx=<value optimized out>, > mode=<value optimized out>) > at > /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:1907 > #4 XPCWrappedNative::CallMethod (ccx=<value optimized out>, > mode=<value optimized out>) > at > /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:1873 > #5 0x411aba2e in XPC_WN_CallMethod (cx=0x40220270, argc=0, > vp=<value optimized out>) > at > /data/mercurial/mozilla-central/js/xpconnect/src/XPCWrappedNativeJSOps.cp--- > Type <return> to continue, or q <return> ---Type <return> to continue, or q > <return> to quit--- A guess: [1] it's likely to have null mProvider (see constructor code). If that's the case, mListener->Disconnect() is never called that might cause some problems in the next run. I am going to rewrite the code as follows and give it a try. if (mListener) { mListener->Disconnect(); if (mProvider) { mProvider->UnregisterMobileConnectionMsg(mClientId, mListener); mProvider = nullptr; } mListener = nullptr; } [1] http://dxr.mozilla.org/mozilla-central/source/dom/network/src/MobileConnection.cpp?from=MobileConnection.cpp#118

another gdb bt: Program received signal SIGSEGV, Segmentation fault. XPCWrappedNativeProto::JSProtoObjectFinalized (this=0x1, fop=0x442e4540, obj=0xbe86a7e0) at ../../../../gecko/js/xpconnect/src/XPCWrappedNativeProto.cpp:137 137 ClassInfo2WrappedNativeProtoMap* map = GetScope()->GetWrappedNativeProtoMap(); (gdb) bt #0 XPCWrappedNativeProto::JSProtoObjectFinalized (this=0x1, fop=0x442e4540, obj=0xbe86a7e0) at ../../../../gecko/js/xpconnect/src/XPCWrappedNativeProto.cpp:137 #1 0x4128493a in mozilla::dom::network::MobileConnection::Listener::NotifyRadioStateChanged (this=<optimized out>) at ../../../../gecko/dom/network/src/MobileConnection.cpp:33 #2 0x40cf5c86 in NS_InvokeByIndex (that=<optimized out>, methodIndex=11, paramCount=<optimized out>, params=0xbe86a6f8) at ../../../../../../../gecko/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:164 #3 0x411ceac2 in Invoke (this=0xbe86a6d0) at ../../../../gecko/js/xpconnect/src/XPCWrappedNative.cpp:2567 #4 Call (this=0xbe86a6d0) at ../../../../gecko/js/xpconnect/src/XPCWrappedNative.cpp:1907 #5 XPCWrappedNative::CallMethod (ccx=<optimized out>, mode=<optimized out>) at ../../../../gecko/js/xpconnect/src/XPCWrappedNative.cpp:1873 #6 0x411cef1e in XPC_WN_CallMethod (cx=0x40423270, argc=0, vp=0xbe86ab48) at ../../../../gecko/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1301 #7 0x41afaa50 in CallJSNative (args=..., native=0x411cee29 <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, cx=0x40423270) at ../../../gecko/js/src/jscntxtinlines.h:220 #8 js::Invoke (cx=0x40423270, args=..., construct=js::NO_CONSTRUCT) at ../../../gecko/js/src/vm/Interpreter.cpp:463 #9 0x41a951d6 in js_fun_call (cx=0x40423270, argc=<optimized out>, vp=0x43b18130) at ../../../gecko/js/src/jsfun.cpp:892 #10 0x41a95b5e in js_fun_apply (cx=0x40423270, argc=<optimized out>, vp=0x43b18130) (gdb) p map $1 = (ClassInfo2WrappedNativeProtoMap *) 0xb (gdb)

My try on comment 39 fails. :(

I think it is related to cycle collector setup of MobileConnectionArray [1]. The UNLINK process may trigger releasing MobileConnection without calling Shutdown(). There are two possible solution: 1). Calling Shutdown() in MobileConnectionArray UNLINK process. 2). Calling Shutdown() in MobileConnection destructor. [1] http://dxr.mozilla.org/mozilla-central/source/dom/network/src/MobileConnectionArray.cpp#13

Hsin-Yi, Edgar, thank you guys for being investigating this issue. I'm wondering if it would be better to file a new bug for the underlying issue and make this bug to depend on the new one. Thoughts?

Yup, agree with Edgar, seems problems with cycle collector. I set break point in [1] and MobileConnection::Shutdown [2]. [1] is reached right before crash without [2]. With the WIP patch, I don't see crash anymore. [1] http://dxr.mozilla.org/mozilla-central/source/dom/network/src/MobileConnection.cpp#54

Comment on attachment 8343079 [details] [diff] [review] 0001-Bug-933203.patch - WIP - fix crash Oops, wrong file.

Shutdown() in 1) MobileConnection unlink 2) MobileConnection destructor

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #43) > Hsin-Yi, Edgar, thank you guys for being investigating this issue. I'm > wondering if it would be better to file a new bug for the underlying issue > and make this bug to depend on the new one. Thoughts? Hi José, Having another bug makes the bug context clear. I'll do that! The WIP was attached here to help you verify the crash is done. :)

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #49) > (In reply to José Antonio Olivera Ortega [:jaoo] from comment #43) > > Hsin-Yi, Edgar, thank you guys for being investigating this issue. I'm > > wondering if it would be better to file a new bug for the underlying issue > > and make this bug to depend on the new one. Thoughts? > > Hi José, > > Having another bug makes the bug context clear. I'll do that! The WIP was > attached here to help you verify the crash is done. :) This crash issue has been addressed in bug 946437.

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #50) > > Hi José, > > > > Having another bug makes the bug context clear. I'll do that! The WIP was > > attached here to help you verify the crash is done. :) > > This crash issue has been addressed in bug 946437. Hi José, could you please check-in your patch?

(In reply to Ken Chang[:ken] from comment #51) > Hi José, could you please check-in your patch? We cannot land this patch again until bug 946437 lands first.

Fabrice, we are waiting bug 946437 to land before re-landing this bug again. Would you mind to take a look at the patch attached here please?. If you are OK with it, I'll re-land this path as soon as bug 946437 gets landed and Travis goes green and tree is open. Thanks.

I can't take a decision base on the PR only, especially with so many changes. Please help me with the following information: - is the code fairly isolated? ("I want to refactor the RIL worker" would not pass). - has this code caused churn or pain before? - is it easy to turn the feature off? - do we have good test coverage? - do we have good QA coverage, ie in smoketests? - do we have strong partner expectations for the feature?

We really need to know when the bug can be landed, because DSDS is a committed feature. Can you help to provide the ETA? Thank you.

(In reply to Fabrice Desré [:fabrice] from comment #54) > - is the code fairly isolated? ("I want to refactor the RIL worker" > would not pass). More or less, It touches both system and setting apps. The code is in charge of auto selecting the APNs on boot and allowing to modify them later by the user. > - has this code caused churn or pain before? Not specially, just a few changes motivated by the new use cases. > - is it easy to turn the feature off? We can just back it out to turn it off. > - do we have good test coverage? Yes, living them in the system app. > - do we have good QA coverage, ie in smoketests? No idea, ni? Enpei. > - do we have strong partner expectations for the feature? Kevin to provide an answer here. Requesting ni? then.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #56) > (In reply to Fabrice Desré [:fabrice] from comment #54) > > > - do we have strong partner expectations for the feature? > > Kevin to provide an answer here. Requesting ni? then. Not from my side. I did not hear anything about this from partners. Maybe Sandip has some comments on this. I would suggest this to 1.4, if we don't have strongly desired voices from partners.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #56) > > - do we have good QA coverage, ie in smoketests? > > No idea, ni? Enpei. DSDS APN test alreay has test cases in moztrap.

(In reply to Kevin Hu [:khu] from comment #57) > (In reply to José Antonio Olivera Ortega [:jaoo] from comment #56) > > (In reply to Fabrice Desré [:fabrice] from comment #54) > > > > > - do we have strong partner expectations for the feature? > > > > Kevin to provide an answer here. Requesting ni? then. > Not from my side. I did not hear anything about this from partners. Maybe > Sandip has some comments on this. I would suggest this to 1.4, if we don't > have strongly desired voices from partners. This blocks several committed user stories. Without this, we are unable to setup a data call or send MMS with the second SIM.

I thought that this is used to adjust the APN setting automatically. Without this, we should be able to do it manually. But, after confirming with Hsinyi and realized that, even via Gaia, we can't configure the APN setting for 2nd SIM manually. So, this is a blocker for sure(for 2nd SIM).

Adding link for the new PR since the previous one is closed after being the patch backed out.

Fabrice, I replied to your request at comment #56. Could you take a look at it please? Thanks!

Approval from me to land on 1.3, thanks.

Landed on Gaia master branch. Last time it was backed out this time everything is fine, apparently. Finger crossed. https://github.com/mozilla-b2g/gaia/commit/4dd40d64f137c48104d5f39e48c2ccb0ce9ab78c

I had to back this out because of a unit test failure. https://github.com/mozilla-b2g/gaia/commit/6415b8b44068596404c10365394544e94edd5ce5 [system] system/LockScreenConnInfoManager > Single sim devices "before each" hook: TypeError: MockNavigatorMozIccManager.addIcc is not a function

Rik thanks, this was causing UI test failures too but we hadn't quite got to the source!

Zac: Do you have some logs for those failures so that José can look at them?

Rik, yes Askeing filed: https://bugzilla.mozilla.org/show_bug.cgi?id=948852

Fabien, the test in the last patch caused unit test failures in other tests that were using a mock object for the mozIccManager API. This was because the one I added was causing conflicts with the one already landed and it results in a mess. Now the tests in this patch are using the existing mock object for the mozIccManager API already landed in master. Everything seems to be right and I don't see any unit test failures. I would need a quick review of the patch since we need to land this ASAP. Thanks!

I’m afraid the tree is closed for now (NPM issues). I’m reviewing your patch, but we’ll have to wait for Travis results before giving an r+.

All green now.

Comment on attachment 8346004 [details] [diff] [review] v4 LGTM. Crossing my fingers… As discussed on IRC, and for the record: this is a good example why I prefer to deal with multiple commits in a pull request. It makes it much easier to review successive versions of a patch.

Travis goes green with the PR and everything seems to work fine for both peak and fugu devices. I hope the patch doesn't broke anything. https://github.com/mozilla-b2g/gaia/commit/53b010647062b4e2637f98ac3538af1c7bfe723a

Travis went green also after landing the patch. See https://travis-ci.org/mozilla-b2g/gaia/builds/15387670.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #76) > Travis went green also after landing the patch. See > https://travis-ci.org/mozilla-b2g/gaia/builds/15387670. \o/

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #75) > https://github.com/mozilla-b2g/gaia/commit/ > 53b010647062b4e2637f98ac3538af1c7bfe723a John, can you please assist with the uplift to v1.3? :)

[v1.3 a99ca07] Merge pull request #14579 from jaoo/933203

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #79) > [v1.3 a99ca07] Merge pull request #14579 from jaoo/933203 Hey John, you didn't uplift this patch correctly. You had to uplift https://github.com/mozilla-b2g/gaia/commit/53b010647062b4e2637f98ac3538af1c7bfe723a, see https://github.com/mozilla-b2g/gaia/pull/14579 please.

I don't see commit a99ca07 anywhere.

weird. I've redone the cherry-pick. [v1.3 1b432b0] Merge pull request #14579 from jaoo/933203 $ git push origin v1.3 Counting objects: 161, done. Delta compression using up to 8 threads. Compressing objects: 100% (20/20), done. Writing objects: 100% (20/20), 2.12 KiB | 0 bytes/s, done. Total 20 (delta 15), reused 0 (delta 0) To git@github.com:mozilla-b2g/gaia.git 1594923..1b432b0 v1.3 -> v1.3