Build Info: -------------------------------------------------------- Gaia 3f1d8332d127f1d6bc0fdbb08146ce1deb0efbc0 Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/5da76152c2bd BuildID 20140421164001 Version 28.1 ro.build.version.incremental=eng.cltbld.20140421.201353 ro.build.date=Mon Apr 21 20:14:01 EDT 2014 Steps to reproduce: -------------------------------------------------------- 1. Launch 'SMS', and then create a new message 2. Tap on the clip icon on the top right for attachment 3 [review]. Select 'Camera' 4. Switch to 'Video' and then start to take a video 5. Make a phone call to the test device while taking video 6. Wait for the call screen to show up Actual results: -------------------------------------------------------- 1. About 2s later, the call screen shows up without the correct wallpaper and incoming call info. It shows a black screen instead of wallpaper. 2. After the call screen shows up, it takes about 5s for the wallpaper and incoming call info to show up. 3. In total, it takes about 7s for full call screen to show up. Pls refer to the STR-video attached. Additional info: ------------------------------------------------------------ Sometimes a black screen shows up for about 2s before call screen appears.

ni? Etienne/Rik

Simplifying the test: Open the camera only, without calling from the SMS app, reproduces this problem. If we do camera preview, then the call screen shows up relatively fast. If we do video recording, then we see the problem. I could be that recording slows down the call screen.

It takes approx 2sec to open the callscreen following the STR on the current upstream/v1.3t. Might need to revisit if we move forward with bug 999478.

