902280 - [Session Restore] When invalidating while writing, we end up deceeding the write interval

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Let's say we just wrote to disk. Everything's fine and _lastSaveTime has been updated with Date.now().

We now immediately close a tab and this leads to saveStateDelayed() being called. Because we just saved, the minimumDelay in saveStateDelayed() will be a few ms less than 15s (the default interval).

After 15s, the timer is fired and _saveTimer is set back to null. SessionFile.write() is called and we update _lastSaveTime when the promise is resolved.

Now here's the problem: if we close another tab while the SessionFile.write() promise hasn't been resolved yet, minimumDelay will be set to a negative value, because we exceeded the target interval of 15s and the parameter passed (aDelay) will be the timer value, which is 2s by default.

Thus we end up writing just 2s after we wrote again. If we now close another tab... we could keep collecting and writing data in very short intervals.

Comment 1

[Steven MacLeod [:smacleod]]

Attachment: Patch - Update _lastSaveTime before writing

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 792305 [details] [diff] [review]
Patch - Update _lastSaveTime before writing

Review of attachment 792305 [details] [diff] [review]:
-----------------------------------------------------------------

This is a great and simple solution. We could correct the timeout after we finished writing but I think we shouldn't overcomplicate the code. This will be fine for all the cases where writing takes less time than what the interval is set to.

Comment 3

[Steven MacLeod [:smacleod]]

Attachment: Patch - Update _lastSaveTime before writing

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/6d655c662b5f

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6d655c662b5f