937485 - WebIccManager API: Use Webidl enum for cardState

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Edgar Chen [:edgar]]

+++ This bug was initially created as a follow-up of Bug #814637 +++

Maybe we could have a WebIDL enum for cardState.
(Please see bug 814637 comment #57)

Comment 1

[Ken Chang[:kenkai]]

Move this bug in backlog.

Comment 2

[Anshul]

I would request to also expose these cardStates through IPDL so other RIL implementations can use these too rather than relying on strings.

Comment 3

[Edgar Chen [:edgar]]

(In reply to Anshul from comment #2)
> I would request to also expose these cardStates through IPDL so other RIL
> implementations can use these too rather than relying on strings.

Hi Anshul:

I am not sure if I get your idea about exposing cardStates through IPDL. After moving to IPDL, other RIL implementations only needs to implements nsIFooGonkService which is an idl interface. They don't have to touch IPDL protocol. So do you mean expose cardStates through nsIFooFonkService? Could you help to elaborate more? 

Thank you.

Comment 4

[Anshul]

Yes I meant exposing the cardStates through whichever interface would be used by RadioIntearfaceLayer.js

Comment 5

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

Attachment: WIP

Just found "illegal" state is still necessary because of bug 916000. However, that's pretty strange because we don't have such card state in AOSP ril.h and the negative value for CARD_APPSTATE_ILLEGAL is just not a good idea.

Comment 6

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

Attachment: patch

This patch re-types WebIDL attribute MozIcc::cardState from DOMString? to a new enum type IccCardState?. In the internal API nsIIccProvider, several new constant numeric attributes named CARD_STATE_* are created, and nsIIccProvider::getCardState returns one of them.

In details:

1) All originally listed as possible values of MozIcc::cardState are included in enum IccCardState. Besides, six additional missed values are added back. They are:

  "networkSubsetLocked"
  "personalizationLocked"
  "networkSubsetPukRequired"
  "personalizationPukRequired"
  "ruimPersonalizationLocked"
  "ruimPersonalizationPukRequired"

2) In XPIDL nsIIccProvider, a CARD_STATE_* entry is added for each WebIDL IccCardState value. They should be kept in the same order, and this is static_asserted in dom/icc/src/Icc.cpp.

An additional entry CARD_STATE_UNDETECTED is added to represent card absent state.

3) nsIIccProvider::getCardState() now returns one of the nsIIccProvider::CARD_STATE_* values. All other implementations of this interface do not have to return strings any more.

4) In ril_consts.js, we still define GECKO_CARDSTATE_* but they are now assigned to the corresponding numeric value of nsIIccProvider::CARD_STATE_*. These variables are supposed to be reference only in ril_worker.js.

5) A missed PERSONSUBSTATE attribute is also added back.

6) About the "illegal" state, there is actually no such app state CARD_APPSTATE_ILLEGAL(-1) defined in AOSP RILv6. It was never had been referenced until bug 916000. Even with those have been done in bug 916000, there is still no guarantee that MozRIL has the support for owed pay cards because the modem may never return CARD_APPSTATE_ILLEGAL(-1). Anyway, we need(?) that value exposed in WebIDL interface, and we need a corresponding value in nsIIccProvider. Until we have more detailed information about how the modem deals with owed pay cards, no much we can do here.

Comment 7

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #6)
>   "personalizationLocked"
>   "personalizationPukRequired"

These twon entries should really be "simPersonalizationLocked" and "simPersonalizationPukRequired" to keep backward compatibility. Please consider that is done.

Comment 8

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #6)
> 6) About the "illegal" state, there is actually no such app state
> CARD_APPSTATE_ILLEGAL(-1) defined in AOSP RILv6. It was never had been
> referenced until bug 916000.

And it was first introduced in bug 728886, if anyone wants to know. :)

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

