Closed Bug 982938 Opened 6 years ago Closed 6 years ago

[e10s] Session restore doesn't seem to save the session on exit

Categories

(Firefox :: Session Restore, defect)

x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
e10s + ---
firefox31 --- verified

People

(Reporter: billm, Assigned: billm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I haven't looked at this too closely yet, but the problem is as follows:

1. Load browser and watch it restore a session.
2. Navigate to a new page.
3. Quit and restart.

The session that will be restored is the same as in step 1. The page in step 2 never registers. I think the problem might be related to the timing of when the content process gets shutdown versus when we save the session.
Attached patch session-restoreSplinter Review
The problem here is that we would set the restore epoch for the tab to be restored and then call updateBrowserRemoteness. If updateBrowserRemoteness actually needed to change the remoteness, then we would get a new XBL binding instance and a new permanentKey for the <browser> element. Later on in the restoration, isCurrentEpoch would return false because we wouldn't have anything in the map for the new permanentKey.

Surprisingly, things mostly worked except that the __SS_data property never got deleted. Consequently, we never saved data for this tab anymore, since we always copy it from __SS_data when that exists.

The easiest way to fix this is to move the updateBrowserRemoteness call before setting the restore epoch. That way we use the new permanentKey.

I think that updateBrowserRemoteness can cause more trouble than just this issue here. However, the only problem I can think of right now is if someone does a navigation that requires a change to the remoteness while we're in the middle of a session restore. That seems like a pretty unusual case, and we're still not sure what the long-term future is for updateBrowserRemoteness. So I'd rather just patch the immediate problem at this point.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8411487 - Flags: review?(ttaubert)
Attachment #8411487 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/mozilla-central/rev/ec9e918e47dd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Henrik, this is another example of something we might be able to catch by running existing Mozmill tests against e10s.
Flags: in-qa-testsuite?(hskupin)
Keywords: verifyme
Verified fixed on latest Firefox Nightly (33.0a1), build ID: 20140714030201.

I was unable to verify this fix on Firefox 31 RC by switching browser.tabs.remote.autostart to true due to bug 1038657, but I managed to confirm it on 2014-04-27 Nightly (31.0a1) - build ID: 20140427030204.

Considering the above, I believe the fix for this issue could be safely marked as verified.

Bill, what do you think?
Flags: needinfo?(wmccloskey)
Keywords: verifyme
Sure, sounds fine.
Flags: needinfo?(wmccloskey)
Thanks!
Marking issue verified.
Status: RESOLVED → VERIFIED
Removing my name from in-qa-testsuite flag for a better query.
Flags: in-qa-testsuite?(hskupin) → in-qa-testsuite?
A test for session restore will be added via bug 1228120 for firefox-ui-tests.
Flags: in-qa-testsuite?
You need to log in before you can comment on or make changes to this bug.