Closed Bug 999356 Opened 8 years ago Closed 8 years ago

[tarako] [specific to some operator] fail to show the charged balance on disconnecting the call after MO call

Categories

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

Other
Other
defect
Not set
normal

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

RESOLVED FIXED
2.0 S1 (9may)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: angelc04, Assigned: etienne)

Details

(Whiteboard: [sprd300433][partner-blocker])

Attachments

(4 files, 1 obsolete file)

Attached file main.log
Steps to reproduce
--------------------------------------------------------------------
a) Power ON 6821A with Vodafone operator in SIM 1 slot and Reliance in Sim 2 slot.
b) Make MO call from Sim 1.
c) Connect the call.
d) Disconnect the call.
   --> 6821 fail to show the charged balance on disconnecting the call after MOC and message comes "Session Expired".

This is specific to Vodafone operator.
Attachment #8410152 - Attachment mime type: application/x-gzip → application/plain
Attachment #8410152 - Attachment mime type: application/plain → text/plain
Attachment #8410152 - Attachment mime type: text/plain → application/x-gzip
Shawn Ku and I checked the log, here are two possible stories:

1. Comms app received 'ussd-received' system message and render charged balance in the background. However Comms app get killed by LMK since it is in background. The charged balance will never have chances to show.

2. After MO call, user goes back to homescreen. Comms app get killed by LMK. And 'ussd-received' system message came at the moment, and Comms app brought back up again but it was still in background. If this is the case, user can see charged balance when he/she launch comms app again.
https://bugzilla.mozilla.org/attachment.cgi?id=8410152 is coming from https://github.com/dwi2/gaia/compare/mozilla-b2g:v1.3t...partner300433

This debug patch is to make sure we really get 'ussd-received' system message and handle it in Gaia. We're asking partner to get more debug log with the patch at their site.
Component: General → Gaia::Dialer
blocking-b2g: 1.3T? → 1.3T+
Tzu-lin, seem like you are working on this, assign to you for now. 
if this is POVB, please ni? James. thanks
Assignee: nobody → tzhuang
After some experiment(by faking network generated USSD), we find out that the timing sequence of Comms app OOM-killed and receiving 'ussd-received' matters.
If:
1. After MO call, comms app goes to background, receives 'ussd-received' but gets killed at the moment. In the case users will never see the network-initiated USSD string.
2. After MO call, comms app goes to background and gets killed by OOM, then we receives 'ussd-received'. Dialer will render a dialog of the USSD string in the background. Users could see the USSD string only if he/she launch dialer again.

So my suggestion of short-term solution on v1.3t is that: we could use Notification API to display USSD string on receiving 'ussd-received' in comms app in addition to USSD dialog in dialer. 
No matter comms app get killed before or after 'ussd-received', users still could see USSD string. But this USSD notification might be annoying in the case of mobile-initiated USSD (User enter special MMI command, say *101#, in dialer).

However we need UX input on this decision.

Hi Carrie,
Is it acceptable to send notification when receiving network-initiated USSD?
Flags: needinfo?(cawang)
Hi 
Per discussion with Tzu-Lin, I'd suggest always providing notification to users instead of displaying two different types of messages. In this way, we can make sure the behavior is consistent. Thanks!
Flags: needinfo?(cawang)
since tzu-lin's actively working on this, set target milestone to 2.0 S1
Target Milestone: --- → 2.0 S1 (9may)
Status: NEW → ASSIGNED
Component: Gaia::Dialer → Gaia::E-Mail
I had a WIP for it. https://github.com/mozilla-b2g/gaia/pull/18728
However, while I am working on the WIP, I think this may not be a proper solution, because:
1. There are two kinds of USSD: one needs user input and the other without user input. I can't put USSD string that needs user input in notification.
2. Even in the case of USSD without user input, the USSD text string could be truncate if it is too long.

I think we still need to figure out another proper solution for this.
Yang, please land this WIP patch in sprd_patch, and ask our Indian QA to catch new log.
Flags: needinfo?(yang.zhao)
(In reply to James Zhang from comment #10)
> Yang, please land this WIP patch in sprd_patch, and ask our Indian QA to
> catch new log.
Push on gerrit.Please review an merge it.
Flags: needinfo?(yang.zhao)
(Changing component back to Dialer.  I assume :timdream moved this to email accidentally.)
Component: Gaia::E-Mail → Gaia::Dialer
(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #9)
> I had a WIP for it. https://github.com/mozilla-b2g/gaia/pull/18728
> However, while I am working on the WIP, I think this may not be a proper
> solution, because:
> 1. There are two kinds of USSD: one needs user input and the other without
> user input. I can't put USSD string that needs user input in notification.
> 2. Even in the case of USSD without user input, the USSD text string could
> be truncate if it is too long.
> 
> I think we still need to figure out another proper solution for this.

Hi,Tzu-LIN
   After the WIP landed ,it seems met another issue.Please see http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=309147 for more details.
Flags: needinfo?(tzhuang)
As Tzu-LIN mentioned in http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=309147  ,I already reverted the WIP.
unassign because I don't know what I can contribute here
Assignee: tzhuang → nobody
Hi Yang,

It seems the WIP is not good. 

But we can't reproduce it at our site, could you help to verify the occurrence rate on latest build (without WIP)?
Flags: needinfo?(tzhuang)
Flags: needinfo?(yang.zhao)
(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #16)
> Hi Yang,
> 
> It seems the WIP is not good. 
> 
> But we can't reproduce it at our site, could you help to verify the
> occurrence rate on latest build (without WIP)?

I've NI our tester in Indian to reproduce it .I'll let you know the result as soon as the tester give me response back.
Flags: needinfo?(yang.zhao)
ni? Etienne
is it something you can help looking into ? Thanks
Flags: needinfo?(etienne)
(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #16)
Below is the result *WITH* your WIP ,just for you information.I will ask the tester reproduce the issue once the version without the WIP released to the tester:


Hi Yang,

I have tested the issue on version FIREFOXOS_V1.3_W14.19.1_SP6821.
Issue still exist on this version.
Probability:- 3/5
S.No:
76571-1 12:45pm 
94318-1 12:46pm
109162-1 12:46pm
Logfile:- 300433_Need_Info_1
Logpath:- Same as earlier.
Gabriele, Hsin-Yi, do you think we could work around the issue by requesting a priority wake lock when this USSD is displayed?

Looks like the use-case is pretty time constraint as the user wants to know how much was charged just after the call (as opposed to coming back later to see it).
Flags: needinfo?(htsai)
Flags: needinfo?(gsvelto)
Flags: needinfo?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #20)
> Gabriele, Hsin-Yi, do you think we could work around the issue by requesting
> a priority wake lock when this USSD is displayed?

Yes, holding a high-priority wakelock until the USSD is displayed should fix the issue. We should just make sure that this doesn't regress anything else (holding the lock might cause some other app to be killed instead).
Flags: needinfo?(gsvelto)
(In reply to Etienne Segonzac (:etienne) from comment #20)
> Gabriele, Hsin-Yi, do you think we could work around the issue by requesting
> a priority wake lock when this USSD is displayed?
> 
> Looks like the use-case is pretty time constraint as the user wants to know
> how much was charged just after the call (as opposed to coming back later to
> see it).

Could someone give a work around here?
Flags: needinfo?(james.zhang)
(In reply to Gabriele Svelto [:gsvelto] from comment #21)
> (In reply to Etienne Segonzac (:etienne) from comment #20)
> > Gabriele, Hsin-Yi, do you think we could work around the issue by requesting
> > a priority wake lock when this USSD is displayed?
> 
> Yes, holding a high-priority wakelock until the USSD is displayed should fix
> the issue. We should just make sure that this doesn't regress anything else
> (holding the lock might cause some other app to be killed instead).

Sounds fine to me!
Flags: needinfo?(htsai)
Flags: needinfo?(james.zhang)
Whiteboard: [tarako_only][sprd300433]
Whiteboard: [tarako_only][sprd300433] → [tarako_only][sprd300433][partner-blocker]
It's time to workaround.
Attached patch POC of the wakeLock option (obsolete) — Splinter Review
Wake locking until the app is displayed should be enough I think.

Is this patch helping with the issue?
Attachment #8418708 - Flags: feedback?(gsvelto)
Flags: needinfo?(pcheng)
james, could you please help find someone to help verify this patch? We don't have a Vodafone operator SIM card.
Flags: needinfo?(pcheng) → needinfo?(james.zhang)
Hi! Etienne,

Since you are working on this case. Do you mind to take it?

--
Keven
Flags: needinfo?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #25)
> Created attachment 8418708 [details] [diff] [review]
> POC of the wakeLock option
> 
> Wake locking until the app is displayed should be enough I think.
> 
> Is this patch helping with the issue?

Have you tested it?Did it affect?

Since I don't have a Vodafone operator ,I have to land the patch in the build version so that the tester could get it and verify this issue.

@Shawn,  Sam told me that you know how to verify it without a Vodafone operator SIM card. Could you help to verify it ?Thanks a lot!
Flags: needinfo?(sku)
Hi yang:
 I cannot repro. this issue locally (even I can fake network ussd), It would be better for your FT member to trial.

Note: 
My previous trial was
fake network USSD (w/ 5sec dealy) after call end, and kill communication ap manually.


Thanks!!
Shawn
Flags: needinfo?(sku)
Yang will help to verify.
Flags: needinfo?(james.zhang)
Comment on attachment 8418708 [details] [diff] [review]
POC of the wakeLock option

Yes, this should definitely help though I currently have no way of testing this.
Attachment #8418708 - Flags: feedback?(gsvelto) → feedback+
(In reply to Keven Kuo [:kkuo] from comment #27)
> Hi! Etienne,
> 
> Since you are working on this case. Do you mind to take it?

Happy to take the bug but I'll need help verifying it.
Assignee: nobody → etienne
Flags: needinfo?(etienne)
Attached file Gaia PR agains master
Attachment #8418708 - Attachment is obsolete: true
Attachment #8420090 - Flags: review?(gsvelto)
Comment on attachment 8420090 [details] [review]
Gaia PR agains master

LGTM with just one modification I forgot when you asked for feedback. It's good practice when you grab the wakelock to fire a timeout that releases it after some time (say 30s). This guarantees that the lock will be released if something funky happens and it's not released via the normal flow.
Attachment #8420090 - Flags: review?(gsvelto) → review+
This should land on master and 1.3t.

The comments are addressed, I'll be out of the office soon but feel free to land on a green travis.
Whiteboard: [tarako_only][sprd300433][partner-blocker] → [sprd300433][partner-blocker]
Landed on master
https://github.com/mozilla-b2g/gaia/commit/cad08e4e4380f3179e611c0c094baf92c8f36e90
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Our QA has verified this patch, please land it on v1.3t.
Flags: needinfo?(yang.zhao)
(In reply to Etienne Segonzac (:etienne) from comment #36)
> Landed on master
> https://github.com/mozilla-b2g/gaia/commit/
> cad08e4e4380f3179e611c0c094baf92c8f36e90

Hi, the attachment 8418708 [details] [diff] [review] is verrified on Tarako.And I find the patch on master you landed have some difference from the attachment 8418708 [details] [diff] [review].And I'd like to know which patch should I landed on v1.3t.Looking forward to your reply.Thank you very much!
Flags: needinfo?(etienne)
(In reply to James Zhang from comment #37)
> Our QA has verified this patch, please land it on v1.3t.

See last comment,already landed on v1.3t.
Flags: needinfo?(yang.zhao)
Triage: this is related to low memory issue. No need in v1.4.
You need to log in before you can comment on or make changes to this bug.