To save more memory to handle a new call, we can play music in system app and change music to a controller. Thus we can kill music app when we receive a call and resume music from system app after phone call.

For tarako, Patrick has done some experiments in bug 973596 and proved that the approach in comment 0 can improve the launch time of dialer, also saves memory and potentially can fix bug 983539 because both the audio elements(dialer and music) are in system, system should be able to control them base on the telephony statuses.

triage: 1.3T+ to resolve bug 973596

The issue with doing that is that this can't work with 3rd party music apps as they can't use the inter-app communication api.

ni Paul for any security issues that may arise.

This bug is short-term solution for service worker is not ready yet. For the long term, we should move to service worker.

(In reply to Fabrice Desré [:fabrice] from comment #4) > ni Paul for any security issues that may arise. Thanks Fabrice. I can't really think of too much that might be a risk here. I'll flag for a sec-review just in case but it should be straight-forward.

Can't we simply move the dialer oncall.html page in-process instead of playing music from the system app ? If oncall.html is moved to a separate app, with the <iframe mozbrowser mozapp="callscreen.manifest"> preloaded in the system app but with src="", then we can load the call screen very fast by just changing the src of the preloaded iframe. There should not be a lot of additional memory used neither as most of the platform things are already loaded in the main process and the additional memory used when there is an incoming call can be retrieved while setting src="" after the call. There is also normally some communications between the call screen window and the dialer window using postMessage, those could be replaced by IAC. This way the call screen should load instantly (like the ringtone that has been moved into the system app) and it will benefit all devices, not only Tarako.

Etienne pointed out some reasons why we don't move oncall.html into system app for 1.3t in https://bugzilla.mozilla.org/show_bug.cgi?id=980984#c15. I think we will need to solve the same problems if we move oncall page to a separate app. I would like to n-i Etienne to see if this is doable for 1.3t.

ni? Rik for comment 10 as Etienne's on PTO

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9) > Can't we simply move the dialer oncall.html page in-process instead of > playing music from the system app ? > > If oncall.html is moved to a separate app, with the <iframe mozbrowser > mozapp="callscreen.manifest"> preloaded in the system app but with src="", > then we can load the call screen very fast by just changing the src of the > preloaded iframe. > > There should not be a lot of additional memory used neither as most of the > platform things are already loaded in the main process and the additional > memory used when there is an incoming call can be retrieved while setting > src="" after the call. > > There is also normally some communications between the call screen window > and the dialer window using postMessage, those could be replaced by IAC. > > This way the call screen should load instantly (like the ringtone that has > been moved into the system app) and it will benefit all devices, not only > Tarako. Thanks Vivien, these information really helps. I was not involved from the beginning so I am not sure if Patrick has tried the preloaded approach before or not, then I went to him and he should be doing some tests on it, but first let me describe what problems we are looking into: [1]. The ringtone(ringer) and music(content) audio channels follow the audio competing policy but resulted in wrong behaviours, like two channels will be mixed together(bug 983539). [2]. The dialer launches too long if music is playing in the background(bug 973596). [3]. Music app might still to be killed by LMK no matter it's playing in the background or not. I think the approach in comment 9 could probably solve [2], but [1] and [3] will still happen unless we do some tricks on it. And what we try to achieve is to keep the song playing in the background, even the music app is killed by LMK[3], also, if the incoming call rings, we can probably force the music app closed by system app to get more memory then speed up the launch time of dialer then fix [2], and since the real audio element is in system app, the system app can pause the audio element to prevent [1], so that's why we thought putting the audio element in system app then communicates with music app via IAC might be a possible solution for tarako, though I can imagine there are lots of efforts to enable it. But if the preloaded approach can save enough memory for tarako, we might have chance to fix [2] and [3] at the same time, and leave [1] to the workaround patch in bug 983539.