This will push the foreground app to background on incoming call in an early stage. It seems to improve in my local test (I don't really like the idea that we couple telephone and priority manager).

(In reply to Cervantes Yu from comment #5) > This will push the foreground app to background on incoming call in an early > stage. It seems to improve in my local test (I don't really like the idea > that we couple telephone and priority manager). How much improvement are we talking about? I'm also not very keen in coupling the process priority manager to the telephony code... It's a lot of added complexity and it really doesn't belong there. I'd like to figure out why switching process priorities takes too long in this situation and what can be done to improve that instead. Now, if the improvement is large and we can't find another way I could f+ this but as it is I'd really prefer not to add more ad-hoc solutions but tackle the general issue instead (speed of process adjustments during these situations).

ni? Cyu/Gabriele, i wonder if one of you can take this bug? Thanks

Cervantes, I can take this and investigate the root issue if you don't mind. I've already got a WIP patch to measure process priority change delay in this condition so I'd start from there.

(In reply to Gabriele Svelto [:gsvelto] from comment #6) > (In reply to Cervantes Yu from comment #5) > > How much improvement are we talking about? I'm also not very keen in It improves a lot. The call screen and the contact information shows up as if the phone were not recording the video. The price for that is we are almost sure that the message app is killed because we pushed it to background. What I observed is that on incoming call, the message app (where the camera activity runs in) still has foreground priority. I think this is the main cause for the slowdown. Video recording has foreground priority and sends lots of IPC messages to the chrome process, slowing down other activities even we already moved the incoming call screen to the system app. There is a bug in priority management: app A calls activity B. Both A and B has foreground priority. On incoming call, we only push B into background. A still has foreground priority. This is the root cause we need to fix. If the fix still cannot make the call show up quickly enough, then we can take the hack here.

(In reply to Cervantes Yu from comment #9) > (In reply to Gabriele Svelto [:gsvelto] from comment #6) > > (In reply to Cervantes Yu from comment #5) > > > > How much improvement are we talking about? I'm also not very keen in > > > It improves a lot. The call screen and the contact information shows up as > if the phone were not recording the video. The price for that is we are > almost sure that the message app is killed because we pushed it to > background. > > What I observed is that on incoming call, the message app (where the camera > activity runs in) still has foreground priority. I think this is the main > cause for the slowdown. Video recording has foreground priority and sends > lots of IPC messages to the chrome process, slowing down other activities > even we already moved the incoming call screen to the system app. > > There is a bug in priority management: app A calls activity B. Both A and B > has foreground priority. On incoming call, we only push B into background. A > still has foreground priority. This is the root cause we need to fix. If the > fix still cannot make the call show up quickly enough, then we can take the > hack here. https://github.com/mozilla-b2g/gaia/pull/18626 Could you try this patch? It should put all window to background right away when call comes.

(In reply to Alive Kuo [:alive][NEEDINFO!][God bless Taiwan.] from comment #10) > https://github.com/mozilla-b2g/gaia/pull/18626 > Could you try this patch? It should put all window to background right away > when call comes. Unfortunately, it doesn't work.

Typo in the patch: --- a/apps/system/js/dialer_agent.js +++ b/apps/system/js/dialer_agent.js @@ -187,7 +187,7 @@ }; DialerAgent.prototype._openCallScreen = function da_openCallScreen() { - window.dispatchEvent(new CustomEven('callscreewillnopen')); + window.dispatchEvent(new CustomEvent('callscreenwillopen')); var callScreen = this._callScreen; var timestamp = new Date().getTime(); After fixing the typo, we see the message app go to background perceivable (and remain alive) on incoming call, but it makes not much difference.

According to :alive, we might kill the running app on incoming call, effectively the same to the proposed WIP, but simpler.

The review-ready patch for 1.3t.

Taking this, will try both patches ASAP.

OK, I've finally had some time to test this. I've tried both attachment 8411775 [details] [diff] [review] and the PR in comment 10 with the fix from comment 12 included. I get pretty much the same behavior for both patches: the original STR cannot be reproduced with either patch attached and the callscreen always shows up with the proper background already shown. However I've noticed that both patches might be masking an issue in the process priority manager that fails to lower the CPU nice value of the foreground app: I'm now testing the fix for bug 994560, we might need to uplift that too if it helps.

I re-test it on build 20140427014001, and upload a video. Here is the result: 1. A black screen shows up for about 2s before call screen appears. 2. Most of the time call screen shows up with the correct wallpaper, but still no incoming call info. It takes about 3 seconds for the call info to show up after call screen appears. 3. Rest of the time call screen and call info can show up at the same time. Build Info: ----------------------------------------------------------------- Gaia 4dd06db93607f55694953fe60751eddb4bbb5afc Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/db579b1c75ed BuildID 20140427014001 Version 28.1 ro.build.version.incremental=eng.cltbld.20140427.054628 ro.build.date=Sun Apr 27 05:46:39 EDT 2014

(In reply to Bingqing Li from comment #17) > It takes about 3 seconds for the call info to show up after call screen > appears. That's a separate issue and it's being tracked in bug 998241, let's focus on the wallpaper (and related priority issue) here.

Comment on attachment 8411775 [details] [diff] [review] Early push the foreground app to background on incoming call. Review of attachment 8411775 [details] [diff] [review]: ----------------------------------------------------------------- I've done some further testing and I've realized that even this patch while improving the overall behavior is not actually fixing the real problem: the real problem is that since the callscreen has been moved into the main process the process priority manager is not taking it into account. So what happens is that the messages app is considered as a background perceivable application with a nice value of 7 instead of being pushed back to 18. This is because this condition doesn't find the callscreen app: http://hg.mozilla.org/mozilla-central/file/86dce59c3d16/dom/ipc/ProcessPriorityManager.cpp#l969 In the case of this patch what happens is that when the call comes the process is given the lowest nice values, this allows the callscreen to come up earlier but then the priority is raised again as the logic is trying to reschedule the task anew. It took me a while to figure this out, now we have to find a proper solution though :-(

(In reply to Gabriele Svelto [:gsvelto] from comment #18) > (In reply to Bingqing Li from comment #17) > > It takes about 3 seconds for the call info to show up after call screen > > appears. > > That's a separate issue and it's being tracked in bug 998241, let's focus on > the wallpaper (and related priority issue) here. I got it. Just want to keep you informed of the behavior on the latest build. And in comment 17, my point is that wallpaper seems fine on build 20140427014001, but I don't know why.

This is a WIP patch that ensures the wake-locks held by the main process are taken into account when computing priorities for child processes. I have to give it some more testing before it's finalized.

This patch addresses all the issue I've found while testing, properly downgrading the CPU priority of child processes when the main process is holding a high-priority wakelock. I've also modified the existing unit-tests to cover the code that was causing this issue. Fabrice, can you do this review? Kyle has been doing most of the reviews in this area lately but he's on PTO and I'm unsure who else can review these type of changes.

Comment on attachment 8415867 [details] [diff] [review] [PATCH] Consider the wake-locks held by the main process when computing priorities Review of attachment 8415867 [details] [diff] [review]: ----------------------------------------------------------------- lgtm, but someone needs to document all this stuff seriously.

(In reply to Fabrice Desré [:fabrice] from comment #23) > lgtm, but someone needs to document all this stuff seriously. Thanks for the review! There's been a bit of discussion on this part of the code and I was asked to take over it which I'll be doing soon; one of the things that needs to be done is documenting how this works and how it interacts with memory management. Ideally this should go into the FxOS architecture documentation (see bug 939415 but I should open more). Some parts of this are in dire need of a good refactoring but right now I have too many things on my hands so in the meantime we have to keep it working as best as we can. BTW here's the try run: https://tbpl.mozilla.org/?tree=Try&rev=c41dbff72624

This issue is verified fixed in today's Tarako build 1.3t Environmental Variables: Device: Tarako 1.3t BuildID: 20140502014001 Gaia: a8e0ff550de08e58e4bf75af3cecf175b9b71e70 Gecko: 71790bf476cb Version: 28.1 Firmware Version: sp6821