Closed Bug 860614 Opened 11 years ago Closed 11 years ago

[Dialer] The dialer still display on screen without caller number after the incomming call finished by caller.

Categories

(Firefox OS Graveyard :: Gaia::Dialer, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:-, b2g18+ affected, b2g18-v1.0.1 affected)

RESOLVED WORKSFORME
blocking-b2g -
Tracking Status
b2g18 + affected
b2g18-v1.0.1 --- affected

People

(Reporter: askeing, Assigned: arthurcc)

References

Details

Attachments

(2 files)

The dialer still display on screen without caller number after the incomming call finished by caller.

### ENV:
Unagi
Gaia:     c614b3f3c956dc1e1adf93cf4cf41511ce75de80
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18/rev/423f7851bdb5
BuildID   20130410070209
Version   18.0

### STR:
0. Perpare another phone.
1. Using another phone call your device.
2. After your device's screen on, hang up quickly before your device ring.

### Expected:
1. The notification with missing call will be displayed on screen.

### Actual:
1. The screen still display the incomming call page without caller number.

### Youtube:
https://www.youtube.com/watch?v=4uJUJLsknjY
blocking-b2g: --- → leo?
Triage 4/12 - Leo+ per attached screenshot is considerable user impact.
blocking-b2g: leo? → leo+
Assignee: nobody → arthur.chen
The root cause is that the call ends before we open call screen and there is no chance to close it. The patch makes sure the there are available calls before opening call screen. Etienne, please help review the change, thanks!
Attachment #737810 - Flags: review?(etienne)
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

We went back and forth on this a lot actually (we had something similar earlier in the project)...

The issue is, a |telephony.calls.length| greater than 0 in dialer.js doesn't guarantee you that |telephony.calls.length| will be greater than 0 in the call screen window scope once the call screen window is opened.

So we're just delaying the opening of the call screen here (which we should try to avoid) but the call could still be hanged up while the call screen is loading.

We should probably discuss this in more details with platform RIL people too, because AFAIK the telephony-new-call system message is not guaranteed to happen before the |oncallschanged| (and if it happens after we will miss the call here.)
Attachment #737810 - Flags: review?(etienne) → review-
(In reply to Etienne Segonzac (:etienne) from comment #3)
> Comment on attachment 737810 [details]
> Link to https://github.com/mozilla-b2g/gaia/pull/9196
> 
> We went back and forth on this a lot actually (we had something similar
> earlier in the project)...
> 
> The issue is, a |telephony.calls.length| greater than 0 in dialer.js doesn't
> guarantee you that |telephony.calls.length| will be greater than 0 in the
> call screen window scope once the call screen window is opened.
> 
> So we're just delaying the opening of the call screen here (which we should
> try to avoid) but the call could still be hanged up while the call screen is
> loading.
> 
> We should probably discuss this in more details with platform RIL people
> too, because AFAIK the telephony-new-call system message is not guaranteed
> to happen before the |oncallschanged| (and if it happens after we will miss
> the call here.)

We can't guarantee the timing of the telephony-new-call system message and the *first* |oncallschanged| event. However, when initiating mozTelephony, the DOM will actively ask RIL for enumerating calls. If there exists a call in RIL, then an additional |oncallschanged| event is gonna be dispatched to this newly initiated object.
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

Based on comment 4, after we initiating mozTelephony, we will receive a |oncallschanged| event if there exist a call. Otherwise, the event never fires and there is no need to open the call screen in this case.

Etienne, could you help review the change again? Thanks!
Attachment #737810 - Flags: review- → review?(etienne)
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

Hey Arthur,

I did some more testing because we've landed a few patches already this week regarding the call screen edge cases.

And I can't reproduce the bug anymore on gaia master :)

I'll let you confirm this before closing worsforme but I did several tests, with the dialer app already launched or not (to get different timings) and I'm not able to reproduce the bug.
Attachment #737810 - Flags: review?(etienne)
Hi Etienne,
I tested it using https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-b2g18-unagi-eng/2013/04/2013-04-18-07-02-05/ and gaia master. I can still reproduce it easily. The tip is hanging up at the time when the buttons light up. 

Note that it does not show empty call screen anymore, but it shows empty blue block in the center with answer and hang up buttons on the bottom. Please let me know if you are able to reproduce it. :)
Flags: needinfo?(etienne)
Testing on an unagi, b2g18 from today, gaia master...

Here is a quick video in case I'm not doing the STR properly:
https://www.dropbox.com/s/fwupwr7ohnw67yy/quick-hangup.mp4
Flags: needinfo?(etienne)
Hang up a little bit earlier then you will see things as the video:

https://www.dropbox.com/s/i5izhs7kc9q0ekg/quick%20hangup.mp4
My patch does not solve the problem. Need to propose another one....
Adding qawanted.

Just spent 10 minutes trying really hard to reproduce it without any success.
I hanged up just before the call gets through, just after, couldn't reproduce.

I wonder if it might actually have a dependency on the Operator/the ril.

(Tried with a French Orange sim and an Spanish Movistar sim)
Keywords: qawanted
I was able to reproduce this about 2 times out of the 10+ times that I tried.. Not exactly sure what I did different when I did repro it :/ 
I used the same build ID as comment 0 

Going to try on today's build now
Keywords: qawanted
Using today's build, I saw the same thing as the video in comment 9. By hanging up after one dialing tone on the call sending device. 

Unagi, Build ID: 20130422070204
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/dfa6bdc5e6da
Gaia: 3cdab45a783efadf9b97decf46bd15b83fb91ed8
I was able to reproduce this issue as shown on video in comment 9 on v1.0.1 as well