(In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #10) > Etienne pointed out some reasons why we don't move oncall.html into system > app for 1.3t in https://bugzilla.mozilla.org/show_bug.cgi?id=980984#c15. I > think we will need to solve the same problems if we move oncall page to a > separate app. > > I would like to n-i Etienne to see if this is doable for 1.3t. It's not clear to me that it can not be fixed. Once the call screen opens, it can send an IAC message to the dialer to wake it up if it is not opened, and then use this communication channel to exchange the necessary informations, like headset, bluetooth and call log.

(In reply to Dominic Kuo [:dkuo] from comment #12) > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9) > > Can't we simply move the dialer oncall.html page in-process instead of > > playing music from the system app ? > > > > If oncall.html is moved to a separate app, with the <iframe mozbrowser > > mozapp="callscreen.manifest"> preloaded in the system app but with src="", > > then we can load the call screen very fast by just changing the src of the > > preloaded iframe. > > > > There should not be a lot of additional memory used neither as most of the > > platform things are already loaded in the main process and the additional > > memory used when there is an incoming call can be retrieved while setting > > src="" after the call. > > > > There is also normally some communications between the call screen window > > and the dialer window using postMessage, those could be replaced by IAC. > > > > This way the call screen should load instantly (like the ringtone that has > > been moved into the system app) and it will benefit all devices, not only > > Tarako. > > Thanks Vivien, these information really helps. I was not involved from the > beginning so I am not sure if Patrick has tried the preloaded approach > before or not, then I went to him and he should be doing some tests on it, > but first let me describe what problems we are looking into: > > [1]. The ringtone(ringer) and music(content) audio channels follow the audio > competing policy but resulted in wrong behaviours, like two channels will be > mixed together(bug 983539). > [2]. The dialer launches too long if music is playing in the background(bug > 973596). > [3]. Music app might still to be killed by LMK no matter it's playing in the > background or not. > > I think the approach in comment 9 could probably solve [2], but [1] and [3] > will still happen unless we do some tricks on it. And what we try to achieve > is to keep the song playing in the background, even the music app is killed > by LMK[3], also, if the incoming call rings, we can probably force the music > app closed by system app to get more memory then speed up the launch time of > dialer then fix [2], and since the real audio element is in system app, the > system app can pause the audio element to prevent [1], so that's why we > thought putting the audio element in system app then communicates with music > app via IAC might be a possible solution for tarako, though I can imagine > there are lots of efforts to enable it. > > But if the preloaded approach can save enough memory for tarako, we might > have chance to fix [2] and [3] at the same time, and leave [1] to the > workaround patch in bug 983539. The approach would likely helps for [2] on all devices. If there is a separate patch for [1] that's perfect. For [3] there is likely a few things we can do: - Does the music app is killed but not the homescreen ? If yes, it probably means that we need to think again about the adjusted OOM score for the homescreen app (adjusted by mozapptype="homescreen") when an application is playing some music. - If the headset/bluetooth code is replicated in the call screen app instead of beeing kept into the dialer then it means that the call screen won't need to open the dialer app at startup, reducing the memory impact. The call log history filling can be done by listening the telephony-call-ended system message and reading the LAST_FAIL_CAUSE from Ril (in fact it may solve some of the issue with the call log history today, which misses some entries if the call terminate too early). If we can do that I feel like the memory impact of the call screen will be greatly reduce, and could help to not kill the current app, music or not. To me it worth trying this approach, as the delayed call screen is an issue not only on Tarako but on other devices as well.

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #14) > The approach would likely helps for [2] on all devices. If there is a > separate patch for [1] that's perfect. Yes, and music app does not need to do anything. > For [3] there is likely a few things we can do: > - Does the music app is killed but not the homescreen ? If yes, it probably > means that we need to think again about the adjusted OOM score for the > homescreen app (adjusted by mozapptype="homescreen") when an application is > playing some music. I guess Patrick has done some similar adjustment before but not sure about it, while he is trying the preloaded approach, I will see if I can help to make some adjustment. > - If the headset/bluetooth code is replicated in the call screen app > instead of beeing kept into the dialer then it means that the call screen > won't need to open the dialer app at startup, reducing the memory impact. > The call log history filling can be done by listening the > telephony-call-ended system message and reading the LAST_FAIL_CAUSE from Ril > (in fact it may solve some of the issue with the call log history today, > which misses some entries if the call terminate too early). > > If we can do that I feel like the memory impact of the call screen will be > greatly reduce, and could help to not kill the current app, music or not. > > To me it worth trying this approach, as the delayed call screen is an issue > not only on Tarako but on other devices as well. Agree, and Patrick and I are applying these approaches on tarako and see if we can gain more memory.

Okay, after discussed with Patrick again, we think there are still rooms we can improve before we actually move the audio element into the system app, and the first candidate is to add the restoring capability(like bug 961349) for the music app, so that once music app is killed or crashed, it can be restored to where it was closed. With this ability, system app(or gecko) can just kill the unnecessary apps(including music) when dialer needs to be launched, this can speed up the launch time of dialer for the first step(still dailer needs to think a way to reduce the startup time). And after the call ends, communications is closed, if music was playing in the background but killed, system has kept this knowledge in the media playback widget, so system can re-launch music and ask music to play via IAC, just like the user tap on the playpause button on lock screen or utility tray. So the flow of how we suspend and resume the background music during the call are: 1. When gecko or system app receive the incoming telephony signal, |pause| the music via IAC to fix [1](or just let the next step to kill music). 2. Kill the unnecessary apps and music should be one of them, this should speed up the launch time of dialer, so partially fix [2]. 3. After the call ends and communications closed, re-launch music app then send |play| via IAC to resume the music. 4. Music should restore its ui and receive the |play| command to resume the song. For [3] I think we can avoid it by adjusting the OOM score like Vivien said. Patrick and I are going to implement this proposal and hope it will bring us some good news, update later.

System app can read event.detail.reason to check if the frame is killed because incoming call, and decide if the frame need to be relaunched after phone call.

Comment on attachment 8397636 [details] [diff] [review] Patch: Kill all app when call is incoming, and let system app know why this app is killed Review of attachment 8397636 [details] [diff] [review]: ----------------------------------------------------------------- That's brutish and makes me sad. But yeah, the code should work. That needs to be behind a pref in any case.

(In reply to Fabrice Desré [:fabrice] from comment #18) > That's brutish and makes me sad. But yeah, the code should work. That needs > to be behind a pref in any case. I will add a pref for this and will continue to find if there's a better way to improve the launch time of dialer.

This is the music WIP and besides Patrick's patch in attachment 8397636 [details] [diff] [review], we will also need a system patch to complete this bug, and I am working on both parts of music and system.

I think MMS/Email should use the same solution.

Comment on attachment 8397636 [details] [diff] [review] Patch: Kill all app when call is incoming, and let system app know why this app is killed I don't really want to do this ... but the code looks fine.

I'd like to see if Bug 989594 is enough before doing that.

Comment on attachment 8397636 [details] [diff] [review] Patch: Kill all app when call is incoming, and let system app know why this app is killed s/Critial/Critical/

This is the system part WIP, I am going to test it with the music WIP and Patrick's patch, see how many ms does these patches improve the dialer's launch time.

Vivien convinced me to give Comment 9 a shot. Clearly none of the patches here are ideal, so we might as well see the pros and cons of each hack :) The patch is rebased on 1.3t. It's just duplicating the dialer app in a callscreen app. And adding a small amount of code in the system app to open it properly as an attention screen. + some orthogonal stuffs regarding a costly transitionend listener. Apparently the memory footprint is pretty good (~ +4MB while in a call). If we want to go forward, we still need: * to replace the attention screen -> app window communications with IAC * to use the telephony-call-ended system message to insert in the call log

Comment on attachment 8399318 [details] System WIP: kill/relaunch music app when telephony state changes I have merged the system WIP to attachment 8398423 [details].

Comment on attachment 8398423 [details] System + Music: implement Media App Agent and State Manager to restore music app from where it was closed Jim, You probably need to read the comments first to know what's going on here, this is a tarako patch that modified both system and music apps for speeding up the dialer's launch time, it's not finished yet but as I have tested it on tarako, it did save time for the dialer so I think I should let you know this WIP first, since if these changes does work, I will try to finish it and request review from you later, would you please take a look on the music part?(if you have time, maybe also the system part because you also wrote the media playback widget) thanks. Alive, Since this patch also modified the system app, I think we better have you to look on the changes to see if it's too risky, though I haven't confirmed with Patrick that it's gecko will kill the music app, or system will do it when the incoming call comes, but would you please take a look on the system part first and give us some feedback? thanks.

Comment on attachment 8398423 [details] System + Music: implement Media App Agent and State Manager to restore music app from where it was closed This is ok if it's only going onto 1.3T, but if we expect this to land on master, I think it needs more work.

Add Jesse, please also track FM background case.

Dominic, could you provide an ETA on your patch? e.g. X days + review. Thanks.

We now play fm radio in mediaserver. Sometimes FM app is killed by LMK when run into background, but fm radio sound is still playing. Do we need to add fm controller in system app like music in this case?

FM is very import for Indian open market, peipei, do you need file a new bug for FM?

Hi! James, I don't think FM has such issue. But please give it a test for verification before submit a case. Thanks -- Keven

(In reply to Etienne Segonzac (:etienne) from comment #26) > Created attachment 8399537 [details] [diff] [review] > WIP : in-process dedicated callscreen app Etienne, would you file a new bug for creating dedicated callscreen app? Let's keep this bug track work on music. I think we would need both. Thanks!

let's use Bug 990003 to track Etienne's dialer work

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #31) > Dominic, could you provide an ETA on your patch? e.g. X days + review. > > Thanks. I should wrap up my patch today and hopefully Jim can review my patch tomorrow, and if everything looks good, ETA should be 4/9. But while I was testing my patch with attachment 8397636 [details] [diff] [review], I found when an incoming call comes, gecko kills all the apps except system app, and if: 1. The dialer launches before the homescreen's re-launch, then that's great, it does improve the launch time of the dialer. 2. The dialer launches after the homescreen's re-launch, then the launch time of the dialer will still be the same, seems like the homescreen has used the memory which the dialer was going to use. We have to think a way to defer the homescreen's re-launch to after the dialer's launch, or we are unable to guarantee the gecko patch works 100% every time.

Comment on attachment 8397636 [details] [diff] [review] Patch: Kill all app when call is incoming, and let system app know why this app is killed Moving Gecko side patch to bug 992759.

(In reply to Patrick Wang (Chih-Kai Wang) (:kk1fff) from comment #35) > (In reply to Etienne Segonzac (:etienne) from comment #26) > > Created attachment 8399537 [details] [diff] [review] > > WIP : in-process dedicated callscreen app > > Etienne, would you file a new bug for creating dedicated callscreen app? > Let's keep this bug track work on music. I think we would need both. Thanks! Yep, we'll track this on bug 990003.

Comment on attachment 8399537 [details] [diff] [review] WIP : in-process dedicated callscreen app Review of attachment 8399537 [details] [diff] [review]: ----------------------------------------------------------------- Etienne, nothing personal and that may be a pretty good patch but I'd really like us to not have to do that. Let's revisit later if we really need.

Comment on attachment 8398423 [details] System + Music: implement Media App Agent and State Manager to restore music app from where it was closed Jim, I think I have wrapped up the patch and ready for review, as we know this is a new feature for both music and system apps so probably I didn't cover them all for the first review, hope you can find some issues on my patch. The issue that I mentioned in comment 37, Patrick has a gaia WIP(in bug 992759) for it and I am testing it with my patch, would you please review it first? thanks.

Comment on attachment 8398423 [details] System + Music: implement Media App Agent and State Manager to restore music app from where it was closed I don't have a Tarako to test this on just yet, but I think this is ok. You should probably have Alive do a full review of the system app part, since I'm not a system app reviewer.

(In reply to Jim Porter (:squib) from comment #42) > Comment on attachment 8398423 [details] > System + Music: Enhance the Media Playback Widget in system and implement > State Manager to restore music app > > I don't have a Tarako to test this on just yet, but I think this is ok. You > should probably have Alive do a full review of the system app part, since > I'm not a system app reviewer. Thanks Jim, that sounds right and because the system part depends on the gecko patch in bug 992759, which was dropped so I also have to change the strategy on how we re-launch the music app, I will modify that part then ask Alive to do a full review of the system part.

Comment on attachment 8398423 [details] System + Music: implement Media App Agent and State Manager to restore music app from where it was closed Alive, I have modified the system part(media_app_agent.js + media_playback.js) to adapt without or with Patrick's gecko patch in bug 992759(I was told it's still possible we bring back bug 992759 as the last ditch), or some other gecko patches that adjust the LMK score and caused the playing music app crash, that's, now this patch should be able to fit any gecko which might crash the playing music app, if it's caused by the incoming call. What I did is listen to the telephony state and the mozbrowsererror event from the music app iframe, then judge to restore the music app or not, hope it works as I imagined. Would you please review it? thanks.

Comment on attachment 8398423 [details] System + Music: implement Media App Agent and State Manager to restore music app from where it was closed r+ for 1.3t

(In reply to Alive Kuo [:alive][NEEDINFO!][God bless Taiwan.] from comment #45) > Comment on attachment 8398423 [details] > System + Music: implement Media App Agent and State Manager to restore music > app from where it was closed > > r+ for 1.3t Thanks Alive! And please wait for me to do the final check on the patch, after that I will land it on 1.3t then close this bug.

Do we want to land this patch disabled on master (with tests?)

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #47) > Do we want to land this patch disabled on master (with tests?) For me I think it's nice to let this "new feature" live on master and disabled by default, because tarako is probably not the "only" device with limited memory for us, we might have more in the future? at that time we can enable this on the devices/branches we want, but we need more work with tests to make this more convinced people then land it on master for sure.

Okay, I have finished checking the patch, it should be good to go, and I am waiting for the travis to get green.

v1.3t: eb0d0ebcd3919eb050f987046158b338733a76ca

We really need that to land on master. Tarako will switch back to master right after we ship 1.3t so we can't afford to have device specific fixes only on this branch.

(In reply to Fabrice Desré [:fabrice] from comment #51) > We really need that to land on master. Tarako will switch back to master > right after we ship 1.3t so we can't afford to have device specific fixes > only on this branch. You mean master branch(2.0?) will support tarako, that's good news for us.

(In reply to James Zhang from comment #52) > You mean master branch(2.0?) will support tarako, that's good news for us. Yes, we really don't want device specific branches in general. One gecko to rule them all!

I notice that the patch only handle incoming case, why not handle outgoing and other LMK cases? It's not enough.

(In reply to Jesse from comment #54) > I notice that the patch only handle incoming case, why not handle outgoing > and other LMK cases? > It's not enough. If the patch also handles the other LMK cases(not because of the incoming call), then once the playing music app is crashed by any reason, the system app will re-launch the music app as well. Before the music is restored and ready to resume some song, the user will notice: 1. The music app was crashed, so the playing song is stopped. 2. There is no sound for seconds, and the user doesn't know why. 3. After the music app is re-launched and resumes the song, the user hears the sound again. These behaviour could be confused because the user will feel the music app is launched automatically, and it's exactly the scenario I mentioned in comment 12, [3], which Vivien suggested us to adjust the OOM score in comment 14 to avoid a playing music crash. (And if we really want to avoid the playing music crash so perfectly, then we have to move the audio element into system app to keep it playing forever, which was this bug filed for but will result in a huge patch in this stage.)

(In reply to Fabrice Desré [:fabrice] from comment #51) > We really need that to land on master. Tarako will switch back to master > right after we ship 1.3t so we can't afford to have device specific fixes > only on this branch. We won't take "this" v1.3t patch for master for any reason. (PM will hate me, I know.) We will need a long term solution to 'keep state of the app when it's killed and then restore it' instead of hacking system app as anything you'd like to. It's not device specific but short term project specific.

(In reply to Vivien Nicolas (:vingtetun) (:21) (NOT reading bugmails, needinfo? please) from comment #14) > (In reply to Dominic Kuo [:dkuo] from comment #12) > > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9) > The approach would likely helps for [2] on all devices. If there is a > separate patch for [1] that's perfect. > > For [3] there is likely a few things we can do: > - Does the music app is killed but not the homescreen ? If yes, it probably > means that we need to think again about the adjusted OOM score for the > homescreen app (adjusted by mozapptype="homescreen") when an application is > playing some music. I don't think adjust OOM score for music help here. The LMK is killing by "count". If there's two app have the same score: one of the last running app will be killed. > To me it worth trying this approach, as the delayed call screen is an issue > not only on Tarako but on other devices as well. Etienne's patch for bug 990003 is releasing us from slow callscreen I think. Not sure what's the impact in the long term though. I guess android is separating callscreen and phone app as well.

(In reply to Alive Kuo [:alive][NEEDINFO!][God bless Taiwan.] from comment #58) > (In reply to Vivien Nicolas (:vingtetun) (:21) (NOT reading bugmails, > needinfo? please) from comment #14) > > (In reply to Dominic Kuo [:dkuo] from comment #12) > > > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9) > > The approach would likely helps for [2] on all devices. If there is a > > separate patch for [1] that's perfect. > > > > For [3] there is likely a few things we can do: > > - Does the music app is killed but not the homescreen ? If yes, it probably > > means that we need to think again about the adjusted OOM score for the > > homescreen app (adjusted by mozapptype="homescreen") when an application is > > playing some music. > > I don't think adjust OOM score for music help here. The LMK is killing by > "count". > If there's two app have the same score: one of the last running app will be > killed. > > > To me it worth trying this approach, as the delayed call screen is an issue > > not only on Tarako but on other devices as well. > > Etienne's patch for bug 990003 is releasing us from slow callscreen I think. > Not sure what's the impact in the long term though. I guess android is > separating callscreen and phone app as well. Android does not separate callscreen and phone app, but uses less memory consuming activity to handle call directly. Maybe we need to redesign web activity to handle request "directly".

(In reply to Jesse from comment #59) > (In reply to Alive Kuo [:alive][NEEDINFO!][God bless Taiwan.] from comment > Android does not separate callscreen and phone app, but uses less memory > consuming activity to handle call directly. Maybe we need to redesign web > activity to handle request "directly". What's interesting is "callscreen" in FxOS is not an activity for now. So redesign web activity doesn't help. Even we implement a new 'oncall' web activity, the same problem will occur: 1. It's a new process. 2. We are still loading it on demand, which causes the performance issue.

Music still can't background play. Peipei, please file a new bug to track.

I don't think this patch move music to system app.

This solution just keeps the playing "state" if music app is killed by LMK in a phone call, because you can't guarantee free memory is always enough to play music in the background while in a phone call. When phone call ends and memory is enough to run music player, music player will resume play at the saved "state".

(In reply to James Zhang from comment #61) > Music still can't background play. Peipei, please file a new bug to track. James, can you specify the scenario? I tried on latest build and Music can play in background. So I'm not sure which scenario you are talking about. Test build --------------------------------------------------------------------- │ Gaia 23488b1a45221c17e6a32fdd4c9d0fdbdcf2d021 │ Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/72055108f470 │ BuildID 20140414164002 │ Version 28.1 │ ro.build.version.incremental=eng.cltbld.20140414.200743

(In reply to pcheng from comment #64) > (In reply to James Zhang from comment #61) > > Music still can't background play. Peipei, please file a new bug to track. > > James, can you specify the scenario? I tried on latest build and Music can > play in background. So I'm not sure which scenario you are talking about. > > Test build > --------------------------------------------------------------------- > │ Gaia 23488b1a45221c17e6a32fdd4c9d0fdbdcf2d021 > │ Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/72055108f470 > │ BuildID 20140414164002 > │ Version 28.1 > │ ro.build.version.incremental=eng.cltbld.20140414.200743 I test today build, with bug 990003 patch, music won't be killed by incoming call. But I found music will be killed by Contacts apps.

It may cause a new bug. Play music -> back to homescreen and play video -> incoming call -> end call -> music will autoplay see http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=301728 Peipei, please file a new bug.

(In reply to James Zhang from comment #66) > It may cause a new bug. > Play music -> back to homescreen and play video -> incoming call -> end call > -> music will autoplay > see http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=301728 > Peipei, please file a new bug. I filed bug 998845

We should revert this change because bug 1002897 fixed.

Removing this change in bug 1022466.