Closed Bug 990467 Opened 6 years ago Closed 6 years ago

[tarako] Cannot place ECC call when leaving shiedling box, and before getting service back.

Categories

(Firefox OS Graveyard :: RIL, defect, P1, blocker)

x86_64
Linux
defect

Tracking

(blocking-b2g:1.3T+, b2g-v1.3 wontfix, 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 --- wontfix
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: sku, Assigned: aknow)

References

Details

(Whiteboard: [priority])

Attachments

(11 files, 9 obsolete files)

759.76 KB, text/plain
Details
3.02 KB, patch
aknow
: review+
Details | Diff | Splinter Review
5.85 KB, patch
aknow
: review+
Details | Diff | Splinter Review
2.15 KB, patch
aknow
: review+
Details | Diff | Splinter Review
3.03 KB, patch
aknow
: review+
Details | Diff | Splinter Review
5.70 KB, patch
aknow
: review+
Details | Diff | Splinter Review
2.12 KB, patch
aknow
: review+
Details | Diff | Splinter Review
7.04 KB, patch
aknow
: review+
Details | Diff | Splinter Review
3.02 KB, patch
aknow
: review+
Details | Diff | Splinter Review
5.79 KB, patch
aknow
: review+
Details | Diff | Splinter Review
1.17 KB, patch
aknow
: review+
Details | Diff | Splinter Review
Attached file radio/device log.
STR:
1. Put device in shielding box. make sure device is out-of-service.
2. get device out of shield box. (make sure no service yet)
3. dial 112

Symptom:
1. ECC can not be made
2. no more call can be made

Recover:
Kill dialer AP to be able to make call again.

Log:
01-02 04:57:42.740    92   442 D RILC    : [0123]> DIAL (num=112,clir=0)
01-02 04:57:42.740    92   442 D RILC    : [0123]< DIAL
01-02 04:57:42.760    92   334 D RILC    : [0124]> GET_CURRENT_CALLS
01-02 04:57:42.760    92   334 D RILC    : [0124]< GET_CURRENT_CALLS }
01-02 04:57:42.780    92   333 D RILC    : [0125]> GET_CURRENT_CALLS
01-02 04:57:42.780    92   333 D RILC    : [0125]< GET_CURRENT_CALLS }
01-02 04:57:42.790    92   442 D RILC    : [0126]> GET_CURRENT_CALLS
01-02 04:57:42.790    92   442 D RILC    : [0126]< GET_CURRENT_CALLS }
Hi Hsinyi:
 Could you please help check this issue first?

It looks like dangling call object that cause no more MO call is available here.

