Closed Bug 814629 Opened 7 years ago Closed 6 years ago

[DSDS] WebMobileConnection API: support multiple sim cards

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
1.3 Sprint 5 - 11/22
blocking-b2g 1.3+
Tracking Status
firefox28 --- fixed

People

(Reporter: hsinyi, Assigned: jessica)

References

Details

(Keywords: dev-doc-needed, Whiteboard: [FT:RIL])

Attachments

(5 files, 12 obsolete files)

2.36 KB, patch
hsinyi
: review+
Details | Diff | Splinter Review
8.54 KB, patch
khuey
: review+
Details | Diff | Splinter Review
3.00 KB, patch
jessica
: review+
Details | Diff | Splinter Review
11.02 KB, patch
jessica
: review+
Details | Diff | Splinter Review
14.68 KB, patch
jessica
: review+
Details | Diff | Splinter Review
Currently B2G supports a single SIM architecture. We use navigator.mozMobileConnection to talk to the sim. In multi-SIMs configuration, there shall be multiple mobileconnection objects in each content process. Each of those objects communicates with an individual sim card. We will also need to enhance the existing API to let content access different sim cards.
Assignee: nobody → kchang
blocking-b2g: --- → leo?
Blocking-, no longer targeting Leo release. Product will retarget for future release.
blocking-b2g: leo? → -
Assignee: kchang → echen
Depends on: 898445
Depends on: 818353
No longer depends on: 814584
No longer depends on: 898445
Assignee: echen → jjong
Blocks: 918553
Blocks: 921980
Blocks: 921991
Blocks: 926169
A reminder: 

As we are here touching mobileconnection API and RIL implementation, it looks a good chance for us to correct inconsistent coding style and namings. Please help rename option to options. In addition to the specific
ones in [1] & [2], we should take care of other methods *Option(), e.g. setCallWaitingOption.

[1]:https://bugzilla.mozilla.org/show_bug.cgi?id=818393#c43
[2]:https://bugzilla.mozilla.org/show_bug.cgi?id=818393#c44
Renaming to Options should be done in another bug since it also involves Gaia change.
Depends on: 926302
Attachment #817080 - Flags: feedback?(htsai)
Attachment #817080 - Flags: feedback?(echen)
Attachment #817081 - Flags: feedback?(htsai)
Attachment #817081 - Flags: feedback?(echen)
Attached patch Part 3: MobileConnection (dom) (obsolete) — Splinter Review
Attachment #817082 - Flags: feedback?(htsai)
Attachment #817082 - Flags: feedback?(echen)
Attached patch Part 4: Navigator (obsolete) — Splinter Review
Attachment #817084 - Flags: feedback?(htsai)
Attachment #817084 - Flags: feedback?(echen)
Attached patch Part 5: test scripts fixes (obsolete) — Splinter Review
Attachment #817085 - Flags: feedback?(htsai)
Attachment #817085 - Flags: feedback?(echen)
Comment on attachment 817080 [details] [diff] [review]
Part 1: MobileConnectionArray webidl

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

The overall structure and API change look good to me.
I would suggest putting MobileConnectionArray.webidl and Navigator.webidl (part 4) together in Part 1 as they both touch web API changes. And moving the rest of implementation files together into another part. Just for a clearer view of the whole picture of what those patches have done. Thank you.

::: dom/network/src/MobileConnectionArray.cpp
@@ +8,5 @@
> +#include "mozilla/dom/MobileConnectionArrayBinding.h"
> +
> +using namespace mozilla::dom::network;
> +
> +NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_1(MobileConnectionArray,

Should cycle collect mWindow as well.

@@ +20,5 @@
> +  NS_INTERFACE_MAP_ENTRY(nsISupports)
> +NS_INTERFACE_MAP_END
> +
> +MobileConnectionArray::MobileConnectionArray(nsPIDOMWindow* aWindow)
> +  : mWindow(aWindow)

nit: indenttion

@@ +87,5 @@
> +  aFound = aIndex < mMobileConnections.Length();
> +
> +  return aFound ? mMobileConnections[aIndex] : nullptr;
> +}
> +

