Closed Bug 984919 Opened 11 years ago Closed 11 years ago

[RIL][DSDS] New calls an inactive SIM when there is an active SIM should be rejected

Categories

(Firefox OS Graveyard :: RIL, defect)

Product:
Firefox OS Graveyard
Component:
RIL
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

Status:
RESOLVED FIXED
Milestone:
1.4 S5 (11apr)
Project Flags:
blocking-b2g 1.4+
Tracking Flags:
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: drs, Assigned: aknow)

References

Details

Attachments

(2 files, 3 obsolete files)

[final] Part 1: Cannot place a new call if other sim is in use. r=hsinyi
2.33 KB, patch
aknow
: review+
Details | Diff | Splinter Review
[final] Part 2: Test DSDS connection conflict. r=hsinyi
3.11 KB, patch
aknow
: review+
Details | Diff | Splinter Review
When a call is currently in progress on SIM 1, and we place a call on SIM 2 (or vice versa), Gecko should generate an error instead of attempting to place the call and getting into a bad state. We can come back to this when we do DSDA.
Blocks: b2g-dsds-1.4
blocking-b2g: --- → 1.4?
See Also: → 978814
Assignee: nobody → szchen
We create a new error cause for this case -- "OtherConnectionInUse".
Attachment #8392970 - Flags: review?(htsai)
Comment on attachment 8392970 [details] [diff] [review] Cannot place a new call if other sim is in use Review of attachment 8392970 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/telephony/gonk/TelephonyProvider.js @@ +430,5 @@ > + // For DSDS, if there is aleady a call on SIM X, we cannot place any new > + // call on other SIM. > + let callOnOtherSim = false; > + for (let cid = 0; cid < this._numClients; ++cid) { > + if (cid == aClientId) continue; with {} please.
Attachment #8392970 - Flags: review?(htsai) → review+
Comment on attachment 8392970 [details] [diff] [review] Cannot place a new call if other sim is in use Review of attachment 8392970 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/telephony/gonk/TelephonyProvider.js @@ +430,5 @@ > + // For DSDS, if there is aleady a call on SIM X, we cannot place any new > + // call on other SIM. > + let callOnOtherSim = false; > + for (let cid = 0; cid < this._numClients; ++cid) { > + if (cid == aClientId) continue; More specifically, if (cid == aClientId) { continue; }
Why is there no tests for this patch?
(In reply to Anthony Ricaud (:rik) from comment #4) > Why is there no tests for this patch? Because we don't even have a DSDS emulator right now ...
ㄚ(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5) > (In reply to Anthony Ricaud (:rik) from comment #4) > > Why is there no tests for this patch? > > Because we don't even have a DSDS emulator right now ... More specifically, we have DSDA, though try server doesn't support it yet.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6) > ㄚ(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5) > > (In reply to Anthony Ricaud (:rik) from comment #4) > > > Why is there no tests for this patch? > > > > Because we don't even have a DSDS emulator right now ... > > More specifically, we have DSDA, though try server doesn't support it yet. I could write a test case but it could only be verified on local DSDA emulator. The emulator on try doesn't support DSDS thus the test case will be skipped.
Makes sense, thanks. Do we have a bug tracking DSDS and DSDA emulator support on try?
Attachment #8393451 - Flags: review?(htsai)
Doug, What is bad state here? What is the impact to the user?
Flags: needinfo?(drs+bugzilla)
(In reply to Preeti Raghunath(:Preeti) from comment #10) > Doug, > > What is bad state here? What is the impact to the user? See bug 978814 comment 5. This bug (referring to bug 984919) is for fixing this issue on the Gecko side, which guarantees that we will never get into this state, but without bug 978814 we will get ugly error messages and some functionality will be broken. For example, we will still show SIM pickers when long pressing, even though the user will get an error message if they select an inactive SIM.
Flags: needinfo?(drs+bugzilla)
1.4+ as the Gecko bug has also been +ed for consistenncy
blocking-b2g: 1.4? → 1.4+
(In reply to Anthony Ricaud (:rik) from comment #8) > Makes sense, thanks. Do we have a bug tracking DSDS and DSDA emulator > support on try? Hey Vicamo, Any idea of this? Also, I just filed bug 985934 for DSDS emulator.
Flags: needinfo?(vyang)
Comment on attachment 8393451 [details] [diff] [review] Part 2: Test DSDS connection conflict Review of attachment 8393451 [details] [diff] [review]: ----------------------------------------------------------------- We have DSDA emulator, and this bug is guarding from some illegal actions in terms of DSDS on gecko side. However the test case could pass even based on the current DSDA emulator because this bug is just restricting some DSDA functionality on emulator. So the test patch looks okay for now because FxOS considers only DSDS currently. Thanks! ::: dom/telephony/test/marionette/test_dsds_connection_conflict.js @@ +67,5 @@ > + ok(false); > + }, cause => { > + is(cause, "OtherConnectionInUse"); > + }) > + .then(() => remoteHangUp(outCall)); Please add one more case: have a call on modem_1 first then dial out 2nd call on modem_0, to show that the guarding behaviour is order-independent. Thanks.
Attachment #8393451 - Flags: review?(htsai)
Attachment #8393451 - Attachment is obsolete: true
Attachment #8394649 - Flags: review?(htsai)
Comment on attachment 8394649 [details] [diff] [review] Part 2#2: Test DSDS connection conflict Review of attachment 8394649 [details] [diff] [review]: ----------------------------------------------------------------- yeah~ thanks.
Attachment #8394649 - Flags: review?(htsai) → review+
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #13) > (In reply to Anthony Ricaud (:rik) from comment #8) > > Makes sense, thanks. Do we have a bug tracking DSDS and DSDA emulator > > support on try? > > Hey Vicamo, any idea of this? No, not yet. Emulator is always DSDA for now. > Also, I just filed bug 985934 for DSDS emulator. Thank you :)
Flags: needinfo?(vyang)
I add one more line in the end of the test cases to set it back to modem 0.
Attachment #8394649 - Attachment is obsolete: true
Attachment #8394668 - Flags: review+
(In reply to Szu-Yu Chen [:aknow] from comment #19) > Created attachment 8394668 [details] [diff] [review] > [final] Part 2: Test DSDS connection conflict. r=hsinyi > > I add one more line in the end of the test cases to set it back to modem 0. Thanks for this!
https://hg.mozilla.org/mozilla-central/rev/a3d1c00b6592 https://hg.mozilla.org/mozilla-central/rev/7dcecf85a869
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S4 (28mar)
Depends on: 988979
Backed out at the request of jsmith for causing bug 988979. remote: https://hg.mozilla.org/mozilla-central/rev/94322fe721fb remote: https://hg.mozilla.org/mozilla-central/rev/5dea2146a099 Note this will also require backout/followups on aurora.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Plz see bug 988979 comment 17. This haven't caused that regression bug 988979. And the try result, which covered the test case of icc card state transition, passed. It's safe to let this go.
Keywords: checkin-needed
No longer depends on: 988979
Status: REOPENED → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/b63903e0338f https://hg.mozilla.org/mozilla-central/rev/c94dbcadec71
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: 1.4 S4 (28mar) → 1.5 S1 (9may)
The following changesets are now in Firefox Nightly: > b63903e0338f Bug 984919 - Part 1: Cannot place a new call if other sim is in use. r=hsinyi > c94dbcadec71 Bug 984919 - Part 2: Test DSDS connection conflict. r=hsinyi Nightly Build Information: ID: 20140402030201 Changeset: 4941a2ac0786109b08856738019b016a6c5a66a6 Version: 31.0a1 TBPL: https://tbpl.mozilla.org/?rev=4941a2ac0786 URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central Download Links: > Linux x86: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-i686.tar.bz2 > Linux x86_64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64.tar.bz2 > Linux x86_64 ASAN: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2 > Mac: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.mac.dmg > Win32: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win32.installer.exe > Win64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win64-x86_64.installer.exe Previous Nightly Build Information: ID: 20140401030203 Changeset: 1417d180a1d8665b1a91b897d1cc4cc31e7980d4 Version: 31.0a1 TBPL: https://tbpl.mozilla.org/?rev=1417d180a1d8 URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-01-03-02-03-mozilla-central
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: