Closed
Bug 929864
Opened 9 years ago
Closed 9 years ago
B2G DSDS: correctly update RILNetworkInterface when wifi connection changes
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: hsinyi, Assigned: jessica)
References
Details
Attachments
(1 file, 1 obsolete file)
2.57 KB,
patch
|
hsinyi
:
review+
|
Details | Diff | Splinter Review |
When wifi connection changes, currently we update the network interface only for radioInterface(0). We need to improve that for DSDS scenario. [1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkManager.js#247
Reporter | ||
Updated•9 years ago
|
Blocks: b2g-dsds-1.3
Reporter | ||
Comment 1•9 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #0) > When wifi connection changes, currently we update the network interface only > for radioInterface(0). We need to improve that for DSDS scenario. > > > [1] > http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkManager. > js#247 And here: http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkManager.js#275
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jjong
Assignee | ||
Comment 2•9 years ago
|
||
In this patch, we update all radio interfaces' ril network interface when wifi is connected/disconnected, then each of the radio interfaces will check if they need to do anything about it. BTW, do we have a preference for data connection default service id, as Bug 926302 does for other services? I was thinking only updating the ril network interface for the default one.. but the current way will work if we are going to support dual-active phones. Any suggestions?
Attachment #820866 -
Flags: feedback?(htsai)
Attachment #820866 -
Flags: feedback?(echen)
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Jessica Jong [:jjong] [:jessica] from comment #2) > Created attachment 820866 [details] [diff] [review] > update-all-ril-network-ifaces.patch > > In this patch, we update all radio interfaces' ril network interface when > wifi is connected/disconnected, then each of the radio interfaces will check > if they need to do anything about it. > > BTW, do we have a preference for data connection default service id, as Bug > 926302 does for other services? I was thinking only updating the ril network > interface for the default one.. but the current way will work if we are > going to support dual-active phones. Any suggestions? No, at this moment, there's no preference for data connection default service id as in this corresponding API, we don't have the concept of 'default' for data connection. But for gaia, there might be a setting key recording the user preference for indication of the primary data connection service. The current solution looks simple, feasible and flexible to me. I am a proponent of that. :)
Comment 4•9 years ago
|
||
Comment on attachment 820866 [details] [diff] [review] update-all-ril-network-ifaces.patch Review of attachment 820866 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/NetworkManager.js @@ +32,5 @@ > "nsIDNSService"); > > +XPCOMUtils.defineLazyGetter(this, "gNumRadioInterfaces", function() { > + try { > + return Services.prefs.getIntPref("ril.numRadioInterfaces"); NetworkManager is in chrome process. Do we need to access preference directly or we should use the |RadioInterfaceLayer.numRadioInterfaces|?
Reporter | ||
Comment 5•9 years ago
|
||
Comment on attachment 820866 [details] [diff] [review] update-all-ril-network-ifaces.patch Review of attachment 820866 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/NetworkManager.js @@ +30,5 @@ > XPCOMUtils.defineLazyServiceGetter(this, "gDNSService", > "@mozilla.org/network/dns-service;1", > "nsIDNSService"); > > +XPCOMUtils.defineLazyGetter(this, "gNumRadioInterfaces", function() { After bug 920551, this needs to be guarded by #ifdef MOZ_B2G_RIL. @@ +32,5 @@ > "nsIDNSService"); > > +XPCOMUtils.defineLazyGetter(this, "gNumRadioInterfaces", function() { > + try { > + return Services.prefs.getIntPref("ril.numRadioInterfaces"); We've been encountering various problems with Bug 926302. Vicamo provided a nice solution eventually. After that, in chrome, we could still use getService(Ci.nsIRadioInterfaceLayer).numRadioInterfaces, while in content, we have no way excepting reading preference. Please help change this to |getService(Ci.nsIRadioInterfaceLayer).numRadioInterfaces|. I am sorry about that.
Attachment #820866 -
Flags: feedback?(htsai) → feedback+
Updated•9 years ago
|
Attachment #820866 -
Flags: feedback?(echen) → feedback+
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Edgar Chen [:edgar][:echen] from comment #4) > Comment on attachment 820866 [details] [diff] [review] > update-all-ril-network-ifaces.patch > > Review of attachment 820866 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/NetworkManager.js > @@ +32,5 @@ > > "nsIDNSService"); > > > > +XPCOMUtils.defineLazyGetter(this, "gNumRadioInterfaces", function() { > > + try { > > + return Services.prefs.getIntPref("ril.numRadioInterfaces"); > > NetworkManager is in chrome process. Do we need to access preference > directly or we should use the |RadioInterfaceLayer.numRadioInterfaces|? Yes, you are right. I'd use |this.mRIL.numRadioInterfaces| directly. Thanks!
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5) > Comment on attachment 820866 [details] [diff] [review] > update-all-ril-network-ifaces.patch > > Review of attachment 820866 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/NetworkManager.js > @@ +30,5 @@ > > XPCOMUtils.defineLazyServiceGetter(this, "gDNSService", > > "@mozilla.org/network/dns-service;1", > > "nsIDNSService"); > > > > +XPCOMUtils.defineLazyGetter(this, "gNumRadioInterfaces", function() { > > After bug 920551, this needs to be guarded by #ifdef MOZ_B2G_RIL. I think I will get rid of gNumRadioInterfaces and use |this.mRIL.numRadioInterfaces| directly, > @@ +32,5 @@ > > "nsIDNSService"); > > > > +XPCOMUtils.defineLazyGetter(this, "gNumRadioInterfaces", function() { > > + try { > > + return Services.prefs.getIntPref("ril.numRadioInterfaces"); > > We've been encountering various problems with Bug 926302. Vicamo provided a > nice solution eventually. After that, in chrome, we could still use > getService(Ci.nsIRadioInterfaceLayer).numRadioInterfaces, while in content, > we have no way excepting reading preference. > > Please help change this to > |getService(Ci.nsIRadioInterfaceLayer).numRadioInterfaces|. I am sorry about > that. Thank you! :)
Assignee | ||
Comment 8•9 years ago
|
||
Review comments in Comment 4 and Comment 5 addressed: 1. Use |this.mRIL.numRadioInterfaces| directly 2. Add f= and r= in patch title
Attachment #820866 -
Attachment is obsolete: true
Attachment #821542 -
Flags: review?(htsai)
Reporter | ||
Comment 9•9 years ago
|
||
Comment on attachment 821542 [details] [diff] [review] update-all-ril-network-ifaces-v2.patch Review of attachment 821542 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thanks.
Attachment #821542 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Try results: https://tbpl.mozilla.org/?tree=Try&rev=028c802d1038
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 11•9 years ago
|
||
remote: https://hg.mozilla.org/integration/b2g-inbound/rev/f0bd0d0d0cd5
Keywords: checkin-needed
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f0bd0d0d0cd5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Blocks: b2g-dsds-1.4
Updated•9 years ago
|
No longer blocks: b2g-dsds-1.4
You need to log in
before you can comment on or make changes to this bug.
Description
•