Closed Bug 1340828 Opened 3 years ago Closed 3 years ago

Use SessionHistory.jsm on Android

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

As far as I can see, the session history code thankfully hasn't diverged very far between desktop and mobile, so it should be relatively straightforward to swap in SessionHistory.jsm and replace our own copy of the code.

Going forward, this should simplify the maintenance of this, since Desktop/Core people won't have to remember any more that any fixes in session history saving/restoring need to be ported over to Fennec as well.
Blocks: 1265818
See Also: → 1340874
Blocks: 1341810
Comment on attachment 8843565 [details]
Bug 1340828 - Part 3 - Switch Fennec's session store over to SessionHistory.jsm.

https://reviewboard.mozilla.org/r/113676/#review119200

Awesome!
Attachment #8843565 - Flags: review?(s.kaspari) → review+
Comment on attachment 8843563 [details]
Bug 1340828 - Part 1 - Move SessionHistory.jsm to toolkit.

https://reviewboard.mozilla.org/r/113672/#review119206

I'm very happy that Fennec is able to use this code! And even happier with the ESLint <3 ;-)
Attachment #8843563 - Flags: review?(mdeboer) → review+
Comment on attachment 8843564 [details]
Bug 1340828 - Part 2 - Make life easier for Fennec.

https://reviewboard.mozilla.org/r/113674/#review119208

LGTM!
Attachment #8843564 - Flags: review?(mdeboer) → review+
Comment on attachment 8843565 [details]
Bug 1340828 - Part 3 - Switch Fennec's session store over to SessionHistory.jsm.

https://reviewboard.mozilla.org/r/113676/#review119218

::: mobile/android/components/SessionStore.js:1293
(Diff revision 1)
> -    aHistory.getEntryAtIndex(activeIndex, true);
>  
> +    // SessionHistory.jsm will have force set the active history item,
> +    // but we still need to reload it in order to finish the process.
>      try {
> -      aHistory.QueryInterface(Ci.nsISHistory).reloadCurrentEntry();
> +      history.reloadCurrentEntry();

Note to myself before landing:
In practice it seems to work anyway, but since SessionHistory.jsm returns an `nsISHistoryInternal` interface, I technically still need to QI this to `nsISHistory` before calling `reloadCurrentEntry()`.
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/94b9be873261
Part 1 - Move SessionHistory.jsm to toolkit. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/bcea389b60b1
Part 2 - Make life easier for Fennec. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/a9c30ad75f43
Part 3 - Switch Fennec's session store over to SessionHistory.jsm. r=sebastian
You need to log in before you can comment on or make changes to this bug.