nit: no new line

::: dom/network/src/MobileConnectionArray.h
@@ +7,5 @@
> +#ifndef mozilla_dom_network_MobileConnectionArray_h__
> +#define mozilla_dom_network_MobileConnectionArray_h__
> +
> +#include "nsIDOMMobileConnection.h"
> +#include "MobileConnection.h"

Should be able to use forward declaration.

@@ +18,5 @@
> +
> +class MobileConnectionArray MOZ_FINAL : public nsISupports,
> +                                        public nsWrapperCache
> +{
> +

remove this empty line

@@ +57,5 @@
> +} // namespace dom
> +} // namespace mozilla
> +
> +#endif // mozilla_dom_network_MobileConnectionArray_h__
> +

nit: no new line

::: dom/webidl/moz.build
@@ +215,5 @@
>      'MessagePort.webidl',
>      'MessagePortList.webidl',
>      'MimeType.webidl',
>      'MimeTypeArray.webidl',
> +    'MobileConnectionArray.webidl',

Should be in the condition of |if CONFIG['MOZ_B2G_RIL']|.
Attachment #817080 - Flags: feedback?(htsai) → feedback+
Attachment #817081 - Flags: feedback?(htsai) → feedback+
Comment on attachment 817082 [details] [diff] [review]
Part 3: MobileConnection (dom)

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

::: dom/network/src/MobileConnection.cpp
@@ +77,5 @@
>  NS_IMPL_EVENT_HANDLER(MobileConnection, cfstatechange)
>  NS_IMPL_EVENT_HANDLER(MobileConnection, emergencycbmodechange)
>  NS_IMPL_EVENT_HANDLER(MobileConnection, otastatuschange)
>  
> +MobileConnection::MobileConnection(uint32_t clientId)

: mClientId(clientId)

::: dom/network/src/MobileConnection.h
@@ +34,5 @@
>    NS_DECL_NSIMOBILECONNECTIONLISTENER
>  
>    NS_REALLY_FORWARD_NSIDOMEVENTTARGET(nsDOMEventTargetHelper)
>  
> +  MobileConnection(uint32_t subscriptionId);

clientId
Attachment #817082 - Flags: feedback?(htsai) → feedback+
Comment on attachment 817084 [details] [diff] [review]
Part 4: Navigator

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

::: dom/webidl/Navigator.webidl
@@ +259,5 @@
>    [Throws, Func="Navigator::HasTelephonySupport"]
>    readonly attribute Telephony? mozTelephony;
>  };
>  
>  // nsIMozNavigatorMobileConnection

This isn't valid now. Could be removed.
Attachment #817084 - Flags: feedback?(htsai) → feedback+
Comment on attachment 817085 [details] [diff] [review]
Part 5: test scripts fixes

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

Nice. Thank you.
Attachment #817085 - Flags: feedback?(htsai) → feedback+
Attachment #817081 - Flags: feedback?(echen) → feedback+
Attachment #817082 - Flags: feedback?(echen) → feedback+
Attachment #817085 - Flags: feedback?(echen) → feedback+
Comment on attachment 817080 [details] [diff] [review]
Part 1: MobileConnectionArray webidl

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

Sorry, I afraid I can not provide useful feedback about webidl, so cancel it.
Attachment #817080 - Flags: feedback?(echen)
Comment on attachment 817084 [details] [diff] [review]
Part 4: Navigator

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

Sorry, I afraid I can not provide useful feedback about webidl, so cancel it.
Attachment #817084 - Flags: feedback?(echen)
blocking-b2g: - → 1.3+
Blocks: 921391
Blocks: 921389
Target Milestone: --- → 1.3 Sprint 3 - 10/25
(In reply to Edgar Chen [:edgar][:echen] from comment #16)
> Comment on attachment 817084 [details] [diff] [review]
> Part 4: Navigator
> 
> Review of attachment 817084 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry, I afraid I can not provide useful feedback about webidl, so cancel it.

Thank you anyway for all your help. :)
Address review Comment 11.

Kyle,

For multisim, we are adding a new MozMobileConnectionArray component, which is just an array of MozMobileConnection(s). API users will then use navigator.mozMobileConnections[id] to access the corresponding MozMobileConnection.

May I have your feedback on this? Thanks.
Attachment #817080 - Attachment is obsolete: true
Attachment #819558 - Flags: feedback?(khuey)
Address review comment 12.

Kyle,

These are the corresponding dom changes.
May I have your feedback on this too? Thanks.
Attachment #817082 - Attachment is obsolete: true
Attachment #819559 - Flags: feedback?(khuey)
Address review comments in comment 11 and comment 13.

Kyle,

This is the implementation part.
May I have your feedback on this too? Thanks.
Attachment #817084 - Attachment is obsolete: true
Attachment #819561 - Flags: feedback?(khuey)
Kyle,

Since I touched the test_interfaces.html file, may I have your feedback on this as well? Thanks!
Attachment #817085 - Attachment is obsolete: true
Attachment #819573 - Flags: feedback?(khuey)
Comment on attachment 819559 [details] [diff] [review]
Part 3: dom: MobileConnection changes

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

::: dom/network/src/MobileConnection.cpp
@@ +77,5 @@
>  NS_IMPL_EVENT_HANDLER(MobileConnection, cfstatechange)
>  NS_IMPL_EVENT_HANDLER(MobileConnection, emergencycbmodechange)
>  NS_IMPL_EVENT_HANDLER(MobileConnection, otastatuschange)
>  
> +MobileConnection::MobileConnection(uint32_t clientId)

Gecko style is to name method arguments aFoo, member variables mFoo, static variables sFoo, global variables gFoo, and only local variables have no prefix.  So this should be aClientId.

@@ +188,5 @@
>    return mProvider->GetDataConnectionInfo(mClientId, data);
>  }
>  
>  NS_IMETHODIMP
> +MobileConnection::GetIccId(nsAString& iccId)

Similarly aIccId

::: dom/network/src/MobileConnection.h
@@ +34,5 @@
>    NS_DECL_NSIMOBILECONNECTIONLISTENER
>  
>    NS_REALLY_FORWARD_NSIDOMEVENTTARGET(nsDOMEventTargetHelper)
>  
> +  MobileConnection(uint32_t clientId);

and here too.
Attachment #819559 - Flags: feedback?(khuey) → feedback+
Comment on attachment 819561 [details] [diff] [review]
Part 4: MobileConnectionArray implementation and Navigator changes

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

::: dom/network/src/MobileConnectionArray.cpp
@@ +28,5 @@
> +  SetIsDOMBinding();
> +}
> +
> +void
> +MobileConnectionArray::Init()

Why not just move all of this into the constructor and drop the Init method?

::: dom/network/src/MobileConnectionArray.h
@@ +48,5 @@
> +private:
> +  ~MobileConnectionArray();
> +
> +  nsCOMPtr<nsPIDOMWindow> mWindow;
> +  nsTArray<nsRefPtr<MobileConnection> > mMobileConnections;

You can use >> now, all of our compilers support it.
Attachment #819561 - Flags: feedback?(khuey) → feedback+
Comment on attachment 819558 [details] [diff] [review]
Part 1: webidl: MozMobileConnectionArray and changes for Navigator

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

::: dom/webidl/MozMobileConnectionArray.webidl
@@ +6,5 @@
> +
> +interface MozMobileConnection;
> +
> +interface MozMobileConnectionArray {
> +  getter MozMobileConnection item(unsigned long index);

This actually should be 'MozMobileConnection?' to allow for null.  Otherwise I think mobileConnections[reallyBigNumber] will assert and crash.
Actually, I take that back, the aFound parameter handles that case.  But it's probably possible for this to be null if someone accesses mobileConnections[0] after the navigator calls shutdown on the MobileConnectionArray, so this should still be 'MozMobileConnection?'.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #25)
> Actually, I take that back, the aFound parameter handles that case.  But
> it's probably possible for this to be null if someone accesses
> mobileConnections[0] after the navigator calls shutdown on the
> MobileConnectionArray, so this should still be 'MozMobileConnection?'.

We'd change it to 'MozMobileConnection?' for possible null.
Thanks for all the valuable review comments.
No longer blocks: 921991
Whiteboard: [FT:RIL]
Comment on attachment 819561 [details] [diff] [review]
Part 4: MobileConnectionArray implementation and Navigator changes

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

::: dom/base/Navigator.cpp
@@ -1279,5 @@
>  }
>  
>  #ifdef MOZ_B2G_RIL
> -nsIDOMMozMobileConnection*
> -Navigator::GetMozMobileConnection(ErrorResult& aRv)

