1341417 - A partial SessionHistory.collect() still serialises all session history and then throws away the unneeded bits

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Jan Henning [:JanH]]

We should rather simply not serialise those session history entries in the first place if we don't need them in the end.

Comment 1

[Jan Henning [:JanH]]

Attachment: Bug 1341417 - Using aFromIdx should skip SHEntry serialisation, not just throw the data away afterwards.

When calling SessionHistory.collect(), we can indicate that we're not interested in the first N (where N = aFromIdx + 1) session history entries, with N potentially being a very big number if we don't want any SHEntries at all and are just interested in updating the value of the current history index.

Currently, we always serialise all available SHEntries and then throw away the first N entries, which is wasteful - instead, we should just skip them right away.

We still need to keep track how many entries we actually skipped and how many we could have collected if we wanted to, so
- we can correctly clamp data.index
- if we end up with no entries, figure out whether we simply skipped all of them or else need to create a fallback entry.

Review commit: https://reviewboard.mozilla.org/r/114232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114232/

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8846429 [details]
Bug 1341417 - Using aFromIdx should skip SHEntry serialisation, not just throw the data away afterwards.

https://reviewboard.mozilla.org/r/114232/#review121518

Nice improvement, thanks!

::: toolkit/modules/sessionstore/SessionHistory.jsm:83
(Diff revision 1)
>      let webNavigation = docShell.QueryInterface(Ci.nsIWebNavigation);
>      let history = webNavigation.sessionHistory.QueryInterface(Ci.nsISHistoryInternal);
>      let ihistory = history.QueryInterface(Ci.nsISHistory);
>  
>      let data = {entries: [], userContextId: loadContext.originAttributes.userContextId };
> +    let skippedCount = 0, entryCount = 0;

Can you add a comment here that explains what you're using these two variables for?

Comment 3

[Jan Henning [:JanH]]

Comment on attachment 8846429 [details]
Bug 1341417 - Using aFromIdx should skip SHEntry serialisation, not just throw the data away afterwards.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114232/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be2bd0abcf3e
Using aFromIdx should skip SHEntry serialisation, not just throw the data away afterwards.r=mikedeboer

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be2bd0abcf3e