Thanks!!
Shawn
Flags: needinfo?(htsai)
Attachment #8399863 - Attachment mime type: text/x-log → text/plain
Flags: needinfo?(htsai)
(In reply to shawn ku [:sku] from comment #1)
> Hi Hsinyi:
>  Could you please help check this issue first?
> 
> It looks like dangling call object that cause no more MO call is available
> here.
> 
> Thanks!!
> Shawn

Gecko is expecting a call coming from modem along with REQUEST_GET_CURRENT_CALLS once REQUEST_DIAL is reported as success. But from the log, we didn't get any subsequent calls from REQUEST_GET_CURRENT_CALLS even REQUEST_DIAL succeeded.
Hi Hsinyi:
 Yes, Unagi will report one call existed, and call disappear.
Thanks for your comment.


01-16 18:57:39.186   107   107 I Gecko   : TelephonyProvider: Dialing 118
01-16 18:57:39.186   111   189 D RILC    : UI --- RIL_REQUEST_DIAL (10) ---> RIL [RID 0, token id 388, data len 12]
01-16 18:57:39.236   111   196 D RILC    : UI <--- RIL_REQUEST_DIAL (10) Complete --- RIL [RID 0, Token 388, Success, Len 0 ]
01-16 18:57:39.366   111   189 D RILC    : Reply to RIL --> Number of calls : 1
01-16 18:57:47.394   111   189 D RILC    : Reply to RIL --> Number of calls : 0
01-16 18:57:47.424   111   189 D RILC    : Reply to RIL --> Number of calls : 0
(In reply to shawn ku [:sku] from comment #3)
> Hi Hsinyi:
>  Yes, Unagi will report one call existed, and call disappear.
> Thanks for your comment.
> 
> 
> 01-16 18:57:39.186   107   107 I Gecko   : TelephonyProvider: Dialing 118
> 01-16 18:57:39.186   111   189 D RILC    : UI --- RIL_REQUEST_DIAL (10) --->
> RIL [RID 0, token id 388, data len 12]
> 01-16 18:57:39.236   111   196 D RILC    : UI <--- RIL_REQUEST_DIAL (10)
> Complete --- RIL [RID 0, Token 388, Success, Len 0 ]
> 01-16 18:57:39.366   111   189 D RILC    : Reply to RIL --> Number of calls
> : 1
> 01-16 18:57:47.394   111   189 D RILC    : Reply to RIL --> Number of calls
> : 0
> 01-16 18:57:47.424   111   189 D RILC    : Reply to RIL --> Number of calls
> : 0

Hi Shawn,

So is partner going to address this on rild/modem?
I am not sure how partner's conclusion so far.
(I can only confirm AOSP will not have such issue due to pendingMO in gsmCallTracker.java)

Will keep you posted if I got any update.
blocking-b2g: --- → 1.3T?
(In reply to shawn ku [:sku] from comment #5)
> I am not sure how partner's conclusion so far.
> (I can only confirm AOSP will not have such issue due to pendingMO in
> gsmCallTracker.java)
> 
> Will keep you posted if I got any update.

Before we figure out how to handle the dangling call object, I'd like to understand why REQUEST_DIAL returns success in this case. Isn't it an expected behaviour that an error is returned if modem/rild isn't going to send gecko a call?  And seems the success of REQUEST_DIAL doesn't guarantee that there would be a call in the subsequent get_current_calls request?

In the meanwhile, I am going to look at AOSP gsmCallTracker.java. Thank you!
(In reply to shawn ku [:sku] from comment #0)
> Created attachment 8399863 [details]
> radio/device log.
> 
> STR:
> 1. Put device in shielding box. make sure device is out-of-service.
> 2. get device out of shield box. (make sure no service yet)
> 3. dial 112
> 
> Symptom:
> 1. ECC can not be made
> 2. no more call can be made
> 
> Recover:
> Kill dialer AP to be able to make call again.
> 

Unable to hang up this call, either?
I think the answer is yes but still want confirmation :P

> Log:
> 01-02 04:57:42.740    92   442 D RILC    : [0123]> DIAL (num=112,clir=0)
> 01-02 04:57:42.740    92   442 D RILC    : [0123]< DIAL
> 01-02 04:57:42.760    92   334 D RILC    : [0124]> GET_CURRENT_CALLS
> 01-02 04:57:42.760    92   334 D RILC    : [0124]< GET_CURRENT_CALLS }
> 01-02 04:57:42.780    92   333 D RILC    : [0125]> GET_CURRENT_CALLS
> 01-02 04:57:42.780    92   333 D RILC    : [0125]< GET_CURRENT_CALLS }
> 01-02 04:57:42.790    92   442 D RILC    : [0126]> GET_CURRENT_CALLS
> 01-02 04:57:42.790    92   442 D RILC    : [0126]< GET_CURRENT_CALLS }
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #7)
> (In reply to shawn ku [:sku] from comment #0)
> > Created attachment 8399863 [details]
> > radio/device log.
> > 
> > STR:
> > 1. Put device in shielding box. make sure device is out-of-service.
> > 2. get device out of shield box. (make sure no service yet)
> > 3. dial 112
> > 
> > Symptom:
> > 1. ECC can not be made
> > 2. no more call can be made
> > 
> > Recover:
> > Kill dialer AP to be able to make call again.
> > 
> 
> Unable to hang up this call, either?
> I think the answer is yes but still want confirmation :P
> 
> > Log:
> > 01-02 04:57:42.740    92   442 D RILC    : [0123]> DIAL (num=112,clir=0)
> > 01-02 04:57:42.740    92   442 D RILC    : [0123]< DIAL
> > 01-02 04:57:42.760    92   334 D RILC    : [0124]> GET_CURRENT_CALLS
> > 01-02 04:57:42.760    92   334 D RILC    : [0124]< GET_CURRENT_CALLS }
> > 01-02 04:57:42.780    92   333 D RILC    : [0125]> GET_CURRENT_CALLS
> > 01-02 04:57:42.780    92   333 D RILC    : [0125]< GET_CURRENT_CALLS }
> > 01-02 04:57:42.790    92   442 D RILC    : [0126]> GET_CURRENT_CALLS
> > 01-02 04:57:42.790    92   442 D RILC    : [0126]< GET_CURRENT_CALLS }

no, can not hangup the call.

there is no even dialing UI show up in this case.
the whole UI saty in dialer, and looks like no response by clicking "Green button"
(In reply to shawn ku [:sku] from comment #8)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #7)
> > (In reply to shawn ku [:sku] from comment #0)
> > > Created attachment 8399863 [details]
> > > radio/device log.
> > > 
> > > STR:
> > > 1. Put device in shielding box. make sure device is out-of-service.
> > > 2. get device out of shield box. (make sure no service yet)
> > > 3. dial 112
> > > 
> > > Symptom:
> > > 1. ECC can not be made
> > > 2. no more call can be made
> > > 
> > > Recover:
> > > Kill dialer AP to be able to make call again.
> > > 
> > 
> > Unable to hang up this call, either?
> > I think the answer is yes but still want confirmation :P
> > 
> > > Log:
> > > 01-02 04:57:42.740    92   442 D RILC    : [0123]> DIAL (num=112,clir=0)
> > > 01-02 04:57:42.740    92   442 D RILC    : [0123]< DIAL
> > > 01-02 04:57:42.760    92   334 D RILC    : [0124]> GET_CURRENT_CALLS
> > > 01-02 04:57:42.760    92   334 D RILC    : [0124]< GET_CURRENT_CALLS }
> > > 01-02 04:57:42.780    92   333 D RILC    : [0125]> GET_CURRENT_CALLS
> > > 01-02 04:57:42.780    92   333 D RILC    : [0125]< GET_CURRENT_CALLS }
> > > 01-02 04:57:42.790    92   442 D RILC    : [0126]> GET_CURRENT_CALLS
> > > 01-02 04:57:42.790    92   442 D RILC    : [0126]< GET_CURRENT_CALLS }
> 
> no, can not hangup the call.
> 
> there is no even dialing UI show up in this case.
> the whole UI saty in dialer, and looks like no response by clicking "Green
> button"

Hmmm... no Red button?
RILD sent 112 via channel1 but got NO CARRIER from channel0. does it cause something wrong? 

01-02 04:57:42.740    92   442 D AT      : Channel1: AT> ATD112;
01-02 04:57:42.740    92   267 D AT      : Channel0: AT< +ECIND: 0,2,1
01-02 04:57:42.740    92   267 D AT      : Channel0: AT< ^DSCI: 1,0,2,0,0,112,129
...
01-02 04:57:42.740    92   267 D AT      : Channel0: AT< NO CARRIER
Hi Sam:
 Please help reply Comment 10.

Thanks!!
Shawn
Flags: needinfo?(sam.hua)
Hi

