Closed Bug 1114938 Opened 5 years ago Closed 5 years ago

[B2G][ICC] Refactor STK in MozIcc.webidl with IPDL.

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(tracking-b2g:backlog, firefox41 fixed)

RESOLVED FIXED
2.2 S13 (29may)
tracking-b2g backlog
Tracking Status
firefox41 --- fixed

People

(Reporter: bevis, Assigned: bevis)

References

Details

Attachments

(8 files, 20 obsolete files)

51.15 KB, patch
bevis
: review+
Details | Diff | Splinter Review
18.08 KB, patch
bevis
: review+
Details | Diff | Splinter Review
48.14 KB, patch
bevis
: review+
Details | Diff | Splinter Review
9.77 KB, patch
bevis
: review+
Details | Diff | Splinter Review
2.03 KB, patch
bevis
: review+
Details | Diff | Splinter Review
8.17 KB, patch
bevis
: review+
Details | Diff | Splinter Review
13.39 KB, patch
bevis
: review+
Details | Diff | Splinter Review
1.07 KB, patch
bevis
: review+
Details | Diff | Splinter Review
I'd like to divide bug 864489 into several tasks.

This bug to refactor STK with IPDL.
Summary: Bug 1114937 - [B2G][ICC] Refactor STK in MozIccManager with IPDL. → [B2G][ICC] Refactor STK in MozIccManager with IPDL.
It shall be MozIcc.webidl instead of MozIccManager.webidl to prevent confusing.
Summary: [B2G][ICC] Refactor STK in MozIccManager with IPDL. → [B2G][ICC] Refactor STK in MozIcc.webidl with IPDL.
Blocks: b2g-stk
[Tracking Requested - why for this release]:
Depends on: 1155142
Depends on: 1159134
This patch is to 
1. Refactor StkProactiveCmdFactory.jsm as a XPCOM service.
2. Move nsIStkXXX interfaces from nsIIccMessenger.idl to nsIStkProactiveCmd.idl where nsIStkProactiveCmd.idl is the pool for all STK related XPCOM components.

Hi Edgar,

May I have your early feedback for this changes?

Thanks!
Attachment #8605777 - Flags: feedback?(echen)
This patch is to move the notification of STK proactive commands from IccProvider to IccService.

Hi Edgar,

May I have your early feedback for these changes?

Thanks!
Attachment #8605779 - Flags: feedback?(echen)
This patch is to move the STK related requests from IccProvider to IccService.

Hi Edgar,

May I have your early feedback for these changes?

Thanks!
Attachment #8605781 - Flags: feedback?(echen)
This patch is to deprecate the implementation in IccProvider.

Hi Edgar,

May I have your early feedback for these changes?

Thanks!
Attachment #8605782 - Flags: feedback?(echen)
This patch is to correct the data type of mcc/mnc to DOMString which is actually used by gaia, IccInfo, MobileConnection.

Hi Edgar,

May I have your early feedback of this change?

Thanks!
Attachment #8605784 - Flags: feedback?(echen)
Comment on attachment 8605777 [details] [diff] [review]
Part 1 v1: Refactor StkProactiveCmdFactory.jsm into a XPCOM Service.

Per offline discussion, we can have more easier way to implement IPDL with JSON.
Since it won't take too much time to finish the IPDL implementation,
I'll provide formal patch to review directly.
Attachment #8605777 - Attachment is obsolete: true
Attachment #8605777 - Flags: feedback?(echen)
Comment on attachment 8605779 [details] [diff] [review]
Part 2.1 v1: (Gonk) Refactor Stk Proactive Commands from IccProvider to IccService.

Per offline discussion, we can have more easier way to implement IPDL with JSON.
Since it won't take too much time to finish the IPDL implementation,
I'll provide formal patch to review directly.
Attachment #8605779 - Attachment is obsolete: true
Attachment #8605779 - Flags: feedback?(echen)
Comment on attachment 8605781 [details] [diff] [review]
Part 2.2 v1: (Gonk) Refactor Stk Request APIs from IccProvider to IccService.

Per offline discussion, we can have more easier way to implement IPDL with JSON.
Since it won't take too much time to finish the IPDL implementation,
I'll provide formal patch to review directly.
Attachment #8605781 - Attachment is obsolete: true
Attachment #8605781 - Flags: feedback?(echen)
Comment on attachment 8605782 [details] [diff] [review]
Part 2.3 v1: (Gonk) Remove Implementation from IccProvider.

Per offline discussion, we can have more easier way to implement IPDL with JSON.
Since it won't take too much time to finish the IPDL implementation,
I'll provide formal patch to review directly.
Attachment #8605782 - Attachment is obsolete: true
Attachment #8605782 - Flags: feedback?(echen)
Comment on attachment 8605784 [details] [diff] [review]
Part 2.4 v1: Type of MNC/MCC shall be the same to the one defined in MozMobileNetworkInfo.

Per offline discussion, we can have more easier way to implement IPDL with JSON.
Since it won't take too much time to finish the IPDL implementation,
I'll provide formal patch to review directly.
Attachment #8605784 - Attachment is obsolete: true
Attachment #8605784 - Flags: feedback?(echen)
1. Move STK related XPCOM interfaces from nsIIccMessenger.idl to newly created nsIStkProactiveCmd.idl
2. Refactor/Rename StkProactiveCmdFactory.jsm to StkCmdFactory.js as a XPCOM service.

Hi Edgar,

May I have your review for these changes?

Thanks!
Attachment #8606862 - Flags: review?(echen)
This patch is to migrate the notification of STK proactive command from IccProvider to IccService.

Hi Edgar,

May I have your review for these changes?

Thanks!
Attachment #8606863 - Flags: review?(echen)
This patch is to migrate STK related requests from IccProvider to IccService.

Hi Edgar,

May I have your review for these changes?

Thanks!
Attachment #8606869 - Flags: review?(echen)
This Patch is to remove the STK related implementation from IccProvider.
Attachment #8606871 - Flags: review?(echen)
Add IPDL declaration for STK-related transactions.
Attachment #8606873 - Flags: review?(echen)
Add Helper to deflate/inflate XPCOM object to/from JSON for IPC.
Attachment #8606874 - Flags: review?(echen)
Add IPDL implementation.
Attachment #8606875 - Flags: review?(echen)
Hi Edgar & Hsinyi,

Actually, the data type of mnc/mcc used by Icc [1], MobileConnection [2] and Gaia [3] is a DOMString instead of an integer type.

Hence, I'd like to modification the webidl definition accordingly to prevent any confusion.

Would you minding reviewing this change?

Thanks!

