792143 - [Native Fennec] Expire unused tabs

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

In the case where the user has many tabs opened, but has not used some of the tabs in a while, we should kill the tab and free the memory it is holding. Serializing the entire page is very tricky to do, but we should be able to kill the tab and use the session-restore code to bring it back if the user switches back to it.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We could use this for armv7 too. I think this is a memory requirement, not CPU requirement.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part (1/2) - Add an TabMruList object to maintain an MRU list of tabs

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: (2/2) Zombify an appropriate tab on opening a new one

I figured opening a new tab would be a good place to zombify an older tab because that's where we know we're going to need a bunch of memory. We could move this call elsewhere though. Also note that even with this code the zombieTime pref is still set to -1 so zombification will be off by default.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 670885 [details] [diff] [review]
Part (1/2) - Add an TabMruList object to maintain an MRU list of tabs

I like the concept here. Every time I start to review the code, I want to change the structure though. A few things:
1. Make "TabMruList" become "Tabs", which manages the tabs. We'd extract a lot of code from BrowserApp.
2. Couldn't we just use Tab.lastTouch (as a Date) and just compare that when zombifying?

#1 could wait for a refactor. What about #2? Do we need the mruIndex too?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 670885 [details] [diff] [review]
Part (1/2) - Add an TabMruList object to maintain an MRU list of tabs

Yeah I was also thinking about how the mruIndex is pretty much useless since we can accomplish the same thing using just the timestamp. I'll redo these patches, I think I can make it much less bloated.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: (1/2) Maintain most recent use timestamps on tabs

Updated as per comments

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: (2/2) Zombify an appropriate tab on opening a new one

Rebased. Note that this still doesn't enable it by default since the zombieTime pref is -1

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 673257 [details] [diff] [review]
(1/2) Maintain most recent use timestamps on tabs

Nice

Comment 9

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c0a5d9f82ee
https://hg.mozilla.org/integration/mozilla-inbound/rev/9086b50a71c3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4c0a5d9f82ee
https://hg.mozilla.org/mozilla-central/rev/9086b50a71c3

Comment 11

[Nicholas Nethercote [inactive]]

Is there no chance that user data (e.g. partially filled-in forms) will be lost?  After all the previous discussion I've seen on this kind of thing, I'm surprised that two small patches is enough to make it work safely.  How does this differ from bug 675539?

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nicholas Nethercote [:njn] from comment #11)
> Is there no chance that user data (e.g. partially filled-in forms) will be
> lost?  After all the previous discussion I've seen on this kind of thing,
> I'm surprised that two small patches is enough to make it work safely.  How
> does this differ from bug 675539?

Currently, Firefox Mobile session restore does not save off form data. Therefore, expired tabs would lose any partial form data the user filled.

We could add that feature, or a minimal version of it.

This bug is similar to bug 675539, but that patch is limited to desktop Firefox.