03-29 15:08:23.441    92   337 D AT      : Channel1: AT> ATD112;
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< +ECIND: 0,2,1 
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< ^DSCI: 1,0,2,0,0,112,129  
//Call indication when call status changed.  1,0,2 means call id =1, mo call, dialing,
//it will trigger UNSOL_RESPONSE_CALL_STATE_CHANGED to Gecko
03-29 15:08:23.441    92   208 D RILC    : [UNSL]< UNSOL_RESPONSE_CALL_STATE_CHANGED 
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< *SPCSC: 1,0,,"112",129,1
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< ^ORIG: 1,9
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< *SPCSC: 1,3,,"112",129,1
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< +SIND: 5,1
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< +SIND: 5,1
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< ^DSCI: 1,0,6,0,0,112,129,,34
//Call indication when call status changed.  1,0,6 means call id =1, mo call, call ended,
//it will trigger UNSOL_RESPONSE_CALL_STATE_CHANGED to Gecko
03-29 15:08:23.441    92   208 D RILC    : [UNSL]< UNSOL_RESPONSE_CALL_STATE_CHANGED 
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< *SPCSC: 1,20,,,,1,67,34
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< NO CARRIER
//NO CARRIER is a unsolicited at command , it will trigger UNSOL_RESPONSE_CALL_STATE_CHANGED to Gecko
03-29 15:08:23.441    92   208 D RILC    : [UNSL]< UNSOL_RESPONSE_CALL_STATE_CHANGED 
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< +ECEER: 34,1
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< +SIND: 6,1,34
03-29 15:08:23.441    92   208 D AT      : Channel0: AT< ^CEND: 1,,104,34
//when call ended, Modem use it to report end cause and call during time.
//104,34 means network disconnect the call,the cause is 34.
03-29 15:08:23.441    92   208 D RIL     : The last call fail cause: 34
03-29 15:08:23.441    92   208 D AT      : Channel1: AT< OK
//the solicited response for ATD
03-29 15:08:23.441    92   337 D RILC    : [0143]< DIAL
Flags: needinfo?(sam.hua)
So "Channel0: AT< NO CARRIER" is a unsolicited response and RILC will trigger UNSOL_RESPONSE_CALL_STATE_CHANGED.

it is okay in this log.
Hi there, 

Seems we don't have other option rather than creating a pendingMO as AOSP does. But please let us know if you see a simple workaround so that we don't rush out a patch in 1.3T.

The proposal is:
1) when REQUEST_DIAL returns success, we create a pendingMO call in ril_worker.js. This is slightly different from AOSP which creates a pendingMO right after querying REQUEST_DIAL.
2) Then unsol_call_state_change should come. we query REQUEST_GET_CURRENT_CALLS and compare the CLCC response with pendingMO. If CLCC is empty, we drop the pendingMO manually and notify gaia. If CLCC isn't empty, we'd make sure we get the right mapping b/w CLCC and pendingMO. 

Kindly let us know if any comments. Thanks!
Assignee: nobody → szchen
Attachment #8400582 - Flags: feedback?(htsai)
Attachment #8400583 - Flags: feedback?(htsai)
Shawn,
Here are some quick WIP patches. I don't know whether the patches indeed resolve the problem (not yet tested). If you have time, could you help me test them on real network (only modified js files)?
Flags: needinfo?(sku)
Haven't read through the WIP but going to. Thanks for the prompt WIP, Aknow!!!
Comment on attachment 8400582 [details] [diff] [review]
(WIP) Part 1: refactor: add new voice call

Review of attachment 8400582 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ -3819,5 @@
>          conferenceChanged = true;
>        }
>      }
>  
> -    // Go through any remaining calls that are new to us.

Looks better to keep this comment.

@@ +3827,1 @@
>            this.currentConference.participants[newCall.callIndex] = newCall;

I'd expect this line is moved to _addNewVoiceCall() as well, in order to avoid the chance that someone forgot to add a new Mpty call into the conference map.
Attachment #8400582 - Flags: feedback?(htsai) → feedback+
I am trying to run some test locally... let me verify the patches first