[1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozIccInfo.webidl#21-29
[2] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozMobileNetworkInfo.webidl#20-28
[3] https://github.com/mozilla-b2g/gaia/blob/d544c9b2a77a889d87566aff0ca957da5089a49e/apps/system/js/icc_events.js#L13-L47
Attachment #8606876 - Flags: review?(htsai)
Attachment #8606876 - Flags: review?(echen)
Comment on attachment 8606862 [details] [diff] [review]
(v2) Part 1: Refactor StkProactiveCmdFactory.jsm into a XPCOM Service.

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

Looks good to me. Thank you.

::: dom/icc/interfaces/nsIStkCmdFactory.idl
@@ +27,5 @@
> +   */
> +  nsIStkProactiveCmd createCommand(in jsval aCommandDetails);
> +
> +  /**
> +   * @param  nsIStkProactiveCmd instance.

Nit:
@param aStkProactiveCmd
       a nsIStkProactiveCmd instance.

::: dom/system/gonk/RILSystemMessenger.jsm
@@ +33,5 @@
>    },
>  
>    /**
> +   * Hook of the function to create MozStkCommand message.
> +   * @param  nsIStkProactiveCmd instance.

Ditto.
Attachment #8606862 - Flags: review?(echen) → review+
Comment on attachment 8606863 [details] [diff] [review]
(v2) Part 2.1: (Gonk) Refactor Stk Proactive Commands from IccProvider to IccService.

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

Mostly looks good but I would like to review the revised version again. Thank you.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +2025,5 @@
>      if (DEBUG) this.debug("handleStkProactiveCommand " + JSON.stringify(message));
>      let iccInfo = this.getIccInfo();
>      if (iccInfo && iccInfo.iccid) {
> +      let command = gStkCmdFactory.createCommand(message);
> +      gIccMessenger.notifyStkProactiveCommand(iccInfo.iccid, command);

Handle system message things in IccService.
And some utility functions probably can be removed as well, e.g. getIccInfo() ... etc.
Attachment #8606863 - Flags: review?(echen)
(In reply to Edgar Chen [:edgar][:echen] from comment #21)
> 
> ::: dom/icc/interfaces/nsIStkCmdFactory.idl
> @@ +27,5 @@
> > +   */
> > +  nsIStkProactiveCmd createCommand(in jsval aCommandDetails);
> > +
> > +  /**
> > +   * @param  nsIStkProactiveCmd instance.
> 
> Nit:
> @param aStkProactiveCmd
>        a nsIStkProactiveCmd instance.
> 
Oops, this change was done in patch part 2.2 accidentally.
I'll move it to patch part 2.1 instead.
(In reply to Edgar Chen [:edgar][:echen] from comment #22)
> Comment on attachment 8606863 [details] [diff] [review]
> (v2) Part 2.1: (Gonk) Refactor Stk Proactive Commands from IccProvider to
> IccService.
> 
> Review of attachment 8606863 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Mostly looks good but I would like to review the revised version again.
> Thank you.
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +2025,5 @@
> >      if (DEBUG) this.debug("handleStkProactiveCommand " + JSON.stringify(message));
> >      let iccInfo = this.getIccInfo();
> >      if (iccInfo && iccInfo.iccid) {
> > +      let command = gStkCmdFactory.createCommand(message);
> > +      gIccMessenger.notifyStkProactiveCommand(iccInfo.iccid, command);
> 
> Handle system message things in IccService.
> And some utility functions probably can be removed as well, e.g.
> getIccInfo() ... etc.

Thanks for reminding this.
I'll address this in next update!
Comment on attachment 8606869 [details] [diff] [review]
(v2) Part 2.2: (Gonk) Refactor Stk Request APIs from IccProvider to IccService.

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

Please see my comments below, thank you.

::: dom/icc/gonk/StkCmdFactory.js
@@ +200,4 @@
>  };
>  
>  /**
> + * The implementation of nsIStkProactiveCommand set and paired JS object set.

Move this change to Part 1.

@@ +1038,5 @@
> +    this.additionalInformation = aStkTerminalResponse.additionalInformation;
> +  }
> +}
> +StkResponseMessage.prototype = {
> +  resultCode: RIL.STK_RESULT_OK

Most of Stk*Message doesn't give a default value to their non-optional attribute. Why give a default value for this one? Is there any special reason?

@@ +1077,5 @@
> +  if (aStkGetInputResponseMessage.input !== undefined) {
> +    // We expect input to be "" if user confirmed the input with nothing,
> +    // and we use null to present 'undefined' internally for the conversion
> +    // between nsIStkTerminalResponse and JS objects.
> +    this.input = (aStkGetInputResponseMessage.input === null)

this.input = aStkGetInputResponseMessage.input || "";

@@ +1505,5 @@
> +      throw new Error("Invalid event message: " + JSON.stringify(aEventMessage));
> +    }
> +
> +    switch (aEventMessage.eventType) {
> +      case RIL.STK_EVENT_TYPE_MT_CALL:

We have a mapping for proactive command. e.g. `MsgPrototypes[RIL.STK_CMD_FOO] = StkBar;`.
Should we use the same way for download event?

@@ +1528,5 @@
> +      throw new Error("Invalid download event: " + JSON.stringify(aStkDownloadEvent));
> +    }
> +
> +    let event;
> +    if (aStkDownloadEvent instanceof Ci.nsIStkLocationEvent) {

Ditto, we have a mapping for proactive command, e.g. `QueriedIFs[RIL.STK_CMD_FOO] = Ci.nsIStkBar;`.
Should we use the same way for download event?

::: dom/icc/interfaces/nsIIccProvider.idl
@@ -10,5 @@
>  
>  /**
>   * XPCOM component (in the content process) that provides the ICC information.
>   */
>  [scriptable, uuid(2fbacfc4-f52d-11e4-9667-33b72f279d14)]

Require a UUID bump.

@@ -19,5 @@
> -   * RadioInterfaceLayer in the chrome process. Only a content process that has
> -   * the 'mobileconnection' permission is allowed to register.
> -   */
> -  void registerIccMsg(in unsigned long clientId, in nsIIccListener listener);
> -  void unregisterIccMsg(in unsigned long clientId, in nsIIccListener listener);

Should the removing {register|unregister}IccMsg belong to Part 2.1?

::: dom/icc/interfaces/nsIIccService.idl
@@ +10,3 @@
>  interface nsIStkProactiveCmd;
> +interface nsIStkTerminalResponse;
> +interface nsIStkTimer;

Don't need this forward declaration for nsIStkTimer.

@@ +458,5 @@
> +  /**
> +   * Send envelope to notify the expiration of a requested timer.
> +   *
> +   * @param aTimer
> +   *        nsIStkTimer with timer id and real used seconds when expired.

Please update the comments.

@@ +467,5 @@
> +  /**
> +   * Send "Event Download" envelope to the ICC.
> +   *
> +   * @param nsIStkDownloadEvent
> +   *        The event that ICC listening to in STK_CMD_SET_UP_EVENT_LIST.

@param aEvent ....

::: dom/icc/interfaces/nsIStkCmdFactory.idl
@@ +22,5 @@
>  interface nsIStkCmdFactory : nsISupports
>  {
>    /**
>     * @param  aCommandDetails
> +   *         A JS object which complies with 'dictionary MozStkCommand'

Move this change to Part 1.

@@ +31,5 @@
>    nsIStkProactiveCmd createCommand(in jsval aCommandDetails);
>  
>    /**
> +   * @param  aStkProactiveCmd
> +   *         a nsIStkProactiveCmd instance.

Ditto.

@@ +36,2 @@
>     *
> +   * @return a JS object which complies with 'dictionary MozStkCommand'

Ditto.

@@ +63,5 @@
> +   *        in MozStkCommandEvent.webidl
> +   *
> +   * @return a nsIStkDownloadEvent instance.
> +   */
> +  nsIStkDownloadEvent createDownloadEvent(in jsval aEventMessage);

Since we have |createEventMessage| below, just follow the same naming rule, s/createDownloadEvent/createEvent/.

@@ +81,5 @@
> +   *        in MozStkCommandEvent.webidl
> +   *
> +   * Note: This API is specific to nsIIcc::sendStkTimerExpiration().
> +   */
> +   nsIStkTimer createStkTimer(in jsval aStkTimerMessage);

Since other function doesn't contain `Stk`, let's just follow the same rule, s/createStkTimer/createTimer/.

::: dom/icc/interfaces/nsIStkProactiveCmd.idl
@@ +825,5 @@
> +{
> +  /**
> +   * Indicate current service state of the MS with one of the values listed
> +   * below:
> +   *  - STK_SERVICE_STATE_NORMAL

Define consts for STK_SERVICE_STATE_*?

@@ +898,5 @@
> +  /**
> +   * The browser termination cause.
> +   * It shall be one of following:
> +   * - STK_BROWSER_TERMINATION_CAUSE_USER
> +   * - STK_BROWSER_TERMINATION_CAUSE_ERROR

Define consts for STK_BROWSER_TERMINATION_CAUSE_*?
Attachment #8606869 - Flags: review?(echen)
Attachment #8606871 - Flags: review?(echen) → review+
Comment on attachment 8606873 [details] [diff] [review]
(v2) Part 3.1: (IPC) IPDL Declaration.

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

Looks good, thank you.

::: dom/icc/ipc/PIcc.ipdl
@@ +89,5 @@
>  
> +  /**
> +   * Notify STK proactive command issue by selected UICC.
> +   */
> +  NotifyStkCommand(nsString aStkProactiveCmd);

Add some comments about aStkProactiveCmd is json format.

@@ +110,5 @@
>  
>    /**
> +   * Send STK response to the selected UICC.
> +   */
> +  StkResponse(nsString aCommand, nsString aResponse);

Ditto.

@@ +125,5 @@
> +
> +  /**
> +   * Send STK Event Download to the selected UICC.
> +   */
> +  StkEventDownload(nsString aEvent);

Ditto.
Attachment #8606873 - Flags: review?(echen) → review+
Comment on attachment 8606874 [details] [diff] [review]
(v2) Part 3.2: (IPC) Add Helper to deflate/inflate XPCOM object to/from JSON for IPC.

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

::: dom/icc/gonk/StkCmdFactory.js
@@ +1153,5 @@
>      return;
>    }
>  
>    if (localInfo.date) {
> +    if (localInfo.date.getTime) {

What about `if (localInfo.date instanceof Date) {`?
Attachment #8606874 - Flags: review?(echen) → review+
Comment on attachment 8606875 [details] [diff] [review]
(v2) Part 3.3: (IPC) IPDL Implementation.

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

r=me with comments addressed, thank you.

::: dom/icc/ipc/IccChild.cpp
@@ +322,5 @@
> +  nsCOMPtr<nsIStkCmdFactory> cmdFactory =
> +    do_GetService(ICC_STK_CMD_FACTORY_CONTRACTID);
> +  NS_ENSURE_TRUE(cmdFactory, NS_ERROR_FAILURE);
> +
> +  nsString cmd, response;

Use nsAutoString which supports stack-based string allocation.

@@ +356,5 @@
> +  nsCOMPtr<nsIStkCmdFactory> cmdFactory =
> +    do_GetService(ICC_STK_CMD_FACTORY_CONTRACTID);
> +  NS_ENSURE_TRUE(cmdFactory, NS_ERROR_FAILURE);
> +
> +  nsString event;

nsAutoString

::: dom/icc/ipc/IccParent.cpp
@@ +228,5 @@
> +  nsCOMPtr<nsIStkCmdFactory> cmdFactory =
> +    do_GetService(ICC_STK_CMD_FACTORY_CONTRACTID);
> +  NS_ENSURE_TRUE(cmdFactory, NS_ERROR_UNEXPECTED);
> +
> +  nsString cmd;

nsAutoString
Attachment #8606875 - Flags: review?(echen) → review+
Attachment #8606876 - Flags: review?(echen) → review+
Address comment 22 to handle both system message & stk event in IccService.

Hi Edgar,

May I have your review again?

Thanks!
Attachment #8606863 - Attachment is obsolete: true
Attachment #8609984 - Flags: review?(echen)
1. Define STK related constants (STK_SERVICE_*, STK_BROWSER_TERMINATION_CAUSE_*, RESULT_*) in nsIStkProactiveCmd.idl.
2. Define maps to create Events accordingly.
3. Naming.
Attachment #8606869 - Attachment is obsolete: true
Attachment #8609986 - Flags: review?(echen)
rebased for v3.
Attachment #8606871 - Attachment is obsolete: true
Attachment #8609988 - Flags: review+
address comment 26.
Attachment #8606873 - Attachment is obsolete: true
Attachment #8609989 - Flags: review+
Attachment #8609988 - Attachment description: (v3) Part 2.3: (Gonk) Remove Implementation from IccProvider. → (v3) Part 2.3: (Gonk) Remove Implementation from IccProvider. r=echen
Attachment #8609989 - Attachment description: (v3) Part 3.1: (IPC) IPDL Declaration. → (v3) Part 3.1: (IPC) IPDL Declaration. r=echen
address comment 28.
Attachment #8606875 - Attachment is obsolete: true
Attachment #8609992 - Flags: review+
Attachment #8606876 - Flags: review?(htsai) → review+
Comment on attachment 8609984 [details] [diff] [review]
(v3) Part 2.1: (Gonk) Refactor Stk Proactive Commands from IccProvider to IccService.

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

Nice. Thank you.
Attachment #8609984 - Flags: review?(echen) → review+
Comment on attachment 8609986 [details] [diff] [review]
(v3) Part 2.2: (Gonk) Refactor Stk Request APIs from IccProvider to IccService.

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

r=me with comments addressed, thank you.

::: dom/icc/interfaces/nsIIccService.idl
@@ +459,5 @@
> +   *
> +   * @param aTimerId
> +   *        The TimerId provided from previous proactive command.
> +   * @param aTimerValue
> +   *        real used seconds when expired.

Nit: s/real/Real/

::: dom/icc/interfaces/nsIStkCmdFactory.idl
@@ +39,5 @@
>     */
>    jsval createCommandMessage(in nsIStkProactiveCmd aStkProactiveCmd);
> +
> +  /**
> +   * @param aResponse

s/aResponse/aResponseMessage/

@@ +78,5 @@
> +  /**
> +   * @param aStkTimerMessage
> +   *        a JS object which complies with 'dictionary MozStkTimer'
> +   *        in MozStkCommandEvent.webidl
> +   *

Add documentation about return value. e.g. @return A nsIStkTimer instance.
Attachment #8609986 - Flags: review?(echen) → review+
update final patch.
Attachment #8609982 - Attachment is obsolete: true
Attachment #8611632 - Flags: review+
update final patch.
Attachment #8609988 - Attachment is obsolete: true
Attachment #8611635 - Flags: review+
update final patch.
Attachment #8609989 - Attachment is obsolete: true
Attachment #8611636 - Flags: review+
update final patch.
Attachment #8609992 - Attachment is obsolete: true
Attachment #8611638 - Flags: review+
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #46)
> try server is green:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9e9792991cc

You are our hero!
\o/
We are getting reports of RIL breakage on Open C builds, as documented on https://bugzilla.frenchmozilla.org/show_bug.cgi?id=659#c4. Specifically, looking at RIL logcat output,

> 06-01 18:30:37.718   298   298 E GeckoConsole: [JavaScript Error: "TypeError: icc.iccInfo is null" {file: "jar:file:///system/b2g/omni.ja!/components/IccService.js" line: 142}]
> 06-01 18:30:37.718   298   298 E GeckoConsole: [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "icc.iccInfo is null" {file: "jar:file:///system/b2g/omni.ja!/components/IccService.js" line: 142}]'[JavaScript Error: "icc.iccInfo is null" {file: "jar:file:///system/b2g/omni.ja!/components/IccService.js" line: 142}]' when calling method: [nsIGonkIccService::notifyStkCommand]" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 1068}]
Flags: needinfo?(htsai)
Flags: needinfo?(btseng)
(In reply to Alexandre LISSY :gerard-majax from comment #51)
> We are getting reports of RIL breakage on Open C builds, as documented on
> https://bugzilla.frenchmozilla.org/show_bug.cgi?id=659#c4. Specifically,
> looking at RIL logcat output,
> 
> > 06-01 18:30:37.718   298   298 E GeckoConsole: [JavaScript Error: "TypeError: icc.iccInfo is null" {file: "jar:file:///system/b2g/omni.ja!/components/IccService.js" line: 142}]
> > 06-01 18:30:37.718   298   298 E GeckoConsole: [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "icc.iccInfo is null" {file: "jar:file:///system/b2g/omni.ja!/components/IccService.js" line: 142}]'[JavaScript Error: "icc.iccInfo is null" {file: "jar:file:///system/b2g/omni.ja!/components/IccService.js" line: 142}]' when calling method: [nsIGonkIccService::notifyStkCommand]" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 1068}]

I'll look into this and feedback you soon.

Keep NI on me.
Flags: needinfo?(htsai)
I can reproduce this locally with the same symptom.
However, the problem that cause SIM card not detected is not related to the error message mentioned in comment 51. (It seems that rild will still report STK proactive command even the bootup sequence in ril_worker is pending.)

Actually, we never get any responses from rild after 1st RIL request 'setRadioEnabled' is sent, and there is no further SIM_STATUS_CHANGED reported from rild for ril_worker to fetch ICC related information in advance.

So far, I have no clue to prove that the patches of this bug will cause this problem.

I'll spend more time to narrow down the problem, so keep NI on me.
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #53)
> I can reproduce this locally with the same symptom.
> However, the problem that cause SIM card not detected is not related to the
> error message mentioned in comment 51. (It seems that rild will still report
> STK proactive command even the bootup sequence in ril_worker is pending.)
> 
> Actually, we never get any responses from rild after 1st RIL request
> 'setRadioEnabled' is sent, and there is no further SIM_STATUS_CHANGED
> reported from rild for ril_worker to fetch ICC related information in
> advance.
> 
> So far, I have no clue to prove that the patches of this bug will cause this
> problem.
> 
> I'll spend more time to narrow down the problem, so keep NI on me.

Not a regression of this bug, because I can reproduce this problem with open_c with reversion 245775:14f0933c537c in m-c and there are 300+ change sets before this bug landed.

Note: This symptom seems only happened when device 1st boot-up and will be recovered if you stop/start b2g.

Clear NI and track this symptom in bug 1170467 instead.
Flags: needinfo?(btseng)
Depends on: 1172873
You need to log in before you can comment on or make changes to this bug.