Closed Bug 818352 Opened 7 years ago Closed 7 years ago

B2G Multi-SIM: voicemail - add client id in nsIRILContentHelper

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 814634

People

(Reporter: hsinyi, Assigned: aknow)

References

Details

Attachments

(2 files)

This bug was originally reported in Bug 814584. This bug focuses on re-factoring 'voicemail' related functions and messages. 

What we probably need to do is:
a) nsIRILContentHelper IDL change: add an attribute 'subscriptionId'
b) DOM changes: assign '0' (default subscriptionId) in every RILContentHelper function call based on a)
c) RIL impl: i) add a property 'subscriptionId' in IPC messages ii) deliver callback to a specific subscriptionId ...
Blocks: 814584
Assignee: nobody → htsai
Attached file Working branch
Assignee: htsai → szchen
Whiteboard: RN5/29
Whiteboard: RN5/29 → RN6/14
I just take a look for the multi-SIM modification on working branch and have a question.

Why we could not simplify have multiple provider (ex: telephonyProvider, MobileConnectionProvider) for multi-sim. Then add a member for 'subscriptionId' in RILContentHelper.

Details:
1. Add 'subscriptionId' in dom interface. <= Must
2. Create multiple providers in dom, each with a specified 'subscriptionId' value.
3. Wrap cpmm message in RILContentHelper. Automatically add 'subscriptionId' based on its own value.
=> then we don't have to add a new argument 'subscriptionId' in all functions.

The proposal assumes that all the functionality of RILContentHelper should be associate with a sim. So for multi-sim, we just have multiple RILContentHelper
Depends on: 888593
No longer depends on: 888593
Comment on attachment 792125 [details] [diff] [review]
WIP: Add clientId for voicemail

Any suggestions?
Attachment #792125 - Flags: feedback?(htsai)
Comment on attachment 792125 [details] [diff] [review]
WIP: Add clientId for voicemail

change to review
Attachment #792125 - Flags: feedback?(htsai) → review?(htsai)
Comment on attachment 792125 [details] [diff] [review]
WIP: Add clientId for voicemail

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

Don't we need to carry 'id' in VoicemailStatus?
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> Comment on attachment 792125 [details] [diff] [review]
> WIP: Add clientId for voicemail
> 
> Review of attachment 792125 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Don't we need to carry 'id' in VoicemailStatus?

Yes. We should.
The modification requires a dom interface change. I think that it is better to handle the issue with API change in Bug 814634.
(In reply to Szu-Yu Chen [:aknow] from comment #7)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> > Comment on attachment 792125 [details] [diff] [review]
> > WIP: Add clientId for voicemail
> > 
> > Review of attachment 792125 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Don't we need to carry 'id' in VoicemailStatus?
> 
> Yes. We should.
> The modification requires a dom interface change. I think that it is better
> to handle the issue with API change in Bug 814634.

I rethink about the two bugs. I think it's better and complete to merge them into one, especially when they both are not so large.

Sorry about the previous bug planning. I've forgotten why I split them up in this way.
Blocks: 814634
Comment on attachment 792125 [details] [diff] [review]
WIP: Add clientId for voicemail

Cancelling r? based on comment 8. Thanks.
Attachment #792125 - Flags: review?(htsai)
Whiteboard: RN6/14
Component: DOM: Device Interfaces → RIL
Product: Core → Boot2Gecko
Depends on: 921422
Summary: B2G Multi-SIM: voicemail - add subscription id in nsIRILContentHelper → B2G Multi-SIM: voicemail - add client id in nsIRILContentHelper
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 814634
You need to log in before you can comment on or make changes to this bug.