Closed
Bug 993255
Opened 10 years ago
Closed 10 years ago
[B2G] [RIL] follow-up of bug 990467 - should hangUp pendingMO properly
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(blocking-b2g:1.3T+, b2g-v1.3 unaffected, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)
Tracking | Status | |
---|---|---|
b2g-v1.3 | --- | unaffected |
b2g-v1.3T | --- | fixed |
b2g-v1.4 | --- | fixed |
b2g-v2.0 | --- | fixed |
People
(Reporter: hsinyi, Assigned: aknow)
References
Details
(Whiteboard: [p=2])
Attachments
(12 files, 16 obsolete files)
3.34 KB,
patch
|
Details | Diff | Splinter Review | |
4.10 KB,
patch
|
Details | Diff | Splinter Review | |
1.34 KB,
patch
|
Details | Diff | Splinter Review | |
3.08 KB,
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
4.11 KB,
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
1.34 KB,
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
5.03 KB,
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
4.27 KB,
patch
|
aknow
:
review+
|
Details | Diff | Splinter Review |
3.34 KB,
patch
|
aknow
:
review+
praghunath
:
approval-mozilla-b2g30+
|
Details | Diff | Splinter Review |
4.02 KB,
patch
|
aknow
:
review+
praghunath
:
approval-mozilla-b2g30+
|
Details | Diff | Splinter Review |
1.34 KB,
patch
|
aknow
:
review+
praghunath
:
approval-mozilla-b2g30+
|
Details | Diff | Splinter Review |
4.31 KB,
patch
|
aknow
:
review+
praghunath
:
approval-mozilla-b2g30+
|
Details | Diff | Splinter Review |
This is a follow-up of bug 990467. 1) We miss _addNewVoiceCall() when REQUEST_DIAL_EMERGENCY_CALL. 2) We should consider the case of hanging up a call with OUTGOING_PLACEHOLDER_CALL_INDEX
Assignee | ||
Comment 1•10 years ago
|
||
Only focus on item 2 in this bug because handle REQUEST_DIAL_EMERGENCY_CALL properly is another topic.
Assignee: nobody → szchen
Summary: [B2G] [RIL] follow-up of bug 990467 - should _addNewVoiceCall() in the handler of REQUEST_DIAL_EMERGENCY_CALL and hangUp pendingMO properly → [B2G] [RIL] follow-up of bug 990467 - should hangUp pendingMO properly
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8403146 -
Flags: review?(htsai)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8403147 -
Flags: review?(htsai)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8403146 [details] [diff] [review] Part 1: Refactor: extract removeVoiceCall Review of attachment 8403146 [details] [diff] [review]: ----------------------------------------------------------------- Seems wrong file was attached.
Attachment #8403146 -
Flags: review?(htsai)
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8403147 [details] [diff] [review] Part 2: Hangup pending outgoing call Review of attachment 8403147 [details] [diff] [review]: ----------------------------------------------------------------- Seems wrong file was attached.
Attachment #8403147 -
Flags: review?(htsai)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8403146 -
Attachment is obsolete: true
Attachment #8403778 -
Flags: review?(htsai)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8403147 -
Attachment is obsolete: true
Attachment #8403780 -
Flags: review?(htsai)
Reporter | ||
Comment 8•10 years ago
|
||
Comment on attachment 8403778 [details] [diff] [review] Part 1#2: Refactor: extract removeVoiceCall Review of attachment 8403778 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +3876,5 @@ > + _removeVoiceCall: function(removedCall, failCause) { > + if (this.currentConference.participants[removedCall.callIndex]) { > + removedCall.isConference = false; > + delete this.currentConference.participants[removedCall.callIndex]; > + delete this.currentCalls[removedCall.callIndex]; Don't we need to check if this.currentCalls[removedCall.callIndex] exists before? @@ +3883,5 @@ > + // in one task. > + this._handleDisconnectedCall(removedCall); > + } else { > + delete this.currentCalls[removedCall.callIndex]; > + if (failCause) { Who is going to assign 'failCause'? I don't see that in the patches...
Attachment #8403778 -
Flags: review?(htsai)
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8403780 [details] [diff] [review] Part 2#2: Hangup pending outgoing call Review of attachment 8403780 [details] [diff] [review]: ----------------------------------------------------------------- I am fine with the behaviour; however, I re-considered your concern, what if CLCC comes back after we hang up the pending call, and checked AOSP's behaviour. Per [1], AOSP hangs up that call again to not surprise user. I think it sounds a better and robust behaviour. Please take care of that. Sorry for the changing mind. [1] https://code.google.com/p/android-source-browsing/source/browse/telephony/java/com/android/internal/telephony/gsm/GsmCallTracker.java?repo=platform--frameworks--base&name=jb-release#466
Attachment #8403780 -
Flags: review?(htsai)
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #8) > Comment on attachment 8403778 [details] [diff] [review] > Part 1#2: Refactor: extract removeVoiceCall > > Review of attachment 8403778 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/ril_worker.js > @@ +3876,5 @@ > > + _removeVoiceCall: function(removedCall, failCause) { > > + if (this.currentConference.participants[removedCall.callIndex]) { > > + removedCall.isConference = false; > > + delete this.currentConference.participants[removedCall.callIndex]; > > + delete this.currentCalls[removedCall.callIndex]; > > Don't we need to check if this.currentCalls[removedCall.callIndex] exists > before? I don't think so. The caller of this function should make sure the function argument, i.e., 'removedCall' is valid. > @@ +3883,5 @@ > > + // in one task. > > + this._handleDisconnectedCall(removedCall); > > + } else { > > + delete this.currentCalls[removedCall.callIndex]; > > + if (failCause) { > > Who is going to assign 'failCause'? I don't see that in the patches... Should be in part 2 patches. I miss that part.
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8403780 -
Attachment is obsolete: true
Attachment #8403838 -
Flags: review?(htsai)
Reporter | ||
Comment 12•10 years ago
|
||
Comment on attachment 8403838 [details] [diff] [review] Part 2#3: Hangup pending outgoing call Review of attachment 8403838 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +3828,2 @@ > pendingOutgoingCall.failCause = GECKO_CALL_ERROR_UNSPECIFIED; > this._handleDisconnectedCall(pendingOutgoingCall); We need to delete pendingOutgoingCall here so that the if-clause in line 3841 could make sense. @@ +3840,5 @@ > } > + if (!pendingOutgoingCall && newCall.state === CALL_STATE_DIALING) { > + // Receive a new outgoing call but which is already hung up by user. > + if (DEBUG) this.context.debug("Hang up pending outgoing call"); > + this.hangUp({callIndex: newCall.callIndex}); Till now we haven't added this newCall to our map. this.hangUp will simply return without sending a request to modem.
Attachment #8403838 -
Flags: review?(htsai)
Reporter | ||
Comment 13•10 years ago
|
||
Comment on attachment 8403838 [details] [diff] [review] Part 2#3: Hangup pending outgoing call Review of attachment 8403838 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +3837,5 @@ > if (newCall.isVoice) { > if (newCall.isMpty) { > conferenceChanged = true; > } > + if (!pendingOutgoingCall && newCall.state === CALL_STATE_DIALING) { It is safer to check DIALING || ALERTING. Please do! :)
Assignee | ||
Comment 14•10 years ago
|
||
comment address
Attachment #8403838 -
Attachment is obsolete: true
Attachment #8403897 -
Flags: review?(htsai)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8403778 [details] [diff] [review] Part 1#2: Refactor: extract removeVoiceCall Restart the review. comment 1: Explained. it looks fine from my point of view. comment 2: Fixed in part 2 patch. So, nothing going to be changed in this patch.
Attachment #8403778 -
Flags: review?(htsai)
Reporter | ||
Updated•10 years ago
|
Attachment #8403778 -
Flags: review?(htsai) → review+
Reporter | ||
Comment 16•10 years ago
|
||
Comment on attachment 8403897 [details] [diff] [review] Part 2#4: Hangup pending outgoing call Review of attachment 8403897 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +1547,1 @@ > switch (call.state) { How about move this switch-clause into "sendHangUpRequest"? In this way, we don't need to check call.state first, then check "isBackgroundCall" later and these two mean the same thing. @@ +3850,5 @@ > } > + if (!pendingOutgoingCall && > + (newCall.state === CALL_STATE_DIALING || > + newCall.state === CALL_STATE_ALERTING)) { > + // Receive a new outgoing call but which is already hung up by user. nit: grammar, no "but" before "which"
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #16) > Comment on attachment 8403897 [details] [diff] [review] > Part 2#4: Hangup pending outgoing call > > Review of attachment 8403897 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/ril_worker.js > @@ +1547,1 @@ > > switch (call.state) { > > How about move this switch-clause into "sendHangUpRequest"? In this way, we > don't need to check call.state first, then check "isBackgroundCall" later > and these two mean the same thing. The switch case is not complete. In some call.state, we might do nothing. If we move the switch-clause into "sendHangUpRequest", chances are we will call the function but do not really send out the request. If we really want to avoid one additional check in "sendHangUpRequest", I'd like to separate it into two different functions. 1) sendHangUpRequest, 2) sendHangUpBackgroundRequest. > @@ +3850,5 @@ > > } > > + if (!pendingOutgoingCall && > > + (newCall.state === CALL_STATE_DIALING || > > + newCall.state === CALL_STATE_ALERTING)) { > > + // Receive a new outgoing call but which is already hung up by user. > > nit: grammar, no "but" before "which"
Reporter | ||
Comment 18•10 years ago
|
||
(In reply to Szu-Yu Chen [:aknow] from comment #17) > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #16) > > Comment on attachment 8403897 [details] [diff] [review] > > Part 2#4: Hangup pending outgoing call > > > > Review of attachment 8403897 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: dom/system/gonk/ril_worker.js > > @@ +1547,1 @@ > > > switch (call.state) { > > > > How about move this switch-clause into "sendHangUpRequest"? In this way, we > > don't need to check call.state first, then check "isBackgroundCall" later > > and these two mean the same thing. > > The switch case is not complete. Could you point me which state is missing in the switch case? I think we cover all. If you were saying "INCOMING" state, then it isn't there because we don't hangup an incoming call, we rejectCall instead. > In some call.state, we might do nothing. If > we move the switch-clause into "sendHangUpRequest", chances are we will call > the function but do not really send out the request. > > If we really want to avoid one additional check in "sendHangUpRequest", I'd > like to separate it into two different functions. 1) sendHangUpRequest, 2) > sendHangUpBackgroundRequest. > > > @@ +3850,5 @@ > > > } > > > + if (!pendingOutgoingCall && > > > + (newCall.state === CALL_STATE_DIALING || > > > + newCall.state === CALL_STATE_ALERTING)) { > > > + // Receive a new outgoing call but which is already hung up by user. > > > > nit: grammar, no "but" before "which"
Assignee | ||
Comment 19•10 years ago
|
||
Attachment #8403897 -
Attachment is obsolete: true
Attachment #8403897 -
Flags: review?(htsai)
Attachment #8404407 -
Flags: review?(htsai)
Reporter | ||
Comment 20•10 years ago
|
||
Comment on attachment 8404407 [details] [diff] [review] Part 2#5: Hangup pending outgoing call Review of attachment 8404407 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks :)
Attachment #8404407 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 21•10 years ago
|
||
Need update the test case. Without setting the pending outgoing call first, the new dialing call will be drop.
Attachment #8404470 -
Flags: review?(htsai)
Reporter | ||
Comment 22•10 years ago
|
||
Comment on attachment 8404470 [details] [diff] [review] Part 3: Fix related test case Review of attachment 8404470 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/tests/test_ril_worker_ssn.js @@ +36,5 @@ > this.number = number; > } > > Call.prototype = { > + state: CALL_STATE_ACTIVE, Please drop a comment to explain why this should set to _ACTIVE. Thank you.
Attachment #8404470 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 23•10 years ago
|
||
Attachment #8403778 -
Attachment is obsolete: true
Assignee | ||
Comment 24•10 years ago
|
||
Attachment #8404407 -
Attachment is obsolete: true
Assignee | ||
Comment 25•10 years ago
|
||
Attachment #8404470 -
Attachment is obsolete: true
Assignee | ||
Comment 26•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=75eb838d43c4
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 1.4 S5 (11apr)
Comment 27•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/b6fa56ead60e https://hg.mozilla.org/integration/b2g-inbound/rev/6ccdf1679557 https://hg.mozilla.org/integration/b2g-inbound/rev/49d3c6beb419
Keywords: checkin-needed
Comment 28•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b6fa56ead60e https://hg.mozilla.org/mozilla-central/rev/6ccdf1679557 https://hg.mozilla.org/mozilla-central/rev/49d3c6beb419
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 29•10 years ago
|
||
This is a follow-up of Bug 990467, which is already land on 1.3T. It's better to also land this one on 1.3T.
blocking-b2g: --- → 1.3T?
Assignee | ||
Comment 30•10 years ago
|
||
Attachment #8411541 -
Flags: review+
Assignee | ||
Comment 31•10 years ago
|
||
Attachment #8411542 -
Flags: review+
Assignee | ||
Comment 32•10 years ago
|
||
Attachment #8411543 -
Flags: review+
Comment 33•10 years ago
|
||
can we understand the user impact of not taking this patch for 1.3T? and also how risky is it to take the patch? thanks
Flags: needinfo?(szchen)
Assignee | ||
Comment 34•10 years ago
|
||
After (a) dialing a call, we are expecting to (b) receive a notification responded from modem side. In normal case, we should receive that notification immediately. Without this patch: 1. Gecko will send out a wrong command to modem and cause some problem if the user try to hang up the call between (a) and (b). 2. User could not hang up the call if (b) is missing. IMO, risk is low. In addition, the fix in this patch is also useful for handling another 1.3T bug (Bug 999334)
Flags: needinfo?(szchen)
Assignee | ||
Updated•10 years ago
|
Attachment #8411541 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8411542 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8411543 -
Attachment is obsolete: true
Assignee | ||
Comment 36•10 years ago
|
||
Attachment #8412427 -
Flags: review?(htsai)
Reporter | ||
Comment 37•10 years ago
|
||
REOPEN because we need part 4.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 38•10 years ago
|
||
Comment on attachment 8412427 [details] [diff] [review] Part 4: Fix hang up pending outgoing call Review of attachment 8412427 [details] [diff] [review]: ----------------------------------------------------------------- Looks good and work fine on real devices. As emulator doesn't support the advanced feature we need for this bug, please file a bug and let's discuss there how we could make it happen. Thank you!
Attachment #8412427 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 39•10 years ago
|
||
Attachment #8412427 -
Attachment is obsolete: true
Attachment #8412454 -
Flags: review+
Comment 40•10 years ago
|
||
triage: 1.3T+, taking this for other tarako blockers checkin-needed
blocking-b2g: 1.3T? → 1.3T+
status-b2g-v1.3T:
--- → affected
Flags: needinfo?(jcheng)
Keywords: checkin-needed
Assignee | ||
Comment 41•10 years ago
|
||
Remove checkin-needed. Testing on Try now. I will add back the flag if the result is good.
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Attachment #8411543 -
Attachment is obsolete: false
Assignee | ||
Updated•10 years ago
|
Attachment #8411542 -
Attachment is obsolete: false
Assignee | ||
Updated•10 years ago
|
Attachment #8411541 -
Attachment is obsolete: false
Assignee | ||
Comment 42•10 years ago
|
||
Attachment #8412486 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [p=2][ETA: 4/26]
Reporter | ||
Comment 43•10 years ago
|
||
Comment on attachment 8412486 [details] [diff] [review] (1.3T) [final] Part 4: Fix hang up pending outgoing call. r=hsinyi Review of attachment 8412486 [details] [diff] [review]: ----------------------------------------------------------------- Hi Aknow, I reviewed the whole process again and worried that we might miss some cases to set |this.hangUpPendingOutgoingCall| back to false. I got headache so kindly let me know if something wrong. :) Let's think about the below cases: 1) hanging up pendingMO & REQUEST_DIAL success & a new call in CLCC ==> covered in this patch 2) hanging up pendingMO & REQUEST_DIAL success & no new call in CLCC ==> missing? 3) hanging uppendingMO & REQUEST_DIAL error ==> missing? In 2) and 3), without rest this.hangUpPendingOutgoingCall, then next time when we try to make a call via STK, the STK call will be released on background, right?
Reporter | ||
Comment 44•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #43) > Comment on attachment 8412486 [details] [diff] [review] > (1.3T) [final] Part 4: Fix hang up pending outgoing call. r=hsinyi > > Review of attachment 8412486 [details] [diff] [review]: > ----------------------------------------------------------------- > > Hi Aknow, > I reviewed the whole process again and worried that we might miss some cases > to set |this.hangUpPendingOutgoingCall| back to false. I got headache so > kindly let me know if something wrong. :) > > Let's think about the below cases: > 1) hanging up pendingMO & REQUEST_DIAL success & a new call in CLCC ==> > covered in this patch > 2) hanging up pendingMO & REQUEST_DIAL success & no new call in CLCC ==> > missing? > 3) hanging uppendingMO & REQUEST_DIAL error ==> missing? > > In 2) and 3), without rest this.hangUpPendingOutgoingCall, then next time ^^^^^ typo: reset > when we try to make a call via STK, the STK call will be released on > background, right?
Assignee | ||
Comment 45•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #44) > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #43) > > Comment on attachment 8412486 [details] [diff] [review] > > (1.3T) [final] Part 4: Fix hang up pending outgoing call. r=hsinyi > > > > Review of attachment 8412486 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > Hi Aknow, > > I reviewed the whole process again and worried that we might miss some cases > > to set |this.hangUpPendingOutgoingCall| back to false. I got headache so > > kindly let me know if something wrong. :) > > > > Let's think about the below cases: > > 1) hanging up pendingMO & REQUEST_DIAL success & a new call in CLCC ==> > > covered in this patch > > 2) hanging up pendingMO & REQUEST_DIAL success & no new call in CLCC ==> > > missing? > > 3) hanging uppendingMO & REQUEST_DIAL error ==> missing? > > > > In 2) and 3), without rest this.hangUpPendingOutgoingCall, then next time > ^^^^^ typo: reset > > > when we try to make a call via STK, the STK call will be released on > > background, right? I think you are right. ni myself to fix it.
Flags: needinfo?(szchen)
Reporter | ||
Comment 46•10 years ago
|
||
(In reply to Szu-Yu Chen [:aknow] from comment #45) > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #44) > > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #43) > > > Comment on attachment 8412486 [details] [diff] [review] > > > (1.3T) [final] Part 4: Fix hang up pending outgoing call. r=hsinyi > > > > > > Review of attachment 8412486 [details] [diff] [review]: > > > ----------------------------------------------------------------- > > > > > > Hi Aknow, > > > I reviewed the whole process again and worried that we might miss some cases > > > to set |this.hangUpPendingOutgoingCall| back to false. I got headache so > > > kindly let me know if something wrong. :) > > > > > > Let's think about the below cases: > > > 1) hanging up pendingMO & REQUEST_DIAL success & a new call in CLCC ==> > > > covered in this patch > > > 2) hanging up pendingMO & REQUEST_DIAL success & no new call in CLCC ==> > > > missing? > > > 3) hanging uppendingMO & REQUEST_DIAL error ==> missing? > > > More accurate, the case is |hanging up pendingMO before REQUEST_DIAL & then REQUEST_DIAL error| The case is valid after bug 999334. Be sure to rebase on that properly! > > > In 2) and 3), without rest this.hangUpPendingOutgoingCall, then next time > > ^^^^^ typo: reset > > > > > when we try to make a call via STK, the STK call will be released on > > > background, right? > > I think you are right. ni myself to fix it. Thank you :)
Assignee | ||
Comment 47•10 years ago
|
||
Attachment #8412454 -
Attachment is obsolete: true
Attachment #8412486 -
Attachment is obsolete: true
Attachment #8413514 -
Flags: review?(htsai)
Flags: needinfo?(szchen)
Reporter | ||
Comment 48•10 years ago
|
||
Comment on attachment 8413514 [details] [diff] [review] Part 4#2: Fix hang up pending outgoing call Review of attachment 8413514 [details] [diff] [review]: ----------------------------------------------------------------- Please correct _removePendingOutgoingCall(): s/this._removeVoiceCall(pendingOutgoingCall, failCause); /this._removeVoiceCall(call, failCause); ::: dom/system/gonk/ril_worker.js @@ +224,5 @@ > this.v5Legacy = RILQUIRKS_V5_LEGACY; > this.cellBroadcastDisabled = RIL_CELLBROADCAST_DISABLED; > this.clirMode = RIL_CLIR_MODE; > + > + this.hangUpPendingOutgoingCall = false; Please define it in RilObject.prototype, and assign the default value in initRILState(). and s/hangUpPendingOutgoingCall/_hangUpPendingOutgoingCall/ Sorry for not point this out in the last review.
Attachment #8413514 -
Flags: review?(htsai)
Assignee | ||
Comment 49•10 years ago
|
||
Attachment #8413514 -
Attachment is obsolete: true
Attachment #8413633 -
Flags: review?(htsai)
Reporter | ||
Comment 50•10 years ago
|
||
Comment on attachment 8413633 [details] [diff] [review] Part 4#32: Fix hang up pending outgoing call Review of attachment 8413633 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8413633 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 51•10 years ago
|
||
Attachment #8413633 -
Attachment is obsolete: true
Attachment #8413653 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [p=2][ETA: 4/26] → [p=2][ETA: 4/28]
Assignee | ||
Comment 52•10 years ago
|
||
Attachment #8413669 -
Flags: review+
Assignee | ||
Comment 53•10 years ago
|
||
checkin-needed Master: part 4 only 1.3T: part 1 to 4 https://tbpl.mozilla.org/?tree=Try&rev=52f37f6a69fa
Keywords: checkin-needed
Reporter | ||
Comment 54•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/2b844dff91b3
Keywords: checkin-needed
Comment 55•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2b844dff91b3
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment 56•10 years ago
|
||
Re-adding checkin-needed for 1.3T parts 1-4. Do we care about auroroa/1.4?
Keywords: checkin-needed
Comment 57•10 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/64a555fcfb12 remote: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/2c2e64f98ec9 remote: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/815c44bd230d remote: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/97952535b72d
Updated•10 years ago
|
Keywords: checkin-needed
Comment 58•10 years ago
|
||
Hi Preeti, Per suggestion in https://bugzilla.mozilla.org/show_bug.cgi?id=990467#c53, also need to get approval for uplift the patches in this bug to v1.4. Since the fixes contain multiple patches, instead of clicking the patch one by one to set approval flag, allow me to ni? you here for the approval to uplift the patches to v1.4. Thanks.
Flags: needinfo?(praghunath)
Whiteboard: [p=2][ETA: 4/28] → [p=2][ETA: 4/28][1.4-approval-needed]
Comment 59•10 years ago
|
||
Also ni? Szu-Yu to check if the patch can be directly uplifted to v1.4.
Flags: needinfo?(szchen)
Assignee | ||
Comment 60•10 years ago
|
||
Attachment #8430584 -
Flags: review+
Flags: needinfo?(szchen)
Assignee | ||
Comment 61•10 years ago
|
||
Attachment #8430585 -
Flags: review+
Assignee | ||
Comment 62•10 years ago
|
||
Attachment #8430587 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Attachment #8430587 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8430585 -
Attachment is obsolete: true
Assignee | ||
Comment 63•10 years ago
|
||
Comment on attachment 8430584 [details] [diff] [review] (1.4) [final] Part 1: Refactor: Extract function of adding new voice call. r=hsinyi attached to wrong bug...
Attachment #8430584 -
Attachment is obsolete: true
Assignee | ||
Comment 64•10 years ago
|
||
Attachment #8430605 -
Flags: review+
Assignee | ||
Comment 65•10 years ago
|
||
Attachment #8430606 -
Flags: review+
Assignee | ||
Comment 66•10 years ago
|
||
Attachment #8430607 -
Flags: review+
Assignee | ||
Comment 67•10 years ago
|
||
Attachment #8430608 -
Flags: review+
Comment 68•10 years ago
|
||
Comment on attachment 8430605 [details] [diff] [review] (1.4) [final] Part 1: Refactor: extract removeVoiceCall. r=hsinyi NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Partner Blocker. This one is the follow up bug of 990467 which is critical bug since ECC is affected Testing completed: Yes, and developer already prepare the 1.4 patch Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: N/A
Attachment #8430605 -
Flags: approval-mozilla-b2g30?
Comment 69•10 years ago
|
||
Comment on attachment 8430606 [details] [diff] [review] (1.4) [final] Part 2: Hangup pending outgoing call. r=hsinyi NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Partner Blocker. This one is the follow up bug of 990467 which is critical bug since ECC is affected Testing completed: Yes, and developer already prepare the 1.4 patch Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: N/A
Attachment #8430606 -
Flags: approval-mozilla-b2g30?
Comment 70•10 years ago
|
||
Comment on attachment 8430607 [details] [diff] [review] (1.4) [final] Part 3: Fix related test case. r=hsinyi NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Partner Blocker. This one is the follow up bug of 990467 which is critical bug since ECC is affected Testing completed: Yes, and developer already prepare the 1.4 patch Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: N/A
Attachment #8430607 -
Flags: approval-mozilla-b2g30?
Comment 71•10 years ago
|
||
Comment on attachment 8430608 [details] [diff] [review] (1.4) [final] Part 4: Fix hang up pending outgoing call. r=hsinyi NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Partner Blocker. This one is the follow up bug of 990467 which is critical bug since ECC is affected Testing completed: Yes, and developer already prepare the 1.4 patch Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: N/A
Attachment #8430608 -
Flags: approval-mozilla-b2g30?
Updated•10 years ago
|
Attachment #8430605 -
Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Flags: needinfo?(praghunath)
Updated•10 years ago
|
Attachment #8430606 -
Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Updated•10 years ago
|
Attachment #8430607 -
Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Updated•10 years ago
|
Attachment #8430608 -
Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Comment 72•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/86f2cd4cb5b1 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/5092f1c8cc44 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/584b4a601157 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/84e6e8ab08f1
status-b2g-v1.3:
--- → unaffected
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
Whiteboard: [p=2][ETA: 4/28][1.4-approval-needed] → [p=2]
Target Milestone: 1.4 S5 (11apr) → 2.0 S1 (9may)
Updated•10 years ago
|
Whiteboard: [p=2] → [p=2][1.4-approval-needed]
Updated•10 years ago
|
Whiteboard: [p=2][1.4-approval-needed] → [p=2]
You need to log in
before you can comment on or make changes to this bug.
Description
•