(In reply to Szu-Yu Chen [:aknow] from comment #17)
> Shawn,
> Here are some quick WIP patches. I don't know whether the patches indeed
> resolve the problem (not yet tested). If you have time, could you help me
> test them on real network (only modified js files)?
Hi Aknow:
 1. Please help providing me 1.3t patch. (I cannot merge TelephonyProvider to my local build.)
 2. I tried the patch at local.

The symptom is weird when hangup.


01-01 01:17:51.700    99   348 D RILC    : [0254]> DIAL (num=112,clir=0)
01-01 01:17:51.700    99   348 D AT      : Channel1: AT> ATD112;
01-01 01:17:51.720    84   315 I Gecko   : RIL Worker: aknow pendingOutgoingCall = true
01-01 01:17:51.730    99   488 D RILC    : [0255]< GET_CURRENT_CALLS }
01-01 01:17:51.730    99   349 D RILC    : [0256]< GET_CURRENT_CALLS }
01-01 01:17:51.730    99   348 D RILC    : [0257]< GET_CURRENT_CALLS }
01-01 01:17:51.990    84   315 I Gecko   : RIL Worker: aknow pendingOutgoingCall = false
01-01 01:17:55.170    99   488 D RILC    : [0258]> HANGUP (-1)
01-01 01:17:55.190    99   488 D RILC    : [0258]< HANGUP fails by E_GENERIC_FAILURE
Flags: needinfo?(sku)
1. Fix a bug.
2. Currently, I use "busyError" so that gaia UI could show a prompt. (only for test)
Attachment #8400583 - Attachment is obsolete: true
Attachment #8400583 - Flags: feedback?(htsai)
Attachment #8401071 - Flags: feedback?(htsai)
Comment on attachment 8401071 [details] [diff] [review]
(WIP) Part 2#2: add temp outgoing call

Review of attachment 8401071 [details] [diff] [review]:
-----------------------------------------------------------------

The patch generally goes toward the right direction. And it works on a real device though I couldn't really test the exactly same scenario as Shawn originally reported.

Below please find my comments, thanks.
1) Please remember to remove [1]. And we need to try the DEBUG build before landing.
[1] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/Telephony.cpp?from=Telephony.cpp&case=true#535
2) Also remember to take care of TelephonyCall::mLive

::: dom/system/gonk/ril_worker.js
@@ +3707,2 @@
>      let conferenceChanged = false;
>      let clearConferenceRequest = false;

add: let pendingOutgoingCall = null;

@@ +3710,5 @@
>      // Go through the calls we currently have on file and see if any of them
>      // changed state. Remove them from the newCalls map as we deal with them
>      // so that only new calls remain in the map after we're done.
>      for each (let currentCall in this.currentCalls) {
> +      if (currentCall.callIndex == OUTGOING_PLACEHOLDER_CALL_INDEX) {

pendingOutgoingCall = currentCall;

@@ +3829,5 @@
>        }
>      }
>  
> +    // Go through any remaining calls that are new to us.
> +    if (this.pendingOutgoingCall) {

if (pendingOutgoingCall)

@@ +5288,5 @@
>      this.sendChromeMessage(options);
> +
> +    // Add a pending call.
> +    this.context.debug("Aknow: add a pending outgoing call.");
> +    this.pendingOutgoingCall = true;

Not necessary to have this flag. We will anyway run through the currentCalls in _processCalls and at that time we know if we have a pendingOutgoingCall or not.

::: dom/telephony/gonk/TelephonyProvider.js
@@ +833,5 @@
>        call.isMergeable = aCall.isMergeable != null ?
>                           aCall.isMergeable : true;
> +
> +      // Add placeholder.
> +      if (!this.pendingOutgoingCall &&

We don't really need this flag. We could just check |this._currentCalls[aClientId][OUTGOING_PLACEHOLDER_CALL_INDEX]|.

@@ +842,5 @@
> +      }
> +
> +      // Get the actual call. Remove the placeholder.
> +      if (this.pendingOutgoingCall &&
> +          call.callIndex != OUTGOING_PLACEHOLDER_CALL_INDEX) {

It's safer to have one more check -- call.isOutgoing
Attachment #8401071 - Flags: feedback?(htsai)
Attachment #8400582 - Attachment is obsolete: true
Attachment #8401149 - Flags: review?(htsai)
Attachment #8401071 - Attachment is obsolete: true
Attachment #8401150 - Flags: review?(htsai)
Attachment #8401152 - Flags: review?(htsai) → review+
Attachment #8401149 - Flags: review?(htsai) → review+
Attachment #8401150 - Attachment is obsolete: true
Attachment #8401150 - Flags: review?(htsai)
Attachment #8401156 - Flags: review?(htsai)
Comment on attachment 8401156 [details] [diff] [review]
Part 2#2: Add pending outgoing call

Review of attachment 8401156 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +3830,5 @@
>      }
>  
> +    if (pendingOutgoingCall) {
> +      // We don't get a successful call for pendingOutgoingCall.
> +      if (!newCalls || Object.keys(newCalls).length === 0) {

Should we consider the case for multi-calls?
ex: 1 active + MO call, but we hit the scenario that Dial success but not that dailing call via GET_CURRENT_CALLS!!
Comment on attachment 8401156 [details] [diff] [review]
Part 2#2: Add pending outgoing call

Review of attachment 8401156 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=me with comment addressed. Thank you!

::: dom/system/gonk/ril_worker.js
@@ +3832,5 @@
> +    if (pendingOutgoingCall) {
> +      // We don't get a successful call for pendingOutgoingCall.
> +      if (!newCalls || Object.keys(newCalls).length === 0) {
> +        this.context.debug("Disconnect pending outgoing call");
> +        let call = this.currentCalls[OUTGOING_PLACEHOLDER_CALL_INDEX];

Should be fine to use "pendingOutgoingCall" instead of creating a new "call" variable.
Attachment #8401156 - Flags: review?(htsai) → review+
(In reply to shawn ku [:sku] from comment #28)
> Comment on attachment 8401156 [details] [diff] [review]
> Part 2#2: Add pending outgoing call
> 
> Review of attachment 8401156 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +3830,5 @@
> >      }
> >  
> > +    if (pendingOutgoingCall) {
> > +      // We don't get a successful call for pendingOutgoingCall.
> > +      if (!newCalls || Object.keys(newCalls).length === 0) {
> 
> Should we consider the case for multi-calls?
> ex: 1 active + MO call, but we hit the scenario that Dial success but not
> that dailing call via GET_CURRENT_CALLS!!

The patch should have covered the case you mention. :)
The code visits the calls in our current map first, then goes through any remaining calls new to us.

I've verified this case. That said, on Buri, dial out the 3rd call while there are two calls existing. The 3rd REQUEST_DIAL returns success but CLCC doesn't have it. And the patch works!
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #30)
> (In reply to shawn ku [:sku] from comment #28)
> > Comment on attachment 8401156 [details] [diff] [review]
> > Part 2#2: Add pending outgoing call
> > 
> > Review of attachment 8401156 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: dom/system/gonk/ril_worker.js
> > @@ +3830,5 @@
> > >      }
> > >  
> > > +    if (pendingOutgoingCall) {
> > > +      // We don't get a successful call for pendingOutgoingCall.
> > > +      if (!newCalls || Object.keys(newCalls).length === 0) {
> > 
> > Should we consider the case for multi-calls?
> > ex: 1 active + MO call, but we hit the scenario that Dial success but not
> > that dailing call via GET_CURRENT_CALLS!!
> 
> The patch should have covered the case you mention. :)
> The code visits the calls in our current map first, then goes through any
> remaining calls new to us.
> 
> I've verified this case. That said, on Buri, dial out the 3rd call while
> there are two calls existing. The 3rd REQUEST_DIAL returns success but CLCC
> doesn't have it. And the patch works!

okay, I missed "!newCalls" check, thanks!!
triage: 1.3T+, partner blocker
blocking-b2g: 1.3T? → 1.3T+
Priority: -- → P1
Whiteboard: [priority]
Severity: normal → blocker
Attachment #8401222 - Attachment description: (1.3T) [final] Part 2: Refactor: extract function of adding new voice call. r=hsinyi → (1.3T) [final] Part 1: Refactor: extract function of adding new voice call. r=hsinyi
Attachment #8401223 - Attachment description: (1.3T) [final] Part 3: Add pending outgoing call. r=hsinyi → (1.3T) [final] Part 2: Add pending outgoing call. r=hsinyi
If review+, please land it.
Flags: needinfo?(styang)
Flags: needinfo?(jcheng)
checkin-needed
Keywords: checkin-needed
Flags: needinfo?(styang)
Flags: needinfo?(jcheng)
(In reply to Szu-Yu Chen [:aknow] from comment #36)
> Created attachment 8401221 [details] [diff] [review]
> (1.3T) Part 0: Add currentCall in TelephonyProvider

Has this been reviewed?
Flags: needinfo?(szchen)
Whiteboard: [priority] → [priority][MP_Blocker]
Comment on attachment 8401221 [details] [diff] [review]
(1.3T) Part 0: Add currentCall in TelephonyProvider

Review of attachment 8401221 [details] [diff] [review]:
-----------------------------------------------------------------

Almost right, just a small thing, but I want to review it again to play it safer. Please ask for review again once the comment has been addressed. Thanks.

::: dom/telephony/gonk/TelephonyProvider.js
@@ +422,1 @@
>        aListener.enumerateCallStateComplete();

enumerateCallStateComplete() should be out of the outter for-loop. This is notified when enumeration on *all* clients is done.
Attachment #8401221 - Flags: review?(htsai)
Fixed. Thank you.
Attachment #8401221 - Attachment is obsolete: true
Attachment #8402444 - Flags: review?(htsai)
Flags: needinfo?(szchen)
Comment on attachment 8402444 [details] [diff] [review]
(1.3T) Part 0#2: Add currentCall in TelephonyProvider

Review of attachment 8402444 [details] [diff] [review]:
-----------------------------------------------------------------

Good!  Please make sure 1.3T patches really work well before landing.
Attachment #8402444 - Flags: review?(htsai) → review+
checkin needed for 1.3T
There are 4 patches, from part 0 to part 3.
Keywords: checkin-needed
Whiteboard: [priority][MP_Blocker] → [priority]
Depends on: 996421
Hi Preeti,

Since the fixes contain multiple patches, ni? you here for the approval to uplift the patches to v1.4.
Flags: needinfo?(praghunath)
Remember to uplift Bug 993255 to v1.4 as well. That is a followup bug of this one.
Whiteboard: [priority] → [priority][approval-v1.4]
Whiteboard: [priority][approval-v1.4] → [priority][1.4-approval-needed]
Also ni? Szu-Yu to check if the patch can be directly uplifted to v1.4.
Flags: needinfo?(szchen)
We might also need this one after uplifting those two bugs to v1.4.
https://bugzilla.mozilla.org/show_bug.cgi?id=999334
Flags: needinfo?(itsay)
Comment on attachment 8430592 [details] [diff] [review]
(1.4) [final] Part 1: Refactor: Extract function of adding new voice 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. Also, it's 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 #8430592 - Flags: approval-mozilla-b2g30?
Flags: needinfo?(itsay)
Comment on attachment 8430593 [details] [diff] [review]
(1.4) [final] Part 2: Add 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: 
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Partner Blocker. Also, it's 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 #8430593 - Flags: approval-mozilla-b2g30?
Comment on attachment 8430594 [details] [diff] [review]
(1.4) [final] Part 3: DOM: Re-order code to avoid multi-thread issue. 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. Also, it's 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 #8430594 - Flags: approval-mozilla-b2g30?
Comment on attachment 8430592 [details] [diff] [review]
(1.4) [final] Part 1: Refactor: Extract function of adding new voice call. r=hsinyi

taking in 1.4
Attachment #8430592 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Flags: needinfo?(praghunath)
Attachment #8430593 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Attachment #8430594 - Flags: approval-mozilla-b2g30? → approval-mozilla-b2g30+
Add whiteboard for query purpose
Whiteboard: [priority] → [priority][1.4-approval-needed]
Blocks: 994411
Whiteboard: [priority][1.4-approval-needed] → [priority]
You need to log in before you can comment on or make changes to this bug.