Closed Bug 935750 (suspending-app) Opened 11 years ago Closed 10 years ago

[Window Management] Keep crashed app slept and revive it once being opened again.

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:-, b2g-v1.3T wontfix, b2g-v1.4 wontfix, b2g-v2.0 fixed, b2g-master verified)

VERIFIED FIXED
blocking-b2g -
Tracking Status
b2g-v1.3T --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- fixed
b2g-master --- verified

People

(Reporter: alive, Assigned: alive)

References

Details

Attachments

(1 file, 1 obsolete file)

I noted that if an app is killed after we open the cardview, the card is still there and click on the card doesn't do anything.
We could: dynamically removing the card(with animation?) when the app is terminated at background OR
Keep the configuration of the appWindow and "relaunch" the app when user click the "card".
Blocks: task-manager
No longer blocks: window-management
We will get this for free with bug 939809 since the StackManager knows when an app is killed.

Maybe we should rename here to represent the work on "relaunch".
Summary: [Window Management] CardView should know app is killed → [Window Management] CardView should relaunch app if the app was killed during cardview is shown
(In reply to Etienne Segonzac (:etienne) from comment #1)
> We will get this for free with bug 939809 since the StackManager knows when
> an app is killed.

I think we could just use AppWindow.prototype.isDead implemented by you and implement AppWindow.prototype.relaunch
I don't see the hard block from bug 939809 now.

> 
> Maybe we should rename here to represent the work on "relaunch".

Renamed
Assignee: nobody → alive
A more simple fix:
Card should knows the app config, so it just needs to tell AppWindowFactory/AppWindowManager to launch the same app with the same config when the user is clicking on the dead card.
I had the patch but the problem is how should I re-instantiate the Wrapper :/
Because the flow of wrapper launch is a little messy now.
WIP, needs tests.
I decide to drop this idea because the call path is somewhat weird. Deassign first.
Assignee: alive → nobody
Have a patch working now. Take back.
Assignee: nobody → alive
Let's see if etienne like this.
Attachment #8349823 - Attachment is obsolete: true
Attachment #8371910 - Flags: review?(etienne)
Summary: [Window Management] CardView should relaunch app if the app was killed during cardview is shown → [Window Management] Keep crashed app slept and revive it once being opened again.
Comment on attachment 8371910 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16061

Frankly Alive you are my hero :)

I'm having issues with the screenshot caching (some previously zombied app never get screenshoted again)... but the main feature works wonderfully!

I think we should land the main feature ASAP without the screenshot/caching part and open a follow up where we get UX input for the transition and figure out the details.

What do you think?
Attachment #8371910 - Flags: review?(etienne) → feedback+
It's fine for me but let me struggle for a while :)
I am trying to do: render the background with (1) icon splash again or (2) the cached screenshot URL.
So we don't need to change AppWindow.prototype._showScreenshotOverlay and _hideScreenshotOverlay to avoid the issue.
Comment on attachment 8371910 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16061

Second struggle ;)
* Render the container background with screenshot cache if any instead of using showScreenshotOverlay.
Attachment #8371910 - Flags: review?(etienne)
Comment on attachment 8371910 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16061

Never thought I would say that, but:

r- because of a risk of zombie apocalypse.

:)

This is a pretty awesome feature for devices with only 256MB of RAM, I'm trying it on a buri with great success, to make it even better:

* We need to remove the screenshot in element.backgroundImage when we revive the app.

* We shouldn't keep more than 10 zombie apps (UX agreed on this number), otherwise things could go crazy especially with the current card view :/

Note sure what's the best way to implement the second item, but we can do something simple at first and fine tune it later.
Attachment #8371910 - Flags: review?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #12)
> r- because of a risk of zombie apocalypse.

BRAINNNNNNNNNNNNNNN

> * We shouldn't keep more than 10 zombie apps (UX agreed on this number),
> otherwise things could go crazy especially with the current card view :/
> 
> Note sure what's the best way to implement the second item, but we can do
> something simple at first and fine tune it later.

The first thing come to my mind is an App-Window-Priority-Manager to actively kill background appWindow by launching time :) Maybe reference to stackManager.
Comment on attachment 8371910 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16061

Hope it's last run!
* Replace all zombie with suspending to avoid confusion. Love 'zombie' but you know that. BRAAAAINNNNNN
* Add SuspendingAppPriorityManager to active kill eldest suspending app.
* Force removing backgroundImage when app is loaded.
* More unit tests.
Attachment #8371910 - Flags: review?(etienne)
Comment on attachment 8371910 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16061

r=me with the few nits addressed and the small follow ups filed.

glad the zombie apocalypse was avoided, very glad to see fast app switching being useful on a buri :)
Attachment #8371910 - Flags: review?(etienne) → review+
Blocks: 972899
Blocks: 972915
https://github.com/mozilla-b2g/gaia/commit/5c33197f62c17adb9c51f9e6539d51a4a69e928e

Thanks Etienne for purging zombies!!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: 973828
Blocks: 992083
Alias: suspending-app
This feature was being discussed for possible uplift to 1.3T to provide better experience.

The master/1.4 patch is quite complex so it clear that we cannot do simple uplift for it. Alive, would you be able to provide a risk assessment and LOE for re-do this on 1.3T?
Flags: needinfo?(alive)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #20)
> This feature was being discussed for possible uplift to 1.3T to provide
> better experience.
> 
> The master/1.4 patch is quite complex so it clear that we cannot do simple
> uplift for it. Alive, would you be able to provide a risk assessment and LOE
> for re-do this on 1.3T?

LOE is about one week, and since we tend not to take any screenshot in tarako,
some cardview logic needs to be changed to reflect, e.g. put app icon as screenshot layer.
Some life cycle event handler needs to be moved into the window.js from window_manager.js

SuspendingAppPriorityManager is still needed because it's easy to have too many killed apps.

I'm not sure if this feature will conflict with reviving music from system work Dominic had already implemented for tarako.
Flags: needinfo?(alive)
Blocks: 995119
No longer blocks: 995119
blocking-b2g: --- → 1.3T?
triage: This feature is too risky to uplift to 1.3T+. minus
blocking-b2g: 1.3T? → -
Hi Alive,

Does v1.4 contain this fix?
Flags: needinfo?(alive)
(In reply to Ivan Tsay (:ITsay) from comment #23)
> Hi Alive,
> 
> Does v1.4 contain this fix?

No.
Flags: needinfo?(alive)
Mass modify - set status-b2g-v2.0 fixed for fixed bugs under vertical homescreen dependency tree.
Not sure if this is suitable for 1.4.  Do you know if we should have this for 1.4, Alive?
ni? james as a fyi.
Flags: needinfo?(james.zhang)
Flags: needinfo?(alive)
I think bug 1005787 patch is better.
Alive, what do you think about?
Flags: needinfo?(james.zhang)
Ah ok.  Thanks James.  I'll leave it up to you and Alive to figure out.  :)
Bug 1005787 is totally different from this; this one is a feature and bug 1005787 is a bug.
The feature is talking about: keep the crashed app even it's killed by OOM at background.
The bug is talking about: relaunch the app once the app is killed "only" during cardsview is opened.

This feature is still pref-off in master now so I would say no for v1.4
Flags: needinfo?(alive)
Verifying fix on flame 3.0 devices.
Results: When apps OoM, they will remain in the task manager as zombie apps, where attempting to access them will effectively relaunch the app. These apps maintain their previous screenshot while in card, but on open relaunch and thus do not retain their previous screenshot state.

--------------------------------------------------
Environmental Variables:
Device: Flame 3.0
BuildID: 20150210010523
Gaia: 0cf517083f7eb5fc269e1236edba50534f65e3cd
Gecko: 2cb22c058add
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
--------------------------------------------------
Repro: 7/7
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: