We will move network information needed by other modules into a separate interface, say nsINetworkInfo, so we'll need to expose only this interface and not the entire nsINetworkInterface.

Comment on attachment 8609217 [details] [diff] [review] Part 1: interface changes, v1. Edgar, may I have your feedback on the interface changes? Thanks.

(In reply to Jessica Jong [:jjong] [:jessica] from comment #2) > Comment on attachment 8609217 [details] [diff] [review] > Part 1: interface changes, v1. > > Edgar, may I have your feedback on the interface changes? Thanks. While modifying the changes needed in other modules, I realized we may need to change addHostRoute()/removeHostRoute() APIs not to take nsINetworkInterface as parameter, cause 'network-connection-state-changed' event is fired with nsINetworkInfo and not nsINetworkInterface now. How about just using Ci.nsINetworkInfo.NETWORK_TYPE_* as parameter?

(In reply to Jessica Jong [:jjong] [:jessica] from comment #3) > (In reply to Jessica Jong [:jjong] [:jessica] from comment #2) > > Comment on attachment 8609217 [details] [diff] [review] > > Part 1: interface changes, v1. > > > > Edgar, may I have your feedback on the interface changes? Thanks. > > While modifying the changes needed in other modules, I realized we may need > to change addHostRoute()/removeHostRoute() APIs not to take > nsINetworkInterface as parameter, cause 'network-connection-state-changed' > event is fired with nsINetworkInfo and not nsINetworkInterface now. How > about just using Ci.nsINetworkInfo.NETWORK_TYPE_* as parameter? Taking only Ci.nsINetworkInfo.NETWORK_TYPE_* as parameter probably isn't enough. For mobile connection, we use serviceId to distinguish different sim slot.

(In reply to Edgar Chen [:edgar][:echen] from comment #4) > (In reply to Jessica Jong [:jjong] [:jessica] from comment #3) > > (In reply to Jessica Jong [:jjong] [:jessica] from comment #2) > > > Comment on attachment 8609217 [details] [diff] [review] > > > Part 1: interface changes, v1. > > > > > > Edgar, may I have your feedback on the interface changes? Thanks. > > > > While modifying the changes needed in other modules, I realized we may need > > to change addHostRoute()/removeHostRoute() APIs not to take > > nsINetworkInterface as parameter, cause 'network-connection-state-changed' > > event is fired with nsINetworkInfo and not nsINetworkInterface now. How > > about just using Ci.nsINetworkInfo.NETWORK_TYPE_* as parameter? > > Taking only Ci.nsINetworkInfo.NETWORK_TYPE_* as parameter probably isn't > enough. For mobile connection, we use serviceId to distinguish different sim > slot. Oh, you're right. So, I guess we should use nsINetworkInfo as parameter for addHostRoute()/removeHostRoute()?

(In reply to Jessica Jong [:jjong] [:jessica] from comment #5) > Oh, you're right. So, I guess we should use nsINetworkInfo as parameter for > addHostRoute()/removeHostRoute()? Yes. There seems no other better idea.

Added changes for addHostRoute()/removeHostRoute().

Comment on attachment 8610418 [details] [diff] [review] Part 1: interface changes, v2. Review of attachment 8610418 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thank you. ::: dom/system/gonk/nsIDataCallManager.idl @@ +7,2 @@ > > +interface nsINetworkInfo; Don't need this forward declaration.

[Tracking Requested - why for this release]:

Bug 1167132 - Part 1: [NetworkManager] move network information into a separate interface (idl). r=echen

Bug 1167132 - Part 2: [NetworkManager] move network information into a separate interface (NetworkManager)

Bug 1167132 - Part 3: [NetworkManager] move network information into a separate interface (Wifi)

Bug 1167132 - Part 4: [NetworkManager] move network information into a separate interface (DataCall)

Bug 1167132 - Part 5: [NetworkManager] move network information into a separate interface (Tethering)

Bug 1167132 - Part 6: [NetworkManager] move network information into a separate interface (RadioInterface)

Bug 1167132 - Part 7: [NetworkManager] move network information into a separate interface (MobileConnectionService)

Bug 1167132 - Part 8: [NetworkManager] move network information into a separate interface (MmsService)

Bug 1167132 - Part 9: [NetworkManager] move network information into a separate interface (shell)

Bug 1167132 - Part 10: [NetworkManager] move network information into a separate interface (PushService)

Bug 1167132 - Part 11: [NetworkManager] move network information into a separate interface (Gps)

Bug 1167132 - Part 12: [NetworkManager] move network information into a separate interface (NetStatistics)

Bug 1167132 - Part 13: [NetworkManager] move network information into a separate interface (discovery)

Bug 1167132 - Part 14: [NetworkManager] move network information into a separate interface (NetworkStats)

Bug 1167132 - Part 15: [NetworkManager] move network information into a separate interface (Mtransport)

Comment on attachment 8620779 [details] MozReview Request: Bug 1167132 - Part 15: [NetworkManager] move network information into a separate interface (Mtransport) Bug 1167132 - Part 15: [NetworkManager] move network information into a separate interface (Mtransport)

Ufff, first time using mozreview. I am not sure why my patches were obsoleted automatically, I'll ask for review again. Sorry if you are getting lots of notifications.

Comment on attachment 8620764 [details] MozReview Request: Bug 1167132 - Part 1: [NetworkManager] move network information into a separate interface (idl). r?echen Edgar, may I have your review on this? Thanks.

Comment on attachment 8620764 [details] MozReview Request: Bug 1167132 - Part 1: [NetworkManager] move network information into a separate interface (idl). r?echen Bug 1167132 - Part 1: [NetworkManager] move network information into a separate interface (idl). r?echen

Comment on attachment 8620766 [details] MozReview Request: Bug 1167132 - Part 2: [NetworkManager] move network information into a separate interface (NetworkManager) Bug 1167132 - Part 2: [NetworkManager] move network information into a separate interface (NetworkManager)

Comment on attachment 8620767 [details] MozReview Request: Bug 1167132 - Part 3: [NetworkManager] move network information into a separate interface (Wifi) Bug 1167132 - Part 3: [NetworkManager] move network information into a separate interface (Wifi)

Comment on attachment 8620768 [details] MozReview Request: Bug 1167132 - Part 4: [NetworkManager] move network information into a separate interface (DataCall) Bug 1167132 - Part 4: [NetworkManager] move network information into a separate interface (DataCall)

Comment on attachment 8620769 [details] MozReview Request: Bug 1167132 - Part 5: [NetworkManager] move network information into a separate interface (Tethering) Bug 1167132 - Part 5: [NetworkManager] move network information into a separate interface (Tethering)

Comment on attachment 8620770 [details] MozReview Request: Bug 1167132 - Part 6: [NetworkManager] move network information into a separate interface (RadioInterface) Bug 1167132 - Part 6: [NetworkManager] move network information into a separate interface (RadioInterface)

Comment on attachment 8620771 [details] MozReview Request: Bug 1167132 - Part 7: [NetworkManager] move network information into a separate interface (MobileConnectionService) Bug 1167132 - Part 7: [NetworkManager] move network information into a separate interface (MobileConnectionService)

Comment on attachment 8620772 [details] MozReview Request: Bug 1167132 - Part 8: [NetworkManager] move network information into a separate interface (MmsService) Bug 1167132 - Part 8: [NetworkManager] move network information into a separate interface (MmsService)

Comment on attachment 8620773 [details] MozReview Request: Bug 1167132 - Part 9: [NetworkManager] move network information into a separate interface (shell) Bug 1167132 - Part 9: [NetworkManager] move network information into a separate interface (shell)

Comment on attachment 8620774 [details] MozReview Request: Bug 1167132 - Part 10: [NetworkManager] move network information into a separate interface (PushService) Bug 1167132 - Part 10: [NetworkManager] move network information into a separate interface (PushService)

Comment on attachment 8620775 [details] MozReview Request: Bug 1167132 - Part 11: [NetworkManager] move network information into a separate interface (Gps) Bug 1167132 - Part 11: [NetworkManager] move network information into a separate interface (Gps)

Comment on attachment 8620776 [details] MozReview Request: Bug 1167132 - Part 12: [NetworkManager] move network information into a separate interface (NetStatistics) Bug 1167132 - Part 12: [NetworkManager] move network information into a separate interface (NetStatistics)

Comment on attachment 8620777 [details] MozReview Request: Bug 1167132 - Part 13: [NetworkManager] move network information into a separate interface (discovery) Bug 1167132 - Part 13: [NetworkManager] move network information into a separate interface (discovery)

Comment on attachment 8620778 [details] MozReview Request: Bug 1167132 - Part 14: [NetworkManager] move network information into a separate interface (NetworkStats) Bug 1167132 - Part 14: [NetworkManager] move network information into a separate interface (NetworkStats)

Comment on attachment 8620779 [details] MozReview Request: Bug 1167132 - Part 15: [NetworkManager] move network information into a separate interface (Mtransport) Bug 1167132 - Part 15: [NetworkManager] move network information into a separate interface (Mtransport)

Comment on attachment 8620764 [details] MozReview Request: Bug 1167132 - Part 1: [NetworkManager] move network information into a separate interface (idl). r?echen https://reviewboard.mozilla.org/r/10827/#review9825 Looks good.

Comment on attachment 8620768 [details] MozReview Request: Bug 1167132 - Part 4: [NetworkManager] move network information into a separate interface (DataCall) https://reviewboard.mozilla.org/r/10833/#review10819 Please see my comments, thank you. ::: dom/system/gonk/DataCallManager.js:1450 (Diff revision 2) > - let reason = Ci.nsINetworkInterface.DATACALL_DEACTIVATE_NO_REASON; > + let reason = Ci.nsIDataCallInterface.DATACALL_DEACTIVATE_NO_REASON; Nice catch!! ::: dom/system/gonk/DataCallManager.js:1659 (Diff revision 2) > return this.state == NETWORK_STATE_CONNECTED; ```this.state == NETWORK_STATE_CONNECTED;``` ::: dom/system/gonk/DataCallManager.js:841 (Diff revision 2) > - let dataCall = networkInterface.dataCall; > + let dataCall = networkInterface.info.dataCall; Should this be? ``` let dataCall = networkInterface.dataCall; ```

Comment on attachment 8620767 [details] MozReview Request: Bug 1167132 - Part 3: [NetworkManager] move network information into a separate interface (Wifi) Looks good to me! Sorry for late review :p

Comment on attachment 8620766 [details] MozReview Request: Bug 1167132 - Part 2: [NetworkManager] move network information into a separate interface (NetworkManager) https://reviewboard.mozilla.org/r/10829/#review10053 Mostly looks good, just few comments. And I would like to review it again since there are a lot of changes in this part. :) ::: dom/system/gonk/NetworkManager.js:262 (Diff revision 2) > - getNetworkId: function(network) { > + getNetworkId: function(networkInfo) { s/networkInfo/aNetworkInfo/ ::: dom/system/gonk/NetworkManager.js:553 (Diff revision 2) > - isValidatedNetwork: function(network) { > + isValidatedNetwork: function(networkInfo) { s/networkInfo/aNetworkInfo/ ::: dom/system/gonk/NetworkManager.js:891 (Diff revision 2) > - convertConnectionType: function(network) { > + convertConnectionType: function(aNetwork) { s/aNetwork/aNetworkInfo/ ::: dom/system/gonk/NetworkManager.js:793 (Diff revision 2) > this.active = null; `active` now caches the networkInfo of active network, how about having a clearer naming, maybe `activeNetworkInfo`?

https://reviewboard.mozilla.org/r/10829/#review10053 > `active` now caches the networkInfo of active network, how about having a clearer naming, maybe `activeNetworkInfo`? `activeNetworkInfo` will definitely be clearer, I was just worried we would be changing too much other module's code since this is exposed in the interface. But I agree with you. :)

Hi Edgar, I have changed 'active' to 'activeNetworkInfo', may I have you review again? Thanks.

Addressed review comment 46.

Addressed review comment 44. Edgar, may I have your review again? Thanks.

Comment on attachment 8630911 [details] [diff] [review] Part 2: NetworkManager changes, v2. Review of attachment 8630911 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thank you. ::: dom/system/gonk/NetworkManager.js @@ -492,5 @@ > } > this._preferredNetworkType = val; > }, > > - active: null, Nit: Put `_activeNetwork: null,` here (follow same style as _preferredNetworkType).

Comment on attachment 8630915 [details] [diff] [review] Part 4: data call changes, v2. Review of attachment 8630915 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/DataCallManager.js @@ +415,5 @@ > this.dataNetworkInterfaces.clear(); > this._dataCalls = []; > this.clientId = null; > > + this.dataCallInterface.unregisterListener(this); Nice catch!!! :)

Comment on attachment 8630916 [details] [diff] [review] Part 5: tethering changes, v1. Review of attachment 8630916 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/TetheringService.js @@ +631,5 @@ > return; > } > > + this._tetheringInterface[TETHERING_TYPE_WIFI].internalInterface = > + aNetwork.info.name; It seems that `setWifiTethering` needs interface name only. Maybe we don't have to take nsINetworkInterface as argument [1], unless we might need more information from nsINetworkInterface in the future. Just share my thought here, we could do further discussion for this. [1] https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/nsITetheringService.idl#13-29

