Closed
Bug 952371
Opened 10 years ago
Closed 10 years ago
[B2G] [DSDS] mozMobileconnection.lastKnownNetwork and .lastKnownHomeNetwork aren't working
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed)
People
(Reporter: hsinyi, Assigned: jessica)
References
Details
Attachments
(4 files, 4 obsolete files)
2.03 KB,
patch
|
jessica
:
review+
|
Details | Diff | Splinter Review |
8.08 KB,
patch
|
hsinyi
:
review+
|
Details | Diff | Splinter Review |
2.05 KB,
patch
|
jessica
:
review+
|
Details | Diff | Splinter Review |
1.20 KB,
patch
|
jessica
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•10 years ago
|
||
This bug was originally reported on https://bugzilla.mozilla.org/show_bug.cgi?id=948222#c21. The root cause is we still use one preference to record two SIMs. We should have separate preferences for each SIM. Re-design the preference format for "ril.lastKnownNetwork" and "ril.lastKnownHomeNetwork" is required.
Summary: [B2G] [DSDS] we dpmpreference ril.lastKnownNetwork and ril.lastKnownHomeNetwork → [B2G] [DSDS] mozMobileconnection.lastKnownNetwork and .lastKnownHomeNetwork aren't working
Assignee | ||
Comment 2•10 years ago
|
||
I am wondering why mozMobileconnection.lastKnownNetwork and .lastKnownHomeNetwork were designed using preferences, and not like the other APIs, go through the RILContentHelper -> RadioInterfaceLayer flow. I mean, can we still have two different permissions, mobilenetwork and mobileconnection, and mobilenetwork will still have access only to lastKnownNetwork and lastKnownHomeNetwork, but the values are retrieved directly from RadioInterfaceLayer? Will this break the current security model? Sorry if this is something trivial...
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to Jessica Jong [:jjong] [:jessica] from comment #2) > I am wondering why mozMobileconnection.lastKnownNetwork and > .lastKnownHomeNetwork were designed using preferences, and not like the > other APIs, go through the RILContentHelper -> RadioInterfaceLayer flow. > I mean, can we still have two different permissions, mobilenetwork and > mobileconnection, and mobilenetwork will still have access only to > lastKnownNetwork and lastKnownHomeNetwork, but the values are retrieved > directly from RadioInterfaceLayer? Will this break the current security > model? > > Sorry if this is something trivial... I tried to get more clues from Bug 866272 but in vain. Hi, Fabrice, may I have your explanation here? Thank you.
Flags: needinfo?(fabrice)
Comment 4•10 years ago
|
||
That was done this way because when RadioInterfaceLayer gets the IPC message to retrieve lastKnownNetwork, there is no way to check for mobilenetwork || mobileconnection. If any of these fail, we end up killing the child process. We could probably do it without preferences by sending different IPC messages based on whether the child has mobilenetwork or mobileconnection permission. I didn't do it for lack of time when adding this...
Flags: needinfo?(fabrice)
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8351318 -
Flags: review?(htsai)
Assignee | ||
Updated•10 years ago
|
Attachment #8351319 -
Flags: feedback?(htsai)
Assignee | ||
Updated•10 years ago
|
Attachment #8351321 -
Flags: review?(htsai)
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8351322 [details] [diff] [review] Part 4: tests I am just testing the basic functions, cause to test other cases, e.g. changing the operator, would require listening events after sending the corresponding commands to the emulator, which would require 'mobileconnection' permission.
Attachment #8351322 -
Flags: review?(htsai)
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8351318 [details] [diff] [review] Part 1: idl changes Review of attachment 8351318 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/network/interfaces/nsIMobileConnectionProvider.idl @@ +48,5 @@ > void unregisterMobileConnectionMsg(in unsigned long clientId, > in nsIMobileConnectionListener listener); > > + DOMString getLastKnownNetwork(in unsigned long clientId); > + DOMString getLastKnownHomeNetwork(in unsigned long clientId); Please add a comment saying 'mobilenetwork' permission is required for accessing these two functions.
Attachment #8351318 -
Flags: review?(htsai) → review+
Reporter | ||
Updated•10 years ago
|
Attachment #8351319 -
Flags: feedback?(htsai) → feedback+
Reporter | ||
Comment 11•10 years ago
|
||
Comment on attachment 8351321 [details] [diff] [review] Part 3: ril implementation Review of attachment 8351321 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +241,5 @@ > ppmm.addMessageListener(msgname, this); > } > + for (let msgname of RIL_IPC_MOBILENETWORK_MSG_NAMES) { > + ppmm.addMessageListener(msgname, this); > + } r- because we need 'ppmm.removeMessageListener' for RIL_IPC_MOBILENETWORK_MSG_NAMES as well.
Attachment #8351321 -
Flags: review?(htsai) → review-
Reporter | ||
Comment 12•10 years ago
|
||
Comment on attachment 8351322 [details] [diff] [review] Part 4: tests Review of attachment 8351322 [details] [diff] [review]: ----------------------------------------------------------------- Thank you!!! ::: dom/network/tests/marionette/test_mobile_last_known_network.js @@ +44,5 @@ > + finish(); > +} > + > +runNextTest(); > + nit: no new line at the end of a file
Attachment #8351322 -
Flags: review?(htsai) → review+
Reporter | ||
Comment 13•10 years ago
|
||
1.3? because this is a miss in DSDS implementation.
blocking-b2g: --- → 1.3?
Assignee | ||
Comment 15•10 years ago
|
||
Address review comment 10: - add a comment saying 'mobilenetwork' permission is required for accessing the two functions.
Attachment #8351318 -
Attachment is obsolete: true
Attachment #8355988 -
Flags: review+
Assignee | ||
Comment 16•10 years ago
|
||
Address review comment 11: - add 'ppmm.removeMessageListener' for RIL_IPC_MOBILENETWORK_MSG_NAMES.
Attachment #8351321 -
Attachment is obsolete: true
Attachment #8355989 -
Flags: review?(htsai)
Assignee | ||
Comment 17•10 years ago
|
||
Address review comment 12: - no new line at the end of a file.
Attachment #8351322 -
Attachment is obsolete: true
Attachment #8355990 -
Flags: review+
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8351319 [details] [diff] [review] Part 2: dom changes smaug, We are using IPC messages instead of preferences to query last known networks. May I have your review on this? Thanks.
Attachment #8351319 -
Flags: review?(bugs)
Reporter | ||
Comment 19•10 years ago
|
||
Comment on attachment 8355989 [details] [diff] [review] Part 3: ril implementation (final) Review of attachment 8355989 [details] [diff] [review]: ----------------------------------------------------------------- Great, thanks!
Attachment #8355989 -
Flags: review?(htsai) → review+
Updated•10 years ago
|
Attachment #8351319 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Add f=hsinyi r=smaug.
Attachment #8351319 -
Attachment is obsolete: true
Attachment #8356478 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Attachment #8355988 -
Attachment description: Part 1: idl changes. v2. → Part 1: idl changes (final)
Assignee | ||
Updated•10 years ago
|
Attachment #8355989 -
Attachment description: Part 3: ril implementation, v2. → Part 3: ril implementation (final)
Assignee | ||
Updated•10 years ago
|
Attachment #8355990 -
Attachment description: Part 4: tests, v2. → Part 4: tests (final)
Assignee | ||
Comment 21•10 years ago
|
||
Try run: https://tbpl.mozilla.org/?tree=Try&rev=75fd2b952561
Assignee | ||
Comment 22•10 years ago
|
||
Full try: https://tbpl.mozilla.org/?tree=Try&rev=bf4bb913b2d5 (Rerun with latest healthy codebase and all green!)
Assignee: nobody → jjong
Keywords: checkin-needed
Reporter | ||
Comment 23•10 years ago
|
||
I modified the comments of patches a little bit to provide more information of what the patches have been trying to do. Otherwise, the comment "Part 1 - idl changes" for example is too rough. remote: https://hg.mozilla.org/integration/b2g-inbound/rev/3826c3a2d5ee remote: https://hg.mozilla.org/integration/b2g-inbound/rev/3fd3861f9778 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/50101ad17ae7 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/3b867c474f45
Keywords: checkin-needed
Comment 24•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3826c3a2d5ee https://hg.mozilla.org/mozilla-central/rev/3fd3861f9778 https://hg.mozilla.org/mozilla-central/rev/50101ad17ae7 https://hg.mozilla.org/mozilla-central/rev/3b867c474f45
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Comment 25•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/031315f1fe0b https://hg.mozilla.org/releases/mozilla-aurora/rev/f58b2c2c9f27 https://hg.mozilla.org/releases/mozilla-aurora/rev/1aab8c2a2114 https://hg.mozilla.org/releases/mozilla-aurora/rev/ef25b3b02dcc
status-b2g-v1.3:
--- → fixed
status-firefox27:
--- → wontfix
status-firefox28:
--- → fixed
status-firefox29:
--- → fixed
Reporter | ||
Updated•10 years ago
|
Blocks: b2g-ril-interface
You need to log in
before you can comment on or make changes to this bug.
Description
•