Open Bug 1538291 Opened 6 years ago Updated 2 years ago

Add partial history aggregation and possibly limit history size in GV session storage code

Categories

(GeckoView :: General, enhancement, P3)

All
Android
enhancement

Tracking

(firefox66 wontfix, firefox67 wontfix, firefox67.0.1 wontfix, firefox68 wontfix, firefox69 wontfix, firefox70 wontfix, firefox71 wontfix)

Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox67.0.1 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix

People

(Reporter: droeh, Unassigned)

References

Details

We want to re-add partial history aggregation in SessionStateAggregator.js and handle the logic for that in updateSessionState; additionally, to prevent the session history size from becoming an issue (in the case of eg history API abuse), we may want to limit the number of entries.

(In reply to Dylan Roeh (:droeh) from comment #0)

we may want to limit the number of entries.

As long as we only restore the current state of the Desktop code, which only uses partial session history updates for a relatively limited number of cases [1], shouldn't that already work through browser.sessionhistory.max_entries, which limits the amount of session history the docshell keeps around in the first place? Therefore the next full history update will truncate our cached data, too.

[1] And using partial session history updates more often than that would require solving bug 1350567 first.

Ah, I think my initial comment was unclear -- we want to limit the number of history entries present in mStateCache in GeckoSession, which (unless I'm misunderstanding something) could currently grow without bounds.

If I'm not mistaken, a full update (which is all we're currently doing, and even in the future it would happen frequently enough because as I said doing it less often, e.g. bug 1350569, requires some more prerequisite work), will replace all the existing contents of the state cache (compare what the TabStateCache does with a fromIdx of -1) with the current session history held by the docshell, which is capped by browser.sessionhistory.max_entries. So the size of the history in the Java state cache couldn't really grow beyond the size of the history held by Gecko itself.

You're right that more partial updates might require further work to properly handle it in the cache (and it might need to mirror the history truncation that occurs in Gecko when you hit browser.sessionhistory.max_entries, too), but at the moment I don't see any problem there.

Priority: -- → P1
Whiteboard: [geckoview:fenix:m4]

67=wontfix. Fenix MVP will use GeckoView 68, so we don't need to uplift this fix to 67 Beta.

Moving from Fenix M4 to M7 because Dylan says this bug doesn't need to block Fenix MVP.

Whiteboard: [geckoview:fenix:m4] → [geckoview:fenix:m7]

Deferring this bug from Fenix's M7 (July) milestone to the M8 backlog for later in Q3.

Whiteboard: [geckoview:fenix:m7] → [geckoview:fenix:m8]

I'm editing a bunch of GeckoView bugs. If you'd like to filter all this bugmail, search and destroy emails containing this UUID:

e88a5094-0fc0-4b7c-b7c5-aef00a11dbc9

Priority: P1 → P2

We are moving to SessionStoreListener rather than SessionStateAggregator. We should check that whatever we want from here is present in the SessionStoreListener.

Blocks: 1693682
Priority: P2 → P3

The bug assignee didn't login in Bugzilla in the last 7 months.
:amoya, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: droeh → nobody
Flags: needinfo?(amoya)
No longer blocks: 1693682
Severity: normal → --
Severity: -- → N/A
Flags: needinfo?(amoya)
Rank: 38 → 333
You need to log in before you can comment on or make changes to this bug.