869778 - [B2G][CDMA]Get the CDMA subscription information.

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Ken Chang[:kenkai]]

We may need the interface, REQUEST_CDMA_SUBSCRIPTION, to know the CDMA subscription information.

Comment 1

[Edgar Chen [:edgar]]

REQUEST_CDMA_SUBSCRIPTION contains the information of PRL version [1]. Maybe we should also handle RIL_UNSOL_CDMA_PRL_CHANGED in this bug [2].

[1] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L3034
[2] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L3750

Thanks

Comment 2

[Kai-Chih Hu [:khu]]

Edgar is studying this part. Put notes here.

Comment 3

[Kai-Chih Hu [:khu]]

Ken, is this one related to bug 890826? Thanks.

Comment 4

[Edgar Chen [:edgar]]

We could use REQUEST_CDMA_SUBSCRIPTION to get below information.

* Mobile Directory Number (MDN)
* Mobile identification number (MIN)
* Home Network Id (H_NID)
* Home System Id (H_SID)
* Preferred Roaming List (PRL) version

Comment 5

[Edgar Chen [:edgar]]

(In reply to khu from comment #3)
> Ken, is this one related to bug 890826? Thanks.

This one is not related to bug 890826 (The information I mentioned in comment #4 does not show in C.S0005 section 3.7.5.).

So, do we have any user story for these informations?

Thanks

Comment 6

[Szu-Yu Chen [:aknow] (inactive)]

MIN in subscription is needed for OTASP.

In Bug 882983 (OTASP), we have to check whether the OTASP is needed. One of the solution is to check whether we have already got a valid mobile identification number (MIN). If it is invalid (length < 6) or equal to some default strings for unactivated, the phone will inform the user that OTASP is needed. After OTASP, MIN could be proper programmed. (Ref: http://en.wikipedia.org/wiki/Over-the-air_programming)

Comment 7

[Edgar Chen [:edgar]]

This bugs is not related to bug 890826, but OTASP.

We need to expose MIN in subscription to Gaia. Gaia needs this information to check the OPASP is needed or not (Please see comment #6).

Thanks

Comment 8

[Edgar Chen [:edgar]]

Mobile identification number (MIN) is stored in EF_IMSI_M in RUIM [1]. Because this information comes from RUIM, I suggest to expose this in IccManager, maybe in iccInfo. Any other suggestion? Thanks. :)

[1] Please see C.S0023-D session 3.4.2

Comment 9

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA MIN to MozIccInfo, v1

Comment 10

[Edgar Chen [:edgar]]

Attachment: Part 2: RIL implementation for CDMA MIN, v1

Comment 11

[Edgar Chen [:edgar]]

Comment on attachment 789457 [details] [diff] [review]
Part 1: Expose CDMA MIN to MozIccInfo, v1

We need to WebIDL-ize MozIccManager, I will file a new bug for this. Thanks

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 789457 [details] [diff] [review]
Part 1: Expose CDMA MIN to MozIccInfo, v1

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

Hi, Mounir
I saw you already gave sr+ on this, 
but I'd still like to get your feedback here.

What Edgar is trying to do here is to add a data field, which is only available on CDMA SIM card.
Should we seperate those data between CDMA and GSM?
Otherwise I'd imagine that later the object will full of CDMA and GSM data.

Should we add more hierarchy on this?

Like 

interface nsIDOMIccInfo {
 // mandatory data in all UICC(SIM)s.
 readonly attribute DOMString iccType;  // 'usim' for GSM/WCDMA, 'ruim' for CDMA
}

interface nsIDOMGsmIccInfo : nsIDOMIccInfo {
  // data only available for GSM
}

interface nsIDOMCdmaIccInfo : nsIDOMIccInfo {
  // data only available for CDMA
}

Or should we discuss on dev-webapi?

Thanks

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 789458 [details] [diff] [review]
Part 2: RIL implementation for CDMA MIN, v1

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

::: dom/system/gonk/ril_worker.js
@@ +12891,5 @@
>      ICCRecordHelper.readICCID();
>      RIL.getIMSI();
>      this.readCST();
>      this.readCDMAHome();
> +    RIL.getCdmaSubscription();

I met a problem when I tested this patch on n970.

The cdma subscription is not available until app state becomes to ready. But currently we changes sim card status to "ready" when perso substate becomes to CARD_PERSOSUBSTATE_READY [1] (Actually the app state is not CARD_APPSTATE_READY yet, but CARD_APPSTATE_SUBSCRIPTION_PERSO). We need to correct this behavior first.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#3310

Comment 14

[Mounir Lamouri (:mounir)]

Comment on attachment 789457 [details] [diff] [review]
Part 1: Expose CDMA MIN to MozIccInfo, v1

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

I'm afraid I can't really speak about CDMA, GSM and stuff like that. Maybe Gregor can help you here?

Comment 15

[Gregor Wagner [:gwagner]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #12)
> Comment on attachment 789457 [details] [diff] [review]
> Part 1: Expose CDMA MIN to MozIccInfo, v1
> 
> Review of attachment 789457 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi, Mounir
> I saw you already gave sr+ on this, 
> but I'd still like to get your feedback here.
> 
> What Edgar is trying to do here is to add a data field, which is only
> available on CDMA SIM card.
> Should we seperate those data between CDMA and GSM?
> Otherwise I'd imagine that later the object will full of CDMA and GSM data.
> 
> Should we add more hierarchy on this?
> 
> Like 
> 
> interface nsIDOMIccInfo {
>  // mandatory data in all UICC(SIM)s.
>  readonly attribute DOMString iccType;  // 'usim' for GSM/WCDMA, 'ruim' for
> CDMA
> }
> 
> interface nsIDOMGsmIccInfo : nsIDOMIccInfo {
>   // data only available for GSM
> }
> 
> interface nsIDOMCdmaIccInfo : nsIDOMIccInfo {
>   // data only available for CDMA
> }
> 
> Or should we discuss on dev-webapi?
> 
> Thanks

Can a single SIM card contain GSM and CDMA information? I think your design is cleaner but it it might be an overkill if we add it because a single field differs. Do we expect further attributes that are GSM or CDMA only?

Comment 16

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Gregor Wagner [:gwagner] from comment #15)
> Can a single SIM card contain GSM and CDMA information? 

No, as far as I know.

>I think your design
> is cleaner but it it might be an overkill if we add it because a single
> field differs. Do we expect further attributes that are GSM or CDMA only?

Hi, Gregor

For mozIccManager, I think there won't be too much differences between CDMA and GSM.
So maybe just less than 3 attributes will only for CDMA.

But for WebMobileConnection there might be more, signalStrength, Cell Location, ...etc.

Since now we're starting the land those CDMA features, so we'd like to know how WebAPI should be designed here.

Thanks

Comment 17

[Gregor Wagner [:gwagner]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #16)
> (In reply to Gregor Wagner [:gwagner] from comment #15)
> > Can a single SIM card contain GSM and CDMA information? 
> 
> No, as far as I know.
> 
> >I think your design
> > is cleaner but it it might be an overkill if we add it because a single
> > field differs. Do we expect further attributes that are GSM or CDMA only?
> 
> Hi, Gregor
> 
> For mozIccManager, I think there won't be too much differences between CDMA
> and GSM.
> So maybe just less than 3 attributes will only for CDMA.
> 
> But for WebMobileConnection there might be more, signalStrength, Cell
> Location, ...etc.
> 
> Since now we're starting the land those CDMA features, so we'd like to know
> how WebAPI should be designed here.
> 
> Thanks

I would create a base interface for the common properties that is hidden. I think we have a noInterface keyword for it. And then you create a CDMA and GSM interface that inherit the base interface.

Comment 18

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Thanks Gregor.

Edgar, can you try when Gergor said in Comment 17?

Thanks

Comment 19

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #18)
> Thanks Gregor.
> 
> Edgar, can you try when Gergor said in Comment 17?
> 
> Thanks

Sure!!
Thanks Yoshi and Gregor.

Comment 20

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA MIN, v2

Comment 21

[Edgar Chen [:edgar]]

Comment on attachment 792659 [details] [diff] [review]
Part 1: Expose CDMA MIN, v2

Yoshi, Gregor, could you give me some feedback about this new interface? Thanks in advance.

Comment 22

[Edgar Chen [:edgar]]

I found another problem when trying to implements the new interface changes. When the card is not detected any more (let's say device is in airplane mode), iccInfo still keeps the old data. This may confuse the API User. In this case, I think iccInfo should changes to null.

Comment 23

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 792659 [details] [diff] [review]
Part 1: Expose CDMA MIN, v2

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

The MIN looks good to me,

but there's another thing,
I think MSISDN is a GSM term.
In CDMA it should be 'MDN'.

Comment 24

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #23)
> Comment on attachment 792659 [details] [diff] [review]
> Part 1: Expose CDMA MIN, v2
> 
> Review of attachment 792659 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The MIN looks good to me,
> 
> but there's another thing,
> I think MSISDN is a GSM term.
> In CDMA it should be 'MDN'.

Okay, I will correct this in next version. Thanks

Comment 25

[Edgar Chen [:edgar]]

Comment on attachment 792659 [details] [diff] [review]
Part 1: Expose CDMA MIN, v2

Cancel feedback, need to address comment #23 first.

Comment 26

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA MIN, v3

Address comment #23

Comment 27

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 793363 [details] [diff] [review]
Part 1: Expose CDMA MIN, v3

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

::: dom/icc/interfaces/nsIDOMIccInfo.idl
@@ +58,5 @@
> +[scriptable, uuid(013e973e-8b56-4525-b634-d23166b86edb)]
> +interface nsIDOMMozCdmaIccInfo : nsIDOMMozIccInfo
> +{
> +  /**
> +   * Mobile Directory Number (MDN) of the subscriber's, aka

remove 's

Comment 28

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA MIN, v4

Address comment #27.

Comment 29

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA MIN, v5

Correct comment: "sim", "usim", "ruim" ...

Comment 30

[Edgar Chen [:edgar]]

Attachment: Part 2: Move mbdn to iccInfoPrivate

Cache mbdn in iccInfoPrivate to prevent mbdn been sent through IPC message.

Comment 31

[Edgar Chen [:edgar]]

Attachment: Part 3: RIL implementation for CDMA MIN, v2

1). Correct behavior of comment #22.
2). RIL implementation for MDN, MIN, iccType.

Comment 32

[Edgar Chen [:edgar]]

Attachment: Part 4: PhoneNumberUtils changes for iccInfo, v1

iccInfo could be null, handle this case in PhoneNumberUtils.

Comment 33

[Edgar Chen [:edgar]]

Attachment: Part 5: GPS changes for msisdn, v1

We move msisdn to nsIMozGsmIccInfo, should do corresponding changes in GPS.

Comment 34

[Edgar Chen [:edgar]]

Attachment: Part 6: Bluetooth changes for msisdn, v1

We move msisdn to nsIMozGsmIccInfo, should do corresponding changes in bluetooth.

Comment 35

[Edgar Chen [:edgar]]

Attachment: Part 7: Marionette tests for iccInfo, v1

Marionette tests

Comment 36

[Edgar Chen [:edgar]]

Comment on attachment 793406 [details] [diff] [review]
Part 1: Expose CDMA MIN, v5

Hi Yoshi, Mounir:

Please help to review the interface changes which is based on suggestion in comment #17 and comment #23.

Thanks

Comment 37

[Edgar Chen [:edgar]]

Attachment: Part 3: RIL implementation for CDMA MIN, v3

Comment 38

[Edgar Chen [:edgar]]

Comment on attachment 793435 [details] [diff] [review]
Part 4: PhoneNumberUtils changes for iccInfo, v1

Gregor, could you help to review this changes in PhoneNumberUtils? In this bug, we correct the behavior for the icc card is not detected. The iccInfo could be null in this case, and PhoneNumberUtils needs to handle this. Thanks

Comment 39

[Edgar Chen [:edgar]]

Comment on attachment 793439 [details] [diff] [review]
Part 5: GPS changes for msisdn, v1

Hi Kan-Ru:

In this bug, we move msisdn to nsIDOMGsmIccInfo because msisdn is a GSM specific item. And GPS side needs to do corresponding changes. Could you help to review this?

Thanks

Comment 40

[Edgar Chen [:edgar]]

Comment on attachment 793446 [details] [diff] [review]
Part 6: Bluetooth changes for msisdn, v1

Hi Eric:

In this bug, we move msisdn to nsIDOMGsmIccInfo because msisdn is a GSM specific item. And Bluetooth side needs to do corresponding changes. Could you help to review this?

Thanks

Comment 41

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 793439 [details] [diff] [review]
Part 5: GPS changes for msisdn, v1

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

::: dom/system/gonk/GonkGPSGeolocationProvider.cpp
@@ +430,5 @@
> +        iccInfo->GetIccType(iccType);
> +
> +        if (iccType.EqualsLiteral("sim") ||
> +            iccType.EqualsLiteral("usim")) {
> +          nsCOMPtr<nsIDOMMozGsmIccInfo> gsmIccInfo = do_QueryInterface(iccInfo);

I think you could

 nsCOMPtr<nsIDOMMozGsmIccInfo> gsmIccInfo = do_QueryInterface(iccInfo);
 if (gsmIccInfo) {
   ...
 }

so no need to check iccType.

Comment 42

[Edgar Chen [:edgar]]

Comment on attachment 793439 [details] [diff] [review]
Part 5: GPS changes for msisdn, v1

I will address comment #41, cancel review first.

Comment 43

[Edgar Chen [:edgar]]

Comment on attachment 793446 [details] [diff] [review]
Part 6: Bluetooth changes for msisdn, v1

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

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +650,5 @@
> +  nsString iccType;
> +  iccInfo->GetIccType(iccType);
> +  if (iccType.EqualsLiteral("sim") ||
> +      iccType.EqualsLiteral("usim")) {
> +    nsCOMPtr<nsIDOMMozGsmIccInfo> gsmIccInfo = do_QueryInterface(iccInfo);

I will use the same way in comment #41. Cancel review first.

Comment 44

[Edgar Chen [:edgar]]

Attachment: Part 5: GPS changes for msisdn, v2

Address review comment #41

Comment 45

[Edgar Chen [:edgar]]

Attachment: Part 6: Bluetooth changes for msisdn, v2

Address comment #43

Comment 46

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 793829 [details] [diff] [review]
Part 3: RIL implementation for CDMA MIN, v3

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

::: dom/system/gonk/RILContentHelper.js
@@ +145,5 @@
>                        retryCount: 'r',
>                        success: 'r'}
>  };
>  
> +function MozGsmIccInfo() {}

Why Moz prefix?

@@ +169,2 @@
>    msisdn: null
>  };

Cannot we do 
GsmIccInfo.prototype = IccInfo.prototype;
and add GSM specific data?

@@ +515,5 @@
> +        this.updateInfo(newInfo, this.rilContext.iccInfo);
> +      }
> +    } else {
> +      this.rilContext.iccInfo = null;
> +    }

This function looks complicated.

::: dom/system/gonk/ril_worker.js
@@ +12574,5 @@
>     */
>    handleICCInfoChange: function handleICCInfoChange() {
> +    RIL.sendChromeMessage({
> +      rilMessageType: "iccinfochange",
> +      data: RIL.iccInfo

Why a data property?

Comment 47

[Francis Lee [:frlee]]

according to the RIL triage meeting consensus on 8/22, koi+ this issue.

Comment 48

[Anshul]

Edger, if we separate gsm and cdma fields in rilContext than UI needs an additional logic to figure out which field to show msisdn or mdn based on radio tech. I think that logic can reside in RIL itself to make gaia side changes cleaner.

Also as I am discussing on bug 890820, the min value might not be needed to be exposed in rilcontext and given that you might not need to separate gsmiccinfo and cdmaiccinfo.

Comment 49

[Edgar Chen [:edgar]]

Oh, I just realize you already have some comments for this patch.

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #46)
> Comment on attachment 793829 [details] [diff] [review]
> Part 3: RIL implementation for CDMA MIN, v3
> 
> Review of attachment 793829 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RILContentHelper.js
> @@ +145,5 @@
> >                        retryCount: 'r',
> >                        success: 'r'}
> >  };
> >  
> > +function MozGsmIccInfo() {}
> 
> Why Moz prefix?
Just because nsIDOMMozGsmIccInfo has Moz prefix, so I use the same naming.

> 
> @@ +169,2 @@
> >    msisdn: null
> >  };
> 
> Cannot we do 
> GsmIccInfo.prototype = IccInfo.prototype;
> and add GSM specific data?

Okay, it looks better.
BTW, we also need to override QueryInterface, classInfo ... etc, right?

> 
> @@ +515,5 @@
> > +        this.updateInfo(newInfo, this.rilContext.iccInfo);
> > +      }
> > +    } else {
> > +      this.rilContext.iccInfo = null;
> > +    }
> 
> This function looks complicated.
There are some cases we need to consider:
1. iccInfo can be null.
2. We need create corresponding object based on iccType.
3. If card is not detected (iccType is null), we need to clear iccInfo to null.

I will add more comment for this function.

> 
> ::: dom/system/gonk/ril_worker.js
> @@ +12574,5 @@
> >     */
> >    handleICCInfoChange: function handleICCInfoChange() {
> > +    RIL.sendChromeMessage({
> > +      rilMessageType: "iccinfochange",
> > +      data: RIL.iccInfo
> 
> Why a data property?

Oh, because I want to avoid |rilMessageType| be sent through IPC.
RadioInterfaceLayer can take data property which only contains the useful information and don't need to consider to filter out the redundant information, like |rilMessageType|.

Comment 50

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Also we didn't expose IMSI for GSM for security reason, Bug 782603
I think MIN is the IMSI for CDMA world,

is it okay if we expose this for CDMA?

Comment 51

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #50)
> Also we didn't expose IMSI for GSM for security reason, Bug 782603
> I think MIN is the IMSI for CDMA world,
> 
> is it okay if we expose this for CDMA?

Thanks for this information.MINat
I think we have the same concern in MIN.

Hi Ken, if we left checking MIN to Gaia, we need to expose MIN to MozMobileConnection. But it seems we may have a security concern for that. Do we have any conclusion for the OTASP?

Thanks

Comment 52

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #51)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #50)
> > Also we didn't expose IMSI for GSM for security reason, Bug 782603
> > I think MIN is the IMSI for CDMA world,
> > 
> > is it okay if we expose this for CDMA?
> 
> Thanks for this information.MINat
> I think we have the same concern in MIN.
> 
> Hi Ken, if we left checking MIN to Gaia, we need to expose MIN to
> MozMobileConnection. But it seems we may have a security concern for that.
  ^^^^^^^^^^^^^^^^^^^
Sorry, it should be MozIccInfo.

> Do we have any conclusion for the OTASP?
> 
> Thanks

Comment 53

[Edgar Chen [:edgar]]

Comment on attachment 793406 [details] [diff] [review]
Part 1: Expose CDMA MIN, v5

Cancel review request due to we need a final conclusion for OTASP implementation.

Comment 54

[Edgar Chen [:edgar]]

Comment on attachment 793829 [details] [diff] [review]
Part 3: RIL implementation for CDMA MIN, v3

Cancel review request due to we need a final conclusion for OTASP implementation.

Comment 55

[Edgar Chen [:edgar]]

Comment on attachment 793447 [details] [diff] [review]
Part 7: Marionette tests for iccInfo, v1

Cancel review request due to we need a final conclusion for OTASP implementation.

Comment 56

[Edgar Chen [:edgar]]

Comment on attachment 793410 [details] [diff] [review]
Part 2: Move mbdn to iccInfoPrivate

Cancel review request due to we need a final conclusion for OTASP implementation.

Comment 57

[Ken Chang[:kenkai]]

(In reply to Edgar Chen [:edgar][:echen] from comment #51)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #50)
> > Also we didn't expose IMSI for GSM for security reason, Bug 782603
> > I think MIN is the IMSI for CDMA world,
> > 
> > is it okay if we expose this for CDMA?
> 
> Thanks for this information.MINat
> I think we have the same concern in MIN.
> 
> Hi Ken, if we left checking MIN to Gaia, we need to expose MIN to
> MozMobileConnection. But it seems we may have a security concern for that.
> Do we have any conclusion for the OTASP?
> 
> Thanks
MIN is the telephone number. I wonder if it has the security issue in user device. However, we should only expose "MIN" instead of exposing all "IMSI_M". This means that it is enough to expose the byte 2,3,4,5,and 6 of IMSI_M.

Comment 58

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #50)
> Also we didn't expose IMSI for GSM for security reason, Bug 782603
> I think MIN is the IMSI for CDMA world,
> 
> is it okay if we expose this for CDMA?

There are two kinds of IMSI for CDMA, IMSI_M and IMSI_T and MIN is just a part of IMSI_M [1][2]. I think it is ok to expose it.

[1] C.S0005-F_v1.0 session 2.3.1.
[2] C.S0023-D_v2.0 session 3.4.2.

Comment 59

[Edgar Chen [:edgar]]

Attachment: Part 4: PhoneNumberUtils changes for iccInfo, v1, r=gwagner

Add r=gwagner after r+.

Comment 60

[Edgar Chen [:edgar]]

Attachment: Part 3: RIL implementation for CDMA MIN, v4

Address the suggestion in comment #46
1). Remove Moz prefix.
2). User GsmIccInfo.prototype = IccInfo.prototype ...
3). Rewrite updateIccInfo and add more comments for it.

Comment 61

[Anshul]

(In reply to Edgar Chen [:edgar][:echen] from comment #51)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #50)
> > Also we didn't expose IMSI for GSM for security reason, Bug 782603
> > I think MIN is the IMSI for CDMA world,
> > 
> > is it okay if we expose this for CDMA?
> 
> Thanks for this information.MINat
> I think we have the same concern in MIN.
> 
> Hi Ken, if we left checking MIN to Gaia, we need to expose MIN to
> MozMobileConnection. But it seems we may have a security concern for that.
> Do we have any conclusion for the OTASP?
> 
> Thanks
Edgar, don't think there was a conclusion on using MIN by gaia to determine the OTASP state. If we are exposing MIN just for that reason that may be we should hold on for some more time.

Comment 62

[Ken Chang[:kenkai]]

(In reply to Anshul from comment #61)
> Edgar, don't think there was a conclusion on using MIN by gaia to determine
> the OTASP state. If we are exposing MIN just for that reason that may be we
> should hold on for some more time.
It's just in review stage now. By the way, if you can provide any result of your experiement, it will be very helpful for us.

Comment 63

[Anshul]

Comment on attachment 793406 [details] [diff] [review]
Part 1: Expose CDMA MIN, v5

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

::: dom/icc/interfaces/nsIDOMIccInfo.idl
@@ +53,3 @@
>     * his phone number.
>     */
>    readonly attribute DOMString msisdn;

msisdn for GSM and mdn for CDMA is implementation detail so I don't think gaia should be making this decision of whether or not to show msisdn or mdn. If the name is of concern, may be change the name to call it phoneNumber which is what it really is.

Comment 64

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Ken Chang from comment #57)
> MIN is the telephone number. I wonder if it has the security issue in user
> device. However, we should only expose "MIN" instead of exposing all
> "IMSI_M". This means that it is enough to expose the byte 2,3,4,5,and 6 of
> IMSI_M.

From Edgar's IDL, telephone number should be MDN.

Comment 65

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Anshul from comment #63)
> ::: dom/icc/interfaces/nsIDOMIccInfo.idl
> @@ +53,3 @@
> >     * his phone number.
> >     */
> >    readonly attribute DOMString msisdn;
> 
> msisdn for GSM and mdn for CDMA is implementation detail so I don't think
> gaia should be making this decision of whether or not to show msisdn or mdn.
> If the name is of concern, may be change the name to call it phoneNumber
> which is what it really is.

Hi, Anshul
Right now we're trying to make our interfaces cleaner between GSM and CDMA,
 
See Comment 12, Comment 15, Comment 16 and Comment 17.

I agree your concern here that Gaia needs to worry about some data structure between different radio technologies.
Gaia could have some kinds of utilities to address that, or we could create another high level abstraction API on top of this API, and you're welcome to file a bug or discuss on the web-api ML.

Comment 66

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 796453 [details] [diff] [review]
Part 3: RIL implementation for CDMA MIN, v4

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

::: dom/system/gonk/RILContentHelper.js
@@ +513,5 @@
> +    }
> +
> +    // If iccInfo is null, new corresponding object based on iccType.
> +    if (!this.rilContext.iccInfo) {
> +      if (newInfo.iccType === "ruim") {

What happens when iccType is 'csim'?

::: dom/system/gonk/ril_worker.js
@@ +6272,5 @@
> +  let result = Buf.readStringList();
> +
> +  this.iccInfo.mdn = result[0];
> +  // The result[1] is Home SID. (Already be handled in readCDMAHome())
> +  // The result[2] is Home NID. (Already be handled in readCDMAHome())

If RIL has already implemented getting SID/NID, why don't we use this RilRequest to get those information?

@@ +12574,5 @@
>     */
>    handleICCInfoChange: function handleICCInfoChange() {
> +    RIL.sendChromeMessage({
> +      rilMessageType: "iccinfochange",
> +      data: RIL.iccInfo

You add another property 'data' here?
Why?

Comment 67

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Edgar
I found when in CDMA, ril_worker will also get getIMSI(), so I think the IMSI-related thing is handling there.
Can you provide some examples for the MIN you got right now?

Thanks

Comment 68

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #66)
> Comment on attachment 796453 [details] [diff] [review]
> Part 3: RIL implementation for CDMA MIN, v4
> 
> Review of attachment 796453 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RILContentHelper.js
> @@ +513,5 @@
> > +    }
> > +
> > +    // If iccInfo is null, new corresponding object based on iccType.
> > +    if (!this.rilContext.iccInfo) {
> > +      if (newInfo.iccType === "ruim") {
> 
> What happens when iccType is 'csim'?
Hmm, we should use cdmaIccInfo when iccType is 'csim', I will address this in next version.

> 
> ::: dom/system/gonk/ril_worker.js
> @@ +6272,5 @@
> > +  let result = Buf.readStringList();
> > +
> > +  this.iccInfo.mdn = result[0];
> > +  // The result[1] is Home SID. (Already be handled in readCDMAHome())
> > +  // The result[2] is Home NID. (Already be handled in readCDMAHome())
> 
> If RIL has already implemented getting SID/NID, why don't we use this
> RilRequest to get those information?
Sounds good, I will file a follow up bug for this.

> 
> @@ +12574,5 @@
> >     */
> >    handleICCInfoChange: function handleICCInfoChange() {
> > +    RIL.sendChromeMessage({
> > +      rilMessageType: "iccinfochange",
> > +      data: RIL.iccInfo
> 
> You add another property 'data' here?
> Why?

Oh, because I want to avoid |rilMessageType| be sent through IPC.
RadioInterfaceLayer can take data property which only contains the useful information and don't need to consider to filter out the redundant information, like |rilMessageType|.

Comment 69

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Edgar Chen [:edgar][:echen] from comment #68)
> > @@ +12574,5 @@
> > >     */
> > >    handleICCInfoChange: function handleICCInfoChange() {
> > > +    RIL.sendChromeMessage({
> > > +      rilMessageType: "iccinfochange",
> > > +      data: RIL.iccInfo
> > 
> > You add another property 'data' here?
> > Why?
> 
> Oh, because I want to avoid |rilMessageType| be sent through IPC.
> RadioInterfaceLayer can take data property which only contains the useful
> information and don't need to consider to filter out the redundant
> information, like |rilMessageType|.

I didn't see this pattern from ril_worker.js,
I think we need to unify the usage for calling sendChromeMessage.
We could file another bug to discuss this.
But unless it's breaking your code, we shouldn't make this change.

Comment 70

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #69)
> (In reply to Edgar Chen [:edgar][:echen] from comment #68)
> > > @@ +12574,5 @@
> > > >     */
> > > >    handleICCInfoChange: function handleICCInfoChange() {
> > > > +    RIL.sendChromeMessage({
> > > > +      rilMessageType: "iccinfochange",
> > > > +      data: RIL.iccInfo
> > > 
> > > You add another property 'data' here?
> > > Why?
> > 
> > Oh, because I want to avoid |rilMessageType| be sent through IPC.
> > RadioInterfaceLayer can take data property which only contains the useful
> > information and don't need to consider to filter out the redundant
> > information, like |rilMessageType|.
> 
> I didn't see this pattern from ril_worker.js,
> I think we need to unify the usage for calling sendChromeMessage.
> We could file another bug to discuss this.
> But unless it's breaking your code, we shouldn't make this change.

I see, I will remove this change in next version and file another bug for this.

Comment 71

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #67)
> Hi, Edgar
> I found when in CDMA, ril_worker will also get getIMSI(), so I think the
> IMSI-related thing is handling there.
> Can you provide some examples for the MIN you got right now?
> 
> Thanks

The min we got from 亞太 sim card is "0080372592".

09-03 15:25:13.095   198   198 I Gecko   : -*- RILContentHelper: Received message 'RIL:IccInfoChanged': {"clientId":0,"data":{"iccType":"ruim","isDisplayNetworkNameRequired":false,"isDisplaySpnRequired":false,"mdn":"","min":"0080372592","mcc":"454","mnc":"00","iccid":"89886050113010023402","spn":"亞太電信"}}

Comment 72

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #71)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #67)
> > Hi, Edgar
> > I found when in CDMA, ril_worker will also get getIMSI(), so I think the
> > IMSI-related thing is handling there.
> > Can you provide some examples for the MIN you got right now?
> > 
> > Thanks
> 
> The min we got from 亞太 sim card is "0080372592".
> 
> 09-03 15:25:13.095   198   198 I Gecko   : -*- RILContentHelper: Received
> message 'RIL:IccInfoChanged':
> {"clientId":0,"data":{"iccType":"ruim","isDisplayNetworkNameRequired":false,
> "isDisplaySpnRequired":false,"mdn":"","min":"0080372592","mcc":"454","mnc":
> "00","iccid":"89886050113010023402","spn":"亞太電信"}}

The IMSI we got from REQUEST_GET_IMSI is "454006302417369" (亞太 sim card).
And the MIN we got from REQUEST_CDMA_SUBSCRIPTION is "0080372592".

It seems the result of REQUEST_GET_IMSI is not IMSI_M (maybe it is IMSI_T, I am not sure the implementation of this ril request, REQUEST_GET_IMSI). If the result is not IMSI_M, then we are unable to get MIN from it.

=======================================
01-13 10:02:35.090   198   420 I Gecko   : RIL Worker[0]: IMSI: 454006302417369
01-13 10:02:35.090   198   198 I Gecko   : -*- RadioInterface[0]: Received message from worker: {"iccType":"ruim","rilMessageType":"iccinfochange","isDisplayNetworkNameRequired":true,"isDisplaySpnRequired":false,"mdn":"","min":"0080372592","mcc":"454","mnc":"00"}

Comment 73

[Edgar Chen [:edgar]]

Attachment: Part 3: RIL implementation for CDMA MIN, v5

Address review comment:
1). If iccType is csim, we should use CdmaIccInfo (comment #66).
2). Remove 'data' property. (comment #69)

Comment 74

[Edgar Chen [:edgar]]

Attachment: Part 1: Expose CDMA MIN, v5, r=allstars.chh, sr=sicking

Add r=allstars.chh and sr=sicking after r+ and sr+.

Comment 75

[Edgar Chen [:edgar]]

Attachment: Part 2: Move mbdn to iccInfoPrivate, v1, r=allstars.chh

Add r=allstars.chh after r+.

Comment 76

[Edgar Chen [:edgar]]

Attachment: Part 3: RIL implementation for CDMA MIN, v6, r=allstars.chh

Correct coding style: missing ';'.

Comment 77

[Edgar Chen [:edgar]]

Attachment: Part 7: Marionette tests for iccInfo, v1, r=allstars.chh

Add r=allstars.chh after r+.

Comment 78

[Edgar Chen [:edgar]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=585168096459

Comment 79

[Edgar Chen [:edgar]]

https://hg.mozilla.org/integration/b2g-inbound/rev/50a1947a8ede
https://hg.mozilla.org/integration/b2g-inbound/rev/ff5b29debc08
https://hg.mozilla.org/integration/b2g-inbound/rev/2d4c400127af
https://hg.mozilla.org/integration/b2g-inbound/rev/65938897536e
https://hg.mozilla.org/integration/b2g-inbound/rev/1eebc1edf40a
https://hg.mozilla.org/integration/b2g-inbound/rev/7d47a1885014
https://hg.mozilla.org/integration/b2g-inbound/rev/9dcc96e41330

Comment 80

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

https://hg.mozilla.org/mozilla-central/rev/50a1947a8ede
https://hg.mozilla.org/mozilla-central/rev/ff5b29debc08
https://hg.mozilla.org/mozilla-central/rev/2d4c400127af
https://hg.mozilla.org/mozilla-central/rev/65938897536e
https://hg.mozilla.org/mozilla-central/rev/1eebc1edf40a
https://hg.mozilla.org/mozilla-central/rev/7d47a1885014
https://hg.mozilla.org/mozilla-central/rev/9dcc96e41330

Comment 81

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

In MmsService[1] we have:

  1234   getMsisdn: function getMsisdn() {
  1235     let iccInfo = gRadioInterface.rilContext.iccInfo;
  1236     let number = iccInfo ? iccInfo.msisdn : null;

Since |iccInfo| is now a nsIDOMMozIccInfo, which has no |msisdn| property because it's moved into nsIDOMMozGsmIccInfo, this triggers a silent bug that we can't have MSISDN in components other than RadioInterface itself.

[1]: http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/MmsService.js#1234