Unagi Build ID: 20130422070206
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3a67efe71e11
Gaia: b750757662209dad81942df13f9997d684e0d9b1
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

I found that if the call ends after we open the call screen but before it is loaded, the call screen is never closed. Simply check if there are available calls after the call screen is loaded. 

I've updated the patch. Etienne, please help review it, thanks.
Attachment #737810 - Flags: review?(etienne)
Hardware: All → ARM
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

Sadly, this exact code was _removed_ at some point because of bug 823958.
And then moved with a postMessage in dialer.js that does pretty much the same check.

On some phones you will always hit the exit conditions because telephony.calls will be empty at first.

Hsin-Yi will know the details better.
Attachment #737810 - Flags: review?(etienne)
I've noticed the 'dialer_oncallschanged' function in dialer.js and found it does not cover all of the cases (i.e. the case described in comment 15). At the time of user hanging up the call, the function checks if the call screen is loaded. If not, we will never have a chance to close the call screen. That's why I added a check in the 'setup' function in oncall.js. As you suggested, adding the check might lead to exit conditions always because telephony.calls will be empty, which is definitely a problem. 

If we add the logic of waiting for call.length before opening the call screen back as my previous patch, then we can make sure that when we see telephony.calls not empty, there was a hanged up call. What do you think?
(In reply to Arthur Chen [:arthurcc] from comment #17)
> I've noticed the 'dialer_oncallschanged' function in dialer.js and found it
> does not cover all of the cases (i.e. the case described in comment 15). At
> the time of user hanging up the call, the function checks if the call screen
> is loaded. 


This makes me think:
In bug 828882 I'm introducing a formal "ready" event for the call screen, can you try moving the telephony.calls.length check in dialer.js to happen when we receive this "ready" message and see if it helps?

Thanks!
The idea of this solution is to check the existence of the incoming call after the call screen loaded and ready. The check can be moved to dialer.js as you suggested and it works well!

You mentioned that on some phones it always hit exit conditions. Did you mean the case of calls.length becoming one from zero after the call screen loaded? If yes, we still need to add logics as the second paragraph of comment 17.
(In reply to Arthur Chen [:arthurcc] from comment #19)
> The idea of this solution is to check the existence of the incoming call
> after the call screen loaded and ready. The check can be moved to dialer.js
> as you suggested and it works well!
> 

Cool!

> You mentioned that on some phones it always hit exit conditions. Did you
> mean the case of calls.length becoming one from zero after the call screen
> loaded? If yes, we still need to add logics as the second paragraph of
> comment 17.

I meant the case of calls.length being equal to 0 the first time we call it in a new window.
bug 828882 landed by the way :)
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

Changed to doing a final check when the call screen is ready. Hsin-yi has confirmed that if we do the check in dialer.js, we can ensure that a if there was an incoming call, when we observe calls.length equals 0, the call was hanged up already.
Attachment #737810 - Flags: review?(etienne)
Comment on attachment 737810 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/9196

So... with this patch I can't receive any incoming calls :(
We always hit the |telephony.calls.length == 0| and exit on incoming calls.
(my tests were done on an inari)

I think that given the low reproduction rate we should wait for bug 823958 and fix this bug properly once and for all then.
Attachment #737810 - Flags: review?(etienne) → review-
Weird... Looks like it really highly depends on phones. It works well on my unagi.
Depends on: 823958
Not a regression from v1.0.1, not a partner required blocker, not leo+. Please nominate for approval if/when resolved.
blocking-b2g: leo+ → -
tracking-b2g18: --- → +
While investigating another bug we found one of the key piece to reproduce this bug: you need to close the dialer app first.

See, the issue is not that the call is ended while the call screen is loading, we handle this quite nicely.

The issue is that the call is ended between the moment the telephony-new-call system message is sent and the moment the dialer app is loaded. So the dialer app never gets a chance to observe the call and react accordingly, all we see is the telephony-new-call system message.

So this is _definitely_ blocked by bug 823958.
I forgot to mention that I always reproduce the issue when put the phone in standby mode. 

Adding a handler to |telephony.oncallschanged| triggers a refresh in gecko, so we can get correct call infos in the handler. By doing this when handling telephony-new-call system message, we can ensure that we call |openCallScreen| only when there is an existing call.

It seems like bug 823958 will not get landed soon. We can try if the workaround described above works.
Patch updated. Now we check the existence of available calls before |openCallScreen| and when call screen is ready.
(In reply to Arthur Chen [:arthurcc] from comment #27)
> It seems like bug 823958 will not get landed soon. We can try if the
> workaround described above works.

I really want to avoid adding anymore workarounds to this code (especially for a non blocking bug).

We already filed/fixed _tens_ of bugs related to the underlying platform issue, let's push to do this correctly.
So this shouldn't happen anymore on a gecko with bug 823958 landed.
Feel free to close if it's not reproducible anymore.
Keywords: qawanted
Issue repros however a lot less then on comment 8 and 9.  View what I am talking about on video.


Unagi Build ID: 20130604070205
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/997cdbf5d012
Gaia: 5534304aee934055f08f21ce93261ba2a596516a


https://www.dropbox.com/s/o6gbsyl8hoqghz8/2013-06-04%2015.51.07.mp4
Keywords: qawanted
Although may need to be tested once again when bug 879343 has been fixed.
This issue does not reproduce anymore. When calling the B2G device, the Dialer does not hang displayed after hanging up the call on the sending device after one dial tone. Resolving as WFM.


Unagi Build ID: 20130610230207
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/78de618c071a
Gaia: ea18de80fd04110756becaed214656642388401d
Platform Version: 18.0
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: