Bug 1190627 - Part 2 - Defer writes if there's already an async write in progress. r=margaret
Currently, it is possible for a second write (sync or async) to be requested while a previous async write operation is still in progress. This might lead to undesired results if the second write is then completed before the first write, or if a sync write is interfering with a parallel async write operation. The only guard against a second async write is the minimum delay of 2 s between successive async writes enforced in saveStateDelayed(); there is no guard against sync writes.
To avoid data loss when the application is backgrounded, it is desirable to reduce or completely eliminate this minimum delay (see Part 3), therefore we need to devise alternative means of ensuring that successive writes won't interfere with each other.
With this patch, only one save operation is allowed to execute within _saveState() at the same time. Successive calls to _saveState() will be deferred, coalesced into one operation and executed once the previous async write returns from _writeFile()'s promise callback. Sync writes take priority, so if any of the deferred calls to _saveState() is a sync write, the resulting operation will be a sync write, too.
This has the slight drawback that we can't execute truly synchronously within Android's onPause() call if an async state save is already in progress, however this should occur only very occasionally and is probably still more desirable than a possible write collision with a previous async state save.
MozReview-Commit-ID: G2eogo1z8vr
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -56,18 +56,20 @@ SessionStore.prototype = {
Ci.nsIDOMEventListener,
Ci.nsIObserver,
Ci.nsISupportsWeakReference]),
_windows: {},
_lastSaveTime: 0,
_interval: 10000,
_maxTabsUndo: 5,
+ _scrollSavePending: null,
_pendingWrite: 0,
- _scrollSavePending: null,
+ _writeInProgress: false,
+ _deferredWrite: {},
// The index where the most recently closed tab was in the tabs array
// when it was closed.
_lastClosedTabIndex: -1,
// Whether or not to send notifications for changes to the closed tabs.
_notifyClosedTabs: false,
@@ -782,16 +784,29 @@ SessionStore.prototype = {
log("_saveState(aAsync = " + aAsync + ")");
// Kill any queued timer and save immediately
if (this._saveTimer) {
this._saveTimer.cancel();
this._saveTimer = null;
log("_saveState() killed queued timer");
}
+ // If there's already an async write in progress, we don't want
+ // to interfere by attempting another write in parallel,
+ // especially especially not a synchronous write.
+ if (this._writeInProgress) {
+ this._deferredWrite.pending = true;
+ // All _saveState() calls with an async write in progress will be coalesced
+ // into one deferred operation, so sync writes take precedence.
+ this._deferredWrite.syncWrite = this._deferredWrite.syncWrite || !aAsync;
+ log("_saveState() deferring write, asyncWrite = " + !this._deferredWrite.syncWrite);
+ return;
+ }
+ this._writeInProgress = true;
+
let data = this._getCurrentState();
let normalData = { windows: [] };
let privateData = { windows: [] };
log("_saveState() current state collected");
for (let winIndex = 0; winIndex < data.windows.length; ++winIndex) {
let win = data.windows[winIndex];
let normalWin = {};
@@ -939,16 +954,28 @@ SessionStore.prototype = {
}
log("_writeFile() _write() returned, _pendingWrite = " + this._pendingWrite);
// We don't use a stopwatch here since the calls are async and stopwatches can only manage
// a single timer per histogram.
Services.telemetry.getHistogramById("FX_SESSION_RESTORE_WRITE_FILE_MS").add(Math.round(stopWriteMs - startWriteMs));
Services.obs.notifyObservers(null, "sessionstore-state-write-complete", "");
+
+ this._writeInProgress = false;
+
+ // A call to _saveState() was deferred while an async write
+ // was in progress, so we execute it now.
+ if (this._deferredWrite.pending) {
+ asyncWrite = !this._deferredWrite.syncWrite;
+ delete this._deferredWrite.syncWrite;
+ delete this._deferredWrite.pending;
+ log("_writeFile() executing deferred write");
+ this._saveState(asyncWrite);
+ }
});
},
/**
* Writes the session state to a disk file, using async or sync methods
* @param aFile nsIFile used for saving the session
* @param aFileTemp nsIFile used as a temporary file in writing the data
* @param aBuffer UTF-8 encoded ArrayBuffer of the session state