Closed Bug 1282830 Opened 4 years ago Closed 4 years ago

Closing a zombie tab doesn't trigger a session save

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 50
Tracking Status
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(1 file)

For whatever reasons, when closing a zombie tab we're exiting early from this onTabRemove here (https://dxr.mozilla.org/mozilla-central/rev/2ab57664bf7cafdd64e136e341527c275fc8c3aa/mobile/android/components/SessionStore.js#466) and never trigger a session save.
This means that if Firefox is subsequently closed without anything else having triggered a session save, those tabs will in fact reappear the next time Firefox is started.

It's also an alternative way for bug 1256277 to have happened, which coincidentally is also not as timing-dependant as my original STR there.
A tab being in a delay-loaded "zombie" state or not shouldn't have any influence on the behaviour of onTabRemove - since we remove it from the session store's sphere of influence, its __SS_data can be safely deleted anyway and whether or not a session save needs to be triggered should depend only on the aNoNotfication parameter passed by the caller.

Otherwise, with the current behaviour, the fact that those tabs have been closed will not get saved to disk if no subsequent session save is triggered through any other means (e.g. selecting a different tab, scrolling, ...) before closing Firefox.

Review commit: https://reviewboard.mozilla.org/r/61340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61340/
Attachment #8766476 - Flags: review?(margaret.leibovic)
Comment on attachment 8766476 [details]
Bug 1282830 - Trigger session saves when closing zombie tabs, too.

Sorry I've been so slow to get around to reviews, redirecting to Sebastian.
Attachment #8766476 - Flags: review?(margaret.leibovic) → review?(s.kaspari)
Comment on attachment 8766476 [details]
Bug 1282830 - Trigger session saves when closing zombie tabs, too.

https://reviewboard.mozilla.org/r/61340/#review59708

Sounds reasonable.

I was trying to track back why this line existed in the first place. It seems like it was added in this changeset from 2010 (bug 617851):
https://hg.mozilla.org/mozilla-central/diff/e6c70153e799/mobile/components/SessionStore.js#l1.125
However I do not see any particular reason why we might still need it, so let's see how it goes.
Attachment #8766476 - Flags: review?(s.kaspari) → review+
Yup, I had already looked at that commit, too, but was left none the wiser as to why that line was there.
Comment on attachment 8766476 [details]
Bug 1282830 - Trigger session saves when closing zombie tabs, too.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61340/diff/1-2/
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/5fce05217908
Trigger session saves when closing zombie tabs, too. r=sebastian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5fce05217908
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Duplicate of this bug: 1285841
Comment on attachment 8766476 [details]
Bug 1282830 - Trigger session saves when closing zombie tabs, too.

Approval Request Comment
[Feature/regressing bug #]: mobile session store
[User impact if declined]: closing a zombified background tab won't trigger a session save, so if Firefox is subsequently closed/killed, those tabs might reappear on next startup
[Describe test coverage new/current, TreeHerder]: manual, some time on Nightly
[Risks and why]: low, sooner or later (after switching to a different tab, opening a new tab, closing a non-zombified tab, scrolling, page navigation, ...) a session save would have been triggered anyway, so we just make sure to immediately trigger one after closing a zombified tab, too
[String/UUID change made/needed]: none
Attachment #8766476 - Flags: approval-mozilla-aurora?
Comment on attachment 8766476 [details]
Bug 1282830 - Trigger session saves when closing zombie tabs, too.

Less zombies is good, taking it!
Attachment #8766476 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.