Last Comment Bug 792143 - [Native Fennec] Expire unused tabs
: [Native Fennec] Expire unused tabs
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: -- normal with 1 vote (vote)
: Firefox 19
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
: general
Mentors:
Depends on:
Blocks: 256meg 803575 816381
  Show dependency treegraph
 
Reported: 2012-09-18 11:39 PDT by Kartikaya Gupta (email:kats@mozilla.com)
Modified: 2013-01-28 08:04 PST (History)
8 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part (1/2) - Add an TabMruList object to maintain an MRU list of tabs (6.48 KB, patch)
2012-10-12 11:28 PDT, Kartikaya Gupta (email:kats@mozilla.com)
bugmail.mozilla: review-
Details | Diff | Review
(2/2) Zombify an appropriate tab on opening a new one (1.01 KB, patch)
2012-10-12 11:30 PDT, Kartikaya Gupta (email:kats@mozilla.com)
bugmail.mozilla: review-
Details | Diff | Review
(1/2) Maintain most recent use timestamps on tabs (5.16 KB, patch)
2012-10-19 08:16 PDT, Kartikaya Gupta (email:kats@mozilla.com)
mark.finkle: review+
Details | Diff | Review
(2/2) Zombify an appropriate tab on opening a new one (1.00 KB, patch)
2012-10-19 08:17 PDT, Kartikaya Gupta (email:kats@mozilla.com)
mark.finkle: review+
Details | Diff | Review

Description Kartikaya Gupta (email:kats@mozilla.com) 2012-09-18 11:39:36 PDT
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?) 2012-09-18 12:28:35 PDT
We could use this for armv7 too. I think this is a memory requirement, not CPU requirement.
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-12 11:28:11 PDT
Created attachment 670885 [details] [diff] [review]
Part (1/2) - Add an TabMruList object to maintain an MRU list of tabs
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-12 11:30:27 PDT
Created attachment 670888 [details] [diff] [review]
(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?) 2012-10-18 06:49:08 PDT
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:kats@mozilla.com) 2012-10-18 07:18:24 PDT
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:kats@mozilla.com) 2012-10-19 08:16:23 PDT
Created attachment 673257 [details] [diff] [review]
(1/2) Maintain most recent use timestamps on tabs

Updated as per comments
Comment 7 Kartikaya Gupta (email:kats@mozilla.com) 2012-10-19 08:17:00 PDT
Created attachment 673258 [details] [diff] [review]
(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?) 2012-10-19 08:28:55 PDT
Comment on attachment 673257 [details] [diff] [review]
(1/2) Maintain most recent use timestamps on tabs

Nice
Comment 11 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-10-19 20:25:03 PDT
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?) 2012-10-19 22:09:10 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.