We could share the same "#ifdef MOZ_B2G_RIL" with other RIL components.
Could you help to put this together with GetMozTelephony, GetMozVoicemail ... above?
Thank you.
Sure!
I will put it together with GetMozCellBroadcast, GetMozVoicemail and GetMozIccManager. GetMozTelephony and GetMozMobileMessage are not guarded with MOZ_B2G_RIL right now, are they?
(In reply to Jessica Jong [:jjong] [:jessica] from comment #28)
> Sure!
> I will put it together with GetMozCellBroadcast, GetMozVoicemail and
> GetMozIccManager. GetMozTelephony and GetMozMobileMessage are not guarded
> with MOZ_B2G_RIL right now, are they?

You are correct, GetMozTelephony was no longer guarded with MOZ_B2G_RIL (done in bug 920551).
Thanks for this feedback :)
Because Sprint 3 was already passed, should we assign new target milestone? Thanks.
Target Milestone: 1.3 Sprint 3 - 10/25 → 1.3 Sprint 4 - 11/8
No longer blocks: 926169
Depends on: 926169
Comment 24 and 25 addressed: use 'MozMobileConnection?' to allow for null.
Attachment #819558 - Attachment is obsolete: true
"oniccchange" event added.
Attachment #817081 - Attachment is obsolete: true
Attachment #825810 - Flags: review?(htsai)
Added "oniccchange" event and addressed Comment 22:
rename all method arguments to use "aXxx"
Attachment #819559 - Attachment is obsolete: true
Attachment #825814 - Attachment description: Part 3: MobileConnection changes (dom) → Part 3: MobileConnection changes (dom), v3
rebased and added feedback/review info.
Attachment #819573 - Attachment is obsolete: true
Attachment #825809 - Flags: review?(khuey)
Attachment #825814 - Flags: review?(khuey)
Attachment #825815 - Flags: review?(khuey)
Attachment #825816 - Flags: review?(khuey)
Kyle, I have modified the patches based on your comments and added an additional "iccchange" event, which is reported when the iccid that corresponds to the mobile connection changes.
May I have your review? Thanks.
Comment on attachment 825815 [details] [diff] [review]
Part 4: MobileConnectionArray implementation and Navigator changes, v2

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

