1868833 - sessionStorage reverts to previous state on back navigation

Status: NEW

(Core :: Storage: localStorage & sessionStorage, defect, P3)

Description

[eyal gruss]

1. sessionStorage.eyal = 'hi'
2. navigate to a different page on the same domain in the same tab
3. delete sessionStorage.eyal
4. press back button

result:
sessionStorage contains the deleted key-value (maybe due to bfcache?)

expected:
sessionStorage should not revert to previous state. this is the behavior on chrome.

not sure if this is related: https://bugzilla.mozilla.org/show_bug.cgi?id=1790832

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Hmm, are we missing to update the sessionstorage in bfcache? I wonder if this is a regression from session-history-in-parent.

janv, do you recall how the updates for sessionstorage work?

Comment 2

[Jan Varga [:janv]]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #1)

Hmm, are we missing to update the sessionstorage in bfcache? I wonder if this is a regression from session-history-in-parent.

janv, do you recall how the updates for sessionstorage work?

I could take a look, but I think Andreas Farre and Andrew Sutherland can answer this better, since they were more involved in the sessionStorage in parent work.

Comment 3

[Andreas Farre [:farre]]

If it's a bfcache problem I'm not that knowledgeable, unfortunately.

If it's a session store thing, I probably know more. For session store we'd do a collect and a write in CanonicalBrowsingContext::UpdateSessionStoreSessionStorage. As it happens, this is behind a pref so we could check it quickly. I did, and it still reproduces (I'm just doing it through the Web Console, which triggers the problem).

On the other hand, we also have the pref fission.disableSessionHistoryInParent, so we should be able to check without SHIP as well. And it reproduces with SHIP disabled.

I'm firing up mozregression.

Comment 4

[Andreas Farre [:farre]]

Ok, https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9d499871dc8c87d1f138d1a954b675df5030a43&tochange=06c7b4e2d14be1ee8050d6c1eb1ab1bab19ac3c2 found a large pushlog, which happens to contain turning on Fission.

So, this is probably a Fission thing that we've managed to not fix so far. Could still be bfcache, but not SHIP.

Comment 5

[Martin Blom]

It might affect localStorage as well? I'm seeing weird behavior on back navigation in code I know uses localStorage for its state.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Martin Blom from comment #5)

It might affect localStorage as well? I'm seeing weird behavior on back navigation in code I know uses localStorage for its state.

It should not.

Does the code depend on "storage" events or could it be using sync XHR?

For the former, "storage" events are discarded and will not be replayed if they would have been fired while a page is in bfcache. There have been some spec discussions about storage events primarily in the context of SessionStorage but it's also a known issue in LocalStorage (although that issue also discusses the synchronization in general, which is mroe difficult).

For the latter (sync XHR), I think that's pretty much the only way one could see stale data for LocalStorage. Our implementation maintains a single authoritative state in the parent process, but we do establish a snapshot in content processes to provide a consistent and coherent run to completion semantics so that within synchronous execution LocalStorage values cannot change until control flow is yielded. Sync XHR inherently messes with that.

Comment 7

[Martin Blom]

The code in question is an old Redmine plugin of mine (which hasn't been updated for recent Redmine versions, but have been working perfectly with our Redmine 3.4 server for ages — until it started misbehaving a couple of weeks or months ago).

My misbehaving code is a single JavaScript file and yep, it does use storage events to keep the state in sync between tabs and navigation.