Comment on attachment 8630917 [details] [diff] [review] Part 6: RadioInterface changes, v1. Review of attachment 8630917 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ -80,5 @@ > -const NETWORK_TYPE_MOBILE = Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE; > -const NETWORK_TYPE_MOBILE_MMS = Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS; > -const NETWORK_TYPE_MOBILE_SUPL = Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_SUPL; > -const NETWORK_TYPE_MOBILE_IMS = Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_IMS; > -const NETWORK_TYPE_MOBILE_DUN = Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_DUN; NETWORK_TYPE_MOBILE_* are unused consts, it seem we could remove them.

Comment on attachment 8630918 [details] [diff] [review] Part 7: MobileConnection changes, v1. Review of attachment 8630918 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobileconnection/gonk/MobileConnectionService.js @@ +1606,1 @@ > this.notifyDataError(rilNetwork.serviceId, rilNetwork); Ah, I found a flaw existing in the code base for a while: the second argument of notifyDataError should be error message [1], not NetworkInterface or NetworkInfo. [1] https://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/gonk/MobileConnectionService.js?from=MobileConnectionService.js#1378-1385

(In reply to Edgar Chen [:edgar][:echen] from comment #59) > Comment on attachment 8630918 [details] [diff] [review] > Part 7: MobileConnection changes, v1. > > Review of attachment 8630918 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/mobileconnection/gonk/MobileConnectionService.js > @@ +1606,1 @@ > > this.notifyDataError(rilNetwork.serviceId, rilNetwork); > > Ah, I found a flaw existing in the code base for a while: the second > argument of notifyDataError should be error message [1], not > NetworkInterface or NetworkInfo. > > [1] > https://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/gonk/ > MobileConnectionService.js?from=MobileConnectionService.js#1378-1385 Right! thanks for catching this, will fix it in the next version.

(In reply to Edgar Chen [:edgar][:echen] from comment #57) > Comment on attachment 8630916 [details] [diff] [review] > Part 5: tethering changes, v1. > > Review of attachment 8630916 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/TetheringService.js > @@ +631,5 @@ > > return; > > } > > > > + this._tetheringInterface[TETHERING_TYPE_WIFI].internalInterface = > > + aNetwork.info.name; > > It seems that `setWifiTethering` needs interface name only. Maybe we don't > have to take nsINetworkInterface as argument [1], unless we might need more > information from nsINetworkInterface in the future. Just share my thought > here, we could do further discussion for this. > > [1] > https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ > nsITetheringService.idl#13-29 I agree, passing network interface name is enough. It's a good opportunity to fix this, will upload a new patch revising this. Thanks.

Addressed nit in review comment 55.

Some more fixes in DataCallManager.js

Use interfaceName as parameter (instead of network interface) for setWifiTethering().

Removed unused consts, as mentioned in comment 58.

Pass error message when calling notifyDataError().

Comment on attachment 8637620 [details] [diff] [review] Part 7: MobileConnection changes. r=echen Review of attachment 8637620 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/DataCallManager.js @@ +840,5 @@ > if (networkInterface && networkInterface.enabled) { > let dataCall = networkInterface.dataCall; > + if (this._compareDataCallOptions(dataCall, aDataCall)) { > + Services.obs.notifyObservers(networkInterface.info, > + TOPIC_DATA_CALL_ERROR, aErrorMsg); I removed the part where we compare cid, cause this is called only on data call setup error.

Bevis, we are exposing nsINetworkInfo instead of nsINetworkInterface now. I have made the changes in mms, would you mind reviewing this? Thanks.

Fabrice, we are exposing nsINetworkInfo instead of nsINetworkInterface in 'network-connection-state-changed' events. I've made the corresponding changes in shell.js, would you mind review this? Thanks.

Hi :nsm, we are exposing nsINetworkInfo (.activeNetworkInfo) instead of nsINetworkInterface (.active) in NetworkManager. I've made the corresponding changes in Push modules, would you mind reviewing this? Thanks.

Hi Kanru, we are exposing nsINetworkInfo instead of nsINetworkInterface in NetworkManager. I've made the corresponding changes in gps, would you mind review this? Thanks.

Comment on attachment 8637615 [details] [diff] [review] Part 2: NetworkManager changes, v3. Review of attachment 8637615 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/NetworkManager.js @@ +573,3 @@ > } > > + return this.resolveHostname(networkInfo, host) nit: The signature of resolveHostname is still unchanged and take "network" as 1st parameter. :p

Comment on attachment 8637627 [details] [diff] [review] Part 8: mms changes, v1. Review of attachment 8637627 [details] [diff] [review]: ----------------------------------------------------------------- r=me after the inlined comment addressed. Thanks! ::: dom/mobilemessage/gonk/MmsService.js @@ +522,5 @@ > return; > } > > // Check if the network state change belongs to this service. > + let network = subject.QueryInterface(Ci.nsIRilNetworkInfo); nit: please replace "network" as "networkInfo" as well for better understanding.

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #72) > Comment on attachment 8637615 [details] [diff] [review] > Part 2: NetworkManager changes, v3. > > Review of attachment 8637615 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/NetworkManager.js > @@ +573,3 @@ > > } > > > > + return this.resolveHostname(networkInfo, host) > > nit: The signature of resolveHostname is still unchanged and take "network" > as 1st parameter. :p Will do! (In reply to Bevis Tseng[:bevistseng][:btseng] from comment #73) > Comment on attachment 8637627 [details] [diff] [review] > Part 8: mms changes, v1. > > Review of attachment 8637627 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me after the inlined comment addressed. > > Thanks! > > ::: dom/mobilemessage/gonk/MmsService.js > @@ +522,5 @@ > > return; > > } > > > > // Check if the network state change belongs to this service. > > + let network = subject.QueryInterface(Ci.nsIRilNetworkInfo); > > nit: please replace "network" as "networkInfo" as well for better > understanding. Will do. Thanks Bevis!

Hi jryans, we are exposing nsINetworkInfo instead of nsINetworkInterface in NetworkManager. I've made the corresponding changes in discovery, would you mind review this? Thanks.

Hi Ethan, we are exposing nsINetworkInfo instead of nsINetworkInterface in NetworkManager. I've made the corresponding changes in necko/netstats, would you mind review this? Thanks.

Ethan, would mind reviewing this too? Thanks.

Edgar, here are the changes for NetworkInterfaceListService, would you mind reviewing this? Thanks.

Comment on attachment 8637646 [details] [diff] [review] Part 11: gps changes, v1. Review of attachment 8637646 [details] [diff] [review]: ----------------------------------------------------------------- LGTM

Comment on attachment 8637617 [details] [diff] [review] Part 5: tethering changes, v2. Review of attachment 8637617 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment addressed, thank you. ::: dom/system/gonk/nsITetheringService.idl @@ +16,5 @@ > * @param enabled > * Boolean that indicates whether tethering should be enabled (true) or > * disabled (false). > + * @param interfaceName > + * The Wifi network interface name for external interface. s/external/internal/

Comment on attachment 8637733 [details] [diff] [review] Part 13: necko/netstats changes, v1. Review of attachment 8637733 [details] [diff] [review]: ----------------------------------------------------------------- This patch simply replaces nsINetworkInterface by nsINetworkInfo. It looks pretty good. Thanks Jessica! r=me. :)

Comment on attachment 8637734 [details] [diff] [review] Part 14: netstats changes, v1. Review of attachment 8637734 [details] [diff] [review]: ----------------------------------------------------------------- This patch also looks perfect to me! r=me.

it's within the 2.5 scope - As a user, I want to have a robust control in data connection.

rebased.

Rebased and renamed the first parameter of resolveHostname to aNetworkInfo.

rebased.

Correct comment in nsITetheringService.idl as comment 80.

rebased.

Use variable name "networkInfo" instead of "network" for better understanding.

Add r=fabrice.

Add r=nsm.

Add r=kanru.

Add r=jryans.

Add r=ethan.

Add r=ethan.

Rebased and add r=echen.

Wow, you got r+ for all parts of patches! Well done!

Thanks to all the reviewers! :) try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=202656ad4802

https://hg.mozilla.org/integration/b2g-inbound/rev/d30bd2a57872 https://hg.mozilla.org/integration/b2g-inbound/rev/c722ee4b5cc3 https://hg.mozilla.org/integration/b2g-inbound/rev/ed6a2c98608d https://hg.mozilla.org/integration/b2g-inbound/rev/d3b33e59a3ce https://hg.mozilla.org/integration/b2g-inbound/rev/5d44751ef356 https://hg.mozilla.org/integration/b2g-inbound/rev/36009a9b7a48 https://hg.mozilla.org/integration/b2g-inbound/rev/b6dc4da81f01 https://hg.mozilla.org/integration/b2g-inbound/rev/b61a73af6333 https://hg.mozilla.org/integration/b2g-inbound/rev/d922c89bb212 https://hg.mozilla.org/integration/b2g-inbound/rev/928cd60fdc21 https://hg.mozilla.org/integration/b2g-inbound/rev/6580a14154ef https://hg.mozilla.org/integration/b2g-inbound/rev/3d3e059df2eb https://hg.mozilla.org/integration/b2g-inbound/rev/1db1eb705ba9 https://hg.mozilla.org/integration/b2g-inbound/rev/efa5238b30d6 https://hg.mozilla.org/integration/b2g-inbound/rev/e684f5746f2f

https://hg.mozilla.org/mozilla-central/rev/d30bd2a57872 https://hg.mozilla.org/mozilla-central/rev/c722ee4b5cc3 https://hg.mozilla.org/mozilla-central/rev/ed6a2c98608d https://hg.mozilla.org/mozilla-central/rev/d3b33e59a3ce https://hg.mozilla.org/mozilla-central/rev/5d44751ef356 https://hg.mozilla.org/mozilla-central/rev/36009a9b7a48 https://hg.mozilla.org/mozilla-central/rev/b6dc4da81f01 https://hg.mozilla.org/mozilla-central/rev/b61a73af6333 https://hg.mozilla.org/mozilla-central/rev/d922c89bb212 https://hg.mozilla.org/mozilla-central/rev/928cd60fdc21 https://hg.mozilla.org/mozilla-central/rev/6580a14154ef https://hg.mozilla.org/mozilla-central/rev/3d3e059df2eb https://hg.mozilla.org/mozilla-central/rev/1db1eb705ba9 https://hg.mozilla.org/mozilla-central/rev/efa5238b30d6 https://hg.mozilla.org/mozilla-central/rev/e684f5746f2f