::: dom/network/src/MobileConnectionArray.cpp
@@ +24,5 @@
> +
> +MobileConnectionArray::MobileConnectionArray(nsPIDOMWindow* aWindow)
> +: mWindow(aWindow)
> +{
> +  int32_t numRil = mozilla::Preferences::GetInt("ril.numRadioInterfaces", 1);

We should probably assert that numRil is >= 0 (or maybe > 0?).
Attachment #825815 - Flags: review?(khuey) → review+
Comment on attachment 825816 [details] [diff] [review]
Part 5: modify mozMobileConnection related tests. f=hsinyi r=khuey (final)

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

I think Hsin-Yi should review this.
Attachment #825816 - Flags: review?(khuey) → review?(htsai)
Attachment #825810 - Flags: review?(htsai) → review+
Comment on attachment 825816 [details] [diff] [review]
Part 5: modify mozMobileConnection related tests. f=hsinyi r=khuey (final)

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

Thank you.
Attachment #825816 - Flags: review?(htsai) → review+
Summary: WebMobileConnection API: support multiple sim cards → [DSDS] WebMobileConnection API: support multiple sim cards
Comment on attachment 825810 [details] [diff] [review]
Part 2: nsIDOMMobileConnection changes (idl). f=edgar r=hsinyi (final)

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

::: dom/network/interfaces/nsIDOMMobileConnection.idl
@@ +72,5 @@
>    /**
> +   * Integrated Circuit Card Identifier of the SIM this
> +   * mobile connection corresponds to.
> +   */
> +  readonly attribute DOMString iccId;

ICCID might not work if there are multiple subscriptions being used on the same card or in the CDMA case (w/o a card).  Perhaps just serviceId or subscriptionId?
(In reply to Michael Schwartz [:m4] from comment #40)
> Comment on attachment 825810 [details] [diff] [review]
> Part 2: nsIDOMMobileConnection (idl), v2
> 
> Review of attachment 825810 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/network/interfaces/nsIDOMMobileConnection.idl
> @@ +72,5 @@
> >    /**
> > +   * Integrated Circuit Card Identifier of the SIM this
> > +   * mobile connection corresponds to.
> > +   */
> > +  readonly attribute DOMString iccId;
> 
> ICCID might not work if there are multiple subscriptions being used on the
> same card or in the CDMA case (w/o a card).  Perhaps just serviceId or
> subscriptionId?

Hi :m4,

Thanks for your insights. :) Let me explain the whole design concept, and hope that makes sense to you.

'iccId' is introduced to have a clear mapping between MobileConnection API and IccManager API. To get more information about the icc, IccManager is the right API to ask for. So if a card has two subscriptions, via IccManager to get IccInfo so that we know which subscription is indeed in use. As the design we chose 'iccId' because it's really the connection with IccManager. For the cdma case, another attribute will be introduced.
Target Milestone: 1.3 Sprint 4 - 11/8 → 1.3 Sprint 5 - 11/22
Hsin-Yi, I feel introducing different fields for CDMA and GSM would land us in the same pickle as with different fields for phone numbers for GSM(MSISDN) and CDMA(MDN) when all the gaia apps that care about phone number needed to change. Are you guys planning to support CDMA with NV mode in future? If not, then can we not use IccId for both GSM and CDMA and better yet just rely on Client/Service IDs.

Do you know how Android solves this problem?
(In reply to Anshul from comment #42)
> Hsin-Yi, I feel introducing different fields for CDMA and GSM would land us
> in the same pickle as with different fields for phone numbers for
> GSM(MSISDN) and CDMA(MDN) when all the gaia apps that care about phone
> number needed to change. Are you guys planning to support CDMA with NV mode
> in future? If not, then can we not use IccId for both GSM and CDMA and
> better yet just rely on Client/Service IDs.
> 

Hi Anshul,

Thanks for the comment; however, I am not very sure I got your whole idea. I am trying to elaborate the API again. Hopefully your question could be answered then. 

First of all, after the patch, we will have an array of mobileConnections. Each mobileConnection object itself is seen as a service/client/subscription from the API perspective. So the index of the array implies the id of each mobileConnection object, i.e. the clientId/serviceId. Is this the same client/service Id that you are defining?

Second, 'iccId' is used to indicate if a card is carried on the object. If we don't care about cdma NV mode at all, then 'iccId' is clear. If we do care about NV mode, then it also distinguishes. I don't think I got your comment on "not using iccId especially if we don't support cdma nv mode."

> Do you know how Android solves this problem?

You meant the Cdma SIM mode and nv mode?
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #43)
> (In reply to Anshul from comment #42)
> > Hsin-Yi, I feel introducing different fields for CDMA and GSM would land us
> > in the same pickle as with different fields for phone numbers for
> > GSM(MSISDN) and CDMA(MDN) when all the gaia apps that care about phone
> > number needed to change. Are you guys planning to support CDMA with NV mode
> > in future? If not, then can we not use IccId for both GSM and CDMA and
> > better yet just rely on Client/Service IDs.
> > 
> 
> Hi Anshul,
> 
> Thanks for the comment; however, I am not very sure I got your whole idea. I
> am trying to elaborate the API again. Hopefully your question could be
> answered then. 
> 
> First of all, after the patch, we will have an array of mobileConnections.
> Each mobileConnection object itself is seen as a service/client/subscription
> from the API perspective. So the index of the array implies the id of each
> mobileConnection object, i.e. the clientId/serviceId. Is this the same
> client/service Id that you are defining?
My question is why use iccid and not service/client id in nsIDOMMobileConnection.idl? Are you trying to cover the case where user swaps the SIM cards?

> 
> Second, 'iccId' is used to indicate if a card is carried on the object. If
Not sure what you mean by "card is carried on the object". Sorry for asking so many questions.

> we don't care about cdma NV mode at all, then 'iccId' is clear. If we do
> care about NV mode, then it also distinguishes. I don't think I got your
> comment on "not using iccId especially if we don't support cdma nv mode."
> 
> > Do you know how Android solves this problem?
> 
> You meant the Cdma SIM mode and nv mode?
(In reply to Anshul from comment #44)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #43)
> > (In reply to Anshul from comment #42)
> > > Hsin-Yi, I feel introducing different fields for CDMA and GSM would land us
> > > in the same pickle as with different fields for phone numbers for
> > > GSM(MSISDN) and CDMA(MDN) when all the gaia apps that care about phone
> > > number needed to change. Are you guys planning to support CDMA with NV mode
> > > in future? If not, then can we not use IccId for both GSM and CDMA and
> > > better yet just rely on Client/Service IDs.
> > > 
> > 
> > Hi Anshul,
> > 
> > Thanks for the comment; however, I am not very sure I got your whole idea. I
> > am trying to elaborate the API again. Hopefully your question could be
> > answered then. 
> > 
> > First of all, after the patch, we will have an array of mobileConnections.
> > Each mobileConnection object itself is seen as a service/client/subscription
> > from the API perspective. So the index of the array implies the id of each
> > mobileConnection object, i.e. the clientId/serviceId. Is this the same
> > client/service Id that you are defining?
> My question is why use iccid and not service/client id in
> nsIDOMMobileConnection.idl? Are you trying to cover the case where user
> swaps the SIM cards?
> 

Yes, we are considering the case user swaps the sim cards or removes a card.

> > 
> > Second, 'iccId' is used to indicate if a card is carried on the object. If
> Not sure what you mean by "card is carried on the object". Sorry for asking
> so many questions.

That said, there could be a sim card inserted. iccId is used to indicate if a card is there.

> 
> > we don't care about cdma NV mode at all, then 'iccId' is clear. If we do
> > care about NV mode, then it also distinguishes. I don't think I got your
> > comment on "not using iccId especially if we don't support cdma nv mode."
> > 
> > > Do you know how Android solves this problem?
> > 
> > You meant the Cdma SIM mode and nv mode?
Attachment #825810 - Attachment description: Part 2: nsIDOMMobileConnection (idl), v2 → Part 2: nsIDOMMobileConnection changes (idl). f=edgar r=hsinyi (final)
Attachment #825814 - Attachment description: Part 3: MobileConnection changes (dom), v3 → Bug 814629 - Part 3: MobileConnection changes (dom). f=hsinyi,edgar r=khuey (final)
Attachment #825814 - Attachment description: Bug 814629 - Part 3: MobileConnection changes (dom). f=hsinyi,edgar r=khuey (final) → Part 3: MobileConnection changes (dom). f=hsinyi,edgar r=khuey (final)
Rebased and addressed review comment 37:
- assert that numRil is > 0
Attachment #825815 - Attachment is obsolete: true
Attachment #831318 - Flags: review+
Attachment #825816 - Attachment description: Part 5: test scripts modifications, v2 → Part 5: modify mozMobileConnection related tests. f=hsinyi r=khuey (final)
modified reviewer.
Attachment #825816 - Attachment is obsolete: true
Attachment #831326 - Flags: review+
Thank you all for the comments and suggestions, we are about to the land the patches as it blocks others' work and the overall testing. We can still discuss about the possibilities for a better outcome if needed.
Keywords: checkin-needed
Blocks: 938440
Blocks: 939110
Blocks: 941476
You need to log in before you can comment on or make changes to this bug.