(I've had a bit reviewing overload, so moving review to baku)

Comment 10

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

Attachment: patch : v2

With comment 7 addressed. Local xpcshell and marionette tests passed but try-server is down now. :(

Comment 11

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

(In reply to Olli Pettay [:smaug] from comment #9)
> (I've had a bit reviewing overload, so moving review to baku)

Thank you :)

Comment 12

[Andrea Marchesini [:baku]]

Comment on attachment 8472423 [details] [diff] [review]
patch : v2

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

With this patch the list of values is written 3 times in 3 different places: nsIIccProvider.idl, MozIcc.webidl and ril_consts.js.
I would like to have it just once, but if we cannot, 2 is enough.

I see two approaches:

1. nsIccProvider.idl doesn't have const values and you add a comment such as: |@see IccCardState in MozIcc.webidl for values|

2.remove the list from ril_consts.js and use nsIIccProvider.CARD_STATE_* everywhere.
In ril_consts.js you can do:
PERSONSUBSTATE[CARD_PERSOSUBSTATE_something] = nsIIccProvider.CARD_STATE_something

::: dom/webidl/MozIcc.webidl
@@ +83,5 @@
>     * Once the ICC becomes undetectable, cardstatechange event will be notified.
>     * Also, the attribute is set to null and this MozIcc object becomes invalid.
>     * Calling asynchronous functions raises exception then.
>     */
> +  readonly attribute IccCardState? cardState;

why '?'. I think that the default cardState is 'unknown'.

Comment 13

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

(In reply to Andrea Marchesini (:baku) from comment #12)
> Comment on attachment 8472423 [details] [diff] [review]
> patch : v2
> 
> Review of attachment 8472423 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> With this patch the list of values is written 3 times in 3 different places:
> nsIIccProvider.idl, MozIcc.webidl and ril_consts.js.
> I would like to have it just once, but if we cannot, 2 is enough.
> 
> I see two approaches:
> 
> 1. nsIccProvider.idl doesn't have const values and you add a comment such
> as: |@see IccCardState in MozIcc.webidl for values|
> 
> 2.remove the list from ril_consts.js and use nsIIccProvider.CARD_STATE_*
> everywhere.
> In ril_consts.js you can do:
> PERSONSUBSTATE[CARD_PERSOSUBSTATE_something] =
> nsIIccProvider.CARD_STATE_something

Unfortunately that's impossible because ril_worker is a ChromeWorker. It cannot access XPCOM. [1]

> ::: dom/webidl/MozIcc.webidl
> @@ +83,5 @@
> >     * Once the ICC becomes undetectable, cardstatechange event will be notified.
> >     * Also, the attribute is set to null and this MozIcc object becomes invalid.
> >     * Calling asynchronous functions raises exception then.
> >     */
> > +  readonly attribute IccCardState? cardState;
> 
> why '?'. I think that the default cardState is 'unknown'.

No, it's not. "the attribute is set to null and this MozIcc object becomes invalid".

[1]: https://developer.mozilla.org/en-US/docs/Web/API/ChromeWorker

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #13)
> (In reply to Andrea Marchesini (:baku) from comment #12)
> > Comment on attachment 8472423 [details] [diff] [review]
> > patch : v2
> > 
> > Review of attachment 8472423 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > With this patch the list of values is written 3 times in 3 different places:
> > nsIIccProvider.idl, MozIcc.webidl and ril_consts.js.
> > I would like to have it just once, but if we cannot, 2 is enough.
> > 
> > I see two approaches:
> > 
> > 1. nsIccProvider.idl doesn't have const values and you add a comment such
> > as: |@see IccCardState in MozIcc.webidl for values|
> > 

nsIIccProvider.idl is a hook for various backend implementation. I think it's worth to have well-defined constants there.

Comment 15

[Edgar Chen [:edgar]]

Comment on attachment 8472423 [details] [diff] [review]
patch : v2

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

::: dom/system/gonk/ril_consts.js
@@ +2533,5 @@
>  PERSONSUBSTATE[CARD_PERSOSUBSTATE_SIM_NETWORK_PUK] = GECKO_CARDSTATE_NETWORK_PUK_REQUIRED;
>  PERSONSUBSTATE[CARD_PERSOSUBSTATE_SIM_NETWORK_SUBSET_PUK] = GECKO_CARDSTATE_NETWORK_SUBSET_PUK_REQUIRED;
>  PERSONSUBSTATE[CARD_PERSOSUBSTATE_SIM_CORPORATE_PUK] = GECKO_CARDSTATE_CORPORATE_PUK_REQUIRED;
>  PERSONSUBSTATE[CARD_PERSOSUBSTATE_SIM_SERVICE_PROVIDER_PUK] = GECKO_CARDSTATE_SERVICE_PROVIDER_PUK_REQUIRED;
> +PERSONSUBSTATE[CARD_PERSOSUBSTATE_SIM_SIM_PUK] = GECKO_CARDSTATE_SIM_PUK_REQUIRED;

Nice catch!!

::: dom/system/gonk/tests/test_ril_worker_icc_CardLock.js
@@ +101,5 @@
>    }
>  
>    // Test GSM personalization state.
>    testPersonalization(false, CARD_PERSOSUBSTATE_SIM_NETWORK,
> +                      Ci.nsIIccProvider.CARD_STATE_NETWORK_LOCKED);

Why Ci.nsIIccProvider.*? This is a unit test for ril_worker, should we keep using GECKO_CARDSTATE_*?

Comment 16

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

(In reply to Edgar Chen [:edgar][:echen] from comment #15)
> Review of attachment 8472423 [details] [diff] [review]:
> 
> Why Ci.nsIIccProvider.*? This is a unit test for ril_worker, should we keep
> using GECKO_CARDSTATE_*?

Then we won't be notified if somehow GECKO_CARDSTATE_* do not match nsIIccProvider.*.

Comment 17

[Edgar Chen [:edgar]]

Comment on attachment 8472423 [details] [diff] [review]
patch : v2

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #16)
> (In reply to Edgar Chen [:edgar][:echen] from comment #15)
> > Review of attachment 8472423 [details] [diff] [review]:
> > 
> > Why Ci.nsIIccProvider.*? This is a unit test for ril_worker, should we keep
> > using GECKO_CARDSTATE_*?
> 
> Then we won't be notified if somehow GECKO_CARDSTATE_* do not match
> nsIIccProvider.*.

I see, thank you.

Comment 18

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

Comment on attachment 8472423 [details] [diff] [review]
patch : v2

Please help review this.

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8472423 [details] [diff] [review]
patch : v2

Very unfortunate to have the same stuff listed 3 times.

Another option would be to just change the webidl to use enum, and then in
Icc::GetCardState() use IccCardStateValues to map the values from
mProvider->GetCardState to the IccCardState values.

Comment 14 mentions that it is useful to have the consts in .idl, but is that
really that useful? Like the code shows, one needs to anyway copy the list of
possible values in case of a js worker.

If you disagree, re-ask review.

Comment 20

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

(In reply to Olli Pettay [:smaug] from comment #19)
> Comment on attachment 8472423 [details] [diff] [review]
> patch : v2
> 
> Very unfortunate to have the same stuff listed 3 times.
> 
> Another option would be to just change the webidl to use enum, and then in
> Icc::GetCardState() use IccCardStateValues to map the values from
> mProvider->GetCardState to the IccCardState values.
> 
> Comment 14 mentions that it is useful to have the consts in .idl, but is that
> really that useful? Like the code shows, one needs to anyway copy the list of
> possible values in case of a js worker.

Comment 2 is a feedback from third party. They re-implement RIL interfaces in C++, not js worker as we do. As a result, we have the same reason to enumerate possible card states in IDL just like we'd prefer enums in WebIDL. In this patch I'd like to eliminate passing strings as suggested in comment 2. In this way we may consolidate common parts of two or more implementations and ease the process to digging out the valid values accepted by upper layers.

We need nsIIccProvider::CARD_STATE_* because we can't access numeric IccCardState values in chrome js components. We need GECKO_CARD_STATE in ril_worker because we can't access XPCOM in ChromeWorker.

> If you disagree, re-ask review.

Comment 21

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

To be landed after Firefox OS 2.1 branches out.

Comment 22

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

https://tbpl.mozilla.org/?tree=Try&rev=9fedbcbe303a

Comment 23

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

Attachment: patch : v3

Fix debug build -- explicitly cast IccCardState to uint32_t when necessary. Also moved assertions to a new Assertions.cpp file because we may have more in bug 1052852, bug 1052825, and bug 864489.

Comment 24

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

Attachment: patch : v4

Rebase after bug 843452.

Comment 25

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

https://hg.mozilla.org/integration/b2g-inbound/rev/e6f223a82d91

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e6f223a82d91

Comment 27

[:gerard-majax]

This is causing very serious bustage: lots of people have their device not booting at all anymore.

I'm getting XPCOMUtils throwing on defineLazyGetterService:
> 09-05 09:15:16.230  1602  1602 I Gecko   : defineLazyServiceGetter: aObject=[object FakeBackstagePass] aName=icc aContract=@mozilla.org/ril/content-helper;1 aInterfaceName=nsIIccProvider

Comment 28

[Carsten Book [:Tomcat]]

backedout from m-c and integration trees for the regressions mentioned in #27 and also by request from Alexandre and Smaug

Comment 29

[:gerard-majax]

FYI, the error comes from:

> 09-05 11:39:52.628  1684  1684 I Gecko   : defineLazyServiceGetter: aObject=[object FakeBackstagePass] aName=icc aContract=@mozilla.org/ril/content-helper;1 aInterfaceName=nsIIccProvider
> 09-05 11:39:52.628  1684  1684 I Gecko   : XPCU_serviceLambda@resource://gre/modules/XPCOMUtils.jsm:222:15
> 09-05 11:39:52.628  1684  1684 I Gecko   : XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:193:33
> 09-05 11:39:52.628  1684  1684 I Gecko   : getCountryName@resource://gre/modules/PhoneNumberUtils.jsm:63:9
> 09-05 11:39:52.628  1684  1684 I Gecko   : this.ContactService.configureSubstringMatching@resource://gre/modules/ContactService.jsm:73:23
> 09-05 11:39:52.628  1684  1684 I Gecko   : this.ContactService.init@resource://gre/modules/ContactService.jsm:47:5
> 09-05 11:39:52.628  1684  1684 I Gecko   : @resource://gre/modules/ContactService.jsm:265:1
> 09-05 11:39:52.628  1684  1684 I Gecko   : @chrome://b2g/content/shell.js:7:1

Comment 30

[Chris Lord [:cwiiis]]

For those whose phones were affected by this bug (and who weren't clever enough to be making backups between flashes...), is there a way to recover from this?

Comment 31

[Chris Lord [:cwiiis]]

(In reply to Chris Lord [:cwiiis] from comment #30)
> For those whose phones were affected by this bug (and who weren't clever
> enough to be making backups between flashes...), is there a way to recover
> from this?

To answer myself, just flashing a rebuilt Gecko is enough - I had to do a clobber build for it to take though.

Comment 32

[Matěj Cepl]

(In reply to Chris Lord [:cwiiis] from comment #30)
> For those whose phones were affected by this bug (and who weren't clever
> enough to be making backups between flashes...), is there a way to recover
> from this?

I have flashed (using https://github.com/Mozilla-TWQA/B2G-flash-tool/blob/master/shallow_flash.sh; see https://developer.mozilla.org/en-US/Firefox_OS/Developer_phone_guide/Flame for more info) all the way to http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/2014-08-31-16-02-03-mozilla-central-flame/ (I haven't been much happy even with previous, albeit working, updates).

Comment 33

[Jason Smith [:jsmith]]

(In reply to Carsten Book [:Tomcat] from comment #28)
> backedout from m-c and integration trees for the regressions mentioned in
> #27 and also by request from Alexandre and Smaug

Backout changeset is here btw:

http://hg.mozilla.org/mozilla-central/rev/dddbe46f3ceb

Comment 34

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

Attachment: setringtone-no-manifest.log

It seems now we have another problem due to Gaia setringtone app. Webapps.jsm keeps throwing errors because it cannot find manifest for setringtone app.

Comment 35

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #34)
> Created attachment 8495222 [details]
> setringtone-no-manifest.log
> 
> It seems now we have another problem due to Gaia setringtone app.
> Webapps.jsm keeps throwing errors because it cannot find manifest for
> setringtone app.

Work-around: Return if |aManifest| is null in http://mxr.mozilla.org/mozilla-central/source/dom/apps/Webapps.jsm#715 .

Comment 36

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

(In reply to Alexandre LISSY :gerard-majax from comment #29)
> FYI, the error comes from:
> 
> > 09-05 11:39:52.628  1684  1684 I Gecko   : defineLazyServiceGetter: aObject=[object FakeBackstagePass] aName=icc aContract=@mozilla.org/ril/content-helper;1 aInterfaceName=nsIIccProvider
> > 09-05 11:39:52.628  1684  1684 I Gecko   : XPCU_serviceLambda@resource://gre/modules/XPCOMUtils.jsm:222:15
> > 09-05 11:39:52.628  1684  1684 I Gecko   : XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:193:33
> > 09-05 11:39:52.628  1684  1684 I Gecko   : getCountryName@resource://gre/modules/PhoneNumberUtils.jsm:63:9
> > 09-05 11:39:52.628  1684  1684 I Gecko   : this.ContactService.configureSubstringMatching@resource://gre/modules/ContactService.jsm:73:23
> > 09-05 11:39:52.628  1684  1684 I Gecko   : this.ContactService.init@resource://gre/modules/ContactService.jsm:47:5
> > 09-05 11:39:52.628  1684  1684 I Gecko   : @resource://gre/modules/ContactService.jsm:265:1
> > 09-05 11:39:52.628  1684  1684 I Gecko   : @chrome://b2g/content/shell.js:7:1

I did not ever have these. From all the logs I captured, Gecko just hangs there.

Now with work-around for that Gaia setringtone problem in comment 35, I can no longer reproduce the bustage. All I do is:

  # cherry-pick the backed-out commit [1] to Flame gecko tree
  # apply work-around in comment 35
  # rebuild Gecko
  $ (cd ${ROM_FILES_DIR}; ./flash.sh)
  $ ./flash.sh gecko
  $ ./flash.sh gaia

[1]: http://git.mozilla.org/?p=releases/gecko.git;a=commit;h=0284125420fd51551b8f51212bfe319edebf2764

Comment 37

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

Attachment: 0001-fix-setringtone.patch

Comment 38

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #34)
> It seems now we have another problem due to Gaia setringtone app.
> Webapps.jsm keeps throwing errors because it cannot find manifest for
> setringtone app.

Filed bug 1072929 for this.

Comment 39

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #36)
> (In reply to Alexandre LISSY :gerard-majax from comment #29)
> > FYI, the error comes from:
> > 
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : defineLazyServiceGetter: aObject=[object FakeBackstagePass] aName=icc aContract=@mozilla.org/ril/content-helper;1 aInterfaceName=nsIIccProvider
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : XPCU_serviceLambda@resource://gre/modules/XPCOMUtils.jsm:222:15
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:193:33
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : getCountryName@resource://gre/modules/PhoneNumberUtils.jsm:63:9
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : this.ContactService.configureSubstringMatching@resource://gre/modules/ContactService.jsm:73:23
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : this.ContactService.init@resource://gre/modules/ContactService.jsm:47:5
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : @resource://gre/modules/ContactService.jsm:265:1
> > > 09-05 11:39:52.628  1684  1684 I Gecko   : @chrome://b2g/content/shell.js:7:1
> 
> I did not ever have these. From all the logs I captured, Gecko just hangs
> there.

Alexandre is right.

RILContentHelper has been instantiated previously. Probably the reason we have a existing mServiceObject pointer. When the lazy getter is reached in PhoneNumberutils.jsm, in nsComponentManagerImpl::GetService() line 1312 [1], xpcom returns the result of QueryInterface of the service object. However, that result is an error, so nsJSCID::GetService() returns NS_ERROR_XPC_GS_RETURNED_FAILURE in line 698 [2].

But that |QueryInterface| method was generated by XPCOMUtils. In a successful Gecko upgrade we have:

  I/Gecko   (  291): -*- PhoneNumberutils: begin lazy getter
  I/VICAMO  (  291): nsComponentManagerImpl::GetService - entry->mServiceObject
  I/Gecko   (  291): -*- RILContentHelper: QueryInterface: {7c67ab92-52a3-4e11-995c-c0ad2f66c4cb}
  I/Gecko   (  291): -*- RILContentHelper: QueryInterface: {00000000-0000-0000-c000-000000000046}
  I/Gecko   (  291): -*- RILContentHelper: QueryInterface: {986c11d0-f340-11d4-9075-0010a4e73d9a}
  I/Gecko   (  291): -*- PhoneNumberutils: end lazy getter

In a failed upgrade we get:

  I/Gecko   (  291): -*- PhoneNumberutils: begin lazy getter
  I/VICAMO  (  291): nsComponentManagerImpl::GetService - entry->mServiceObject
  I/Gecko   (  291): -*- PhoneNumberutils: exception thrown

Somehow the RILContentHelper instance was replaced??

[1]: http://mxr.mozilla.org/mozilla-central/source/xpcom/components/nsComponentManager.cpp#1312
[2]: http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCJSID.cpp#698

Comment 40

[Edgar Chen [:edgar]]

Comment on attachment 8483905 [details] [diff] [review]
patch : v4

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1872,5 @@
>    };
>    aWorkerMessenger.registerClient(aClientId, this);
>  
>    this.rilContext = {
> +    cardState:      Ci.nsIIccProvider.CARD_STATE_UNKNOWN,

Just found we missed modifying the type of |cardState| in nsIRiLContext [1].
After correcting the type to "unsigned long", device boots up successful with base image v123 + flashing latest gecko and gaia.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/nsIRadioInterfaceLayer.idl#29

Comment 41

[Edgar Chen [:edgar]]

If you don't mind ... :)

Comment 42

[Edgar Chen [:edgar]]

Attachment: Patch, v5

- Rebase
- Address comment #40

Comment 43

[Edgar Chen [:edgar]]

try result: https://tbpl.mozilla.org/?tree=Try&rev=2ab5115e6b2d

Comment 44

[Edgar Chen [:edgar]]

https://hg.mozilla.org/integration/b2g-inbound/rev/957c83999c91

Comment 45

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

https://hg.mozilla.org/mozilla-central/rev/957c83999c91

Comment 46

[Hsin-Yi Tsai (she/her) [:hsinyi]]

\o/