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)

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-b2g:1.3T+, b2g-v1.3 unaffected, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S1 (9may)
blocking-b2g 1.3T+
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+
Details | Diff | Splinter Review
4.02 KB, patch
aknow
: review+
Details | Diff | Splinter Review
1.34 KB, patch
aknow
: review+
Details | Diff | Splinter Review
4.31 KB, patch
aknow
: review+
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
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
Attachment #8403146 - Flags: review?(htsai)
Attachment #8403147 - Flags: review?(htsai)
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)
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)
Attachment #8403146 - Attachment is obsolete: true
Attachment #8403778 - Flags: review?(htsai)
Attachment #8403147 - Attachment is obsolete: true
Attachment #8403780 - Flags: review?(htsai)
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)
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)
(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.
Attachment #8403780 - Attachment is obsolete: true
Attachment #8403838 - Flags: review?(htsai)
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)
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! :)
comment address
Attachment #8403838 - Attachment is obsolete: true
Attachment #8403897 - Flags: review?(htsai)
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)
Attachment #8403778 - Flags: review?(htsai) → review+
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"
(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"
(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"
Attachment #8403897 - Attachment is obsolete: true
Attachment #8403897 - Flags: review?(htsai)
Attachment #8404407 - Flags: review?(htsai)
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+
Attached patch Part 3: Fix related test case (obsolete) — Splinter Review
Need update the test case.
Without setting the pending outgoing call first, the new dialing call will be drop.
Attachment #8404470 - Flags: review?(htsai)
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+
Target Milestone: --- → 1.4 S5 (11apr)
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?
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)
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)
Will it be 1.3T+ ?
Flags: needinfo?(jcheng)
Attachment #8411541 - Attachment is obsolete: true
Attachment #8411542 - Attachment is obsolete: true
Attachment #8411543 - Attachment is obsolete: true
Attachment #8412427 - Flags: review?(htsai)
REOPEN because we need part 4.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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+
Attachment #8412427 - Attachment is obsolete: true
Attachment #8412454 - Flags: review+
triage: 1.3T+, taking this for other tarako blockers
checkin-needed
blocking-b2g: 1.3T? → 1.3T+
Flags: needinfo?(jcheng)
Keywords: checkin-needed
Remove checkin-needed. Testing on Try now. I will add back the flag if the result is good.
Keywords: checkin-needed
Attachment #8411543 - Attachment is obsolete: false
Attachment #8411542 - Attachment is obsolete: false
Attachment #8411541 - Attachment is obsolete: false
Whiteboard: [p=2][ETA: 4/26]
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?
(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?
(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)
(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 :)
Attachment #8412454 - Attachment is obsolete: true
Attachment #8412486 - Attachment is obsolete: true
Attachment #8413514 - Flags: review?(htsai)
Flags: needinfo?(szchen)
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)
Attachment #8413514 - Attachment is obsolete: true
Attachment #8413633 - Flags: review?(htsai)
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+
Whiteboard: [p=2][ETA: 4/26] → [p=2][ETA: 4/28]
Blocks: 999334
checkin-needed

Master: part 4 only
1.3T: part 1 to 4

https://tbpl.mozilla.org/?tree=Try&rev=52f37f6a69fa
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2b844dff91b3
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Re-adding checkin-needed for 1.3T parts 1-4.


Do we care about auroroa/1.4?
Keywords: checkin-needed
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]
Also ni? Szu-Yu to check if the patch can be directly uplifted to v1.4.
Flags: needinfo?(szchen)
Attachment #8430587 - Attachment is obsolete: true
Attachment #8430585 - Attachment is obsolete: true
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
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 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 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 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?
Attachment #8430605 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Flags: needinfo?(praghunath)
Attachment #8430606 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Attachment #8430607 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Attachment #8430608 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Whiteboard: [p=2] → [p=2][1.4-approval-needed]
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.