Bug 965137 - Make session saving more reliable. r=mfinkle, a=sledru
authorBrian Nicholson <bnicholson@mozilla.com>
Wed, 05 Feb 2014 23:47:51 -0800
changeset 176288 8cb5df1f8bc8cee15cb6bc3f2d76b04d8bf9165e
parent 176287 02935dcaa786a541f4018ad9795a70eae51a9519
child 176289 f4b5ae829393ea6a10be2af54b8a0eede7d80818
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmfinkle, sledru
bugs965137
milestone28.0
Bug 965137 - Make session saving more reliable. r=mfinkle, a=sledru
mobile/android/base/tests/SessionTest.java
mobile/android/base/tests/testSessionOOMSave.java
mobile/android/components/SessionStore.js
--- a/mobile/android/base/tests/SessionTest.java
+++ b/mobile/android/base/tests/SessionTest.java
@@ -373,16 +373,20 @@ public abstract class SessionTest extend
         try {
             writeFile(new File(mProfile, filename), data);
         } catch (IOException e) {
             mAsserter.ok(false, "Error writing to " + filename, getStackTraceString(e));
         }
     }
 
     private String readFile(File target) throws IOException {
+        if (!target.exists()) {
+            return null;
+        }
+
         FileReader fr = new FileReader(target);
         try {
             StringBuffer sb = new StringBuffer();
             char[] buf = new char[8192];
             int read = fr.read(buf);
             while (read >= 0) {
                 sb.append(buf, 0, read);
                 read = fr.read(buf);
--- a/mobile/android/base/tests/testSessionOOMSave.java
+++ b/mobile/android/base/tests/testSessionOOMSave.java
@@ -58,16 +58,21 @@ public class testSessionOOMSave extends 
         public VerifyJSONCondition(Session session) {
             mSession = session;
         }
 
         @Override
         public boolean isSatisfied() {
             try {
                 String sessionString = readProfileFile("sessionstore.js");
+                if (sessionString == null) {
+                    mLastException = new AssertException("Could not read sessionstore.js");
+                    return false;
+                }
+
                 verifySessionJSON(mSession, sessionString, mAsserter);
             } catch (AssertException e) {
                 mLastException = e;
                 return false;
             }
             return true;
         }
 
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -39,16 +39,17 @@ SessionStore.prototype = {
                                          Ci.nsIDOMEventListener,
                                          Ci.nsIObserver,
                                          Ci.nsISupportsWeakReference]),
 
   _windows: {},
   _lastSaveTime: 0,
   _interval: 10000,
   _maxTabsUndo: 1,
+  _pendingWrite: 0,
 
   init: function ss_init() {
     // Get file references
     this._sessionFile = Services.dirsvc.get("ProfD", Ci.nsILocalFile);
     this._sessionFileBackup = this._sessionFile.clone();
     this._sessionFile.append("sessionstore.js");
     this._sessionFileBackup.append("sessionstore.bak");
 
@@ -77,16 +78,17 @@ SessionStore.prototype = {
         observerService.addObserver(this, "domwindowopened", true);
         observerService.addObserver(this, "domwindowclosed", true);
         observerService.addObserver(this, "browser-lastwindow-close-granted", true);
         observerService.addObserver(this, "browser:purge-session-history", true);
         observerService.addObserver(this, "quit-application-requested", true);
         observerService.addObserver(this, "quit-application-granted", true);
         observerService.addObserver(this, "quit-application", true);
         observerService.addObserver(this, "Session:Restore", true);
+        observerService.addObserver(this, "application-background", true);
         break;
       case "final-ui-startup":
         observerService.removeObserver(this, "final-ui-startup");
         this.init();
         break;
       case "domwindowopened": {
         let window = aSubject;
         window.addEventListener("load", function() {
@@ -153,27 +155,30 @@ SessionStore.prototype = {
           return;
 
         // Clear all data about closed tabs
         for (let [ssid, win] in Iterator(this._windows))
           win.closedTabs = [];
 
         if (this._loadState == STATE_RUNNING) {
           // Save the purged state immediately
-          this.saveStateNow();
+          this.saveState();
         }
 
         Services.obs.notifyObservers(null, "sessionstore-state-purge-complete", "");
         break;
       case "timer-callback":
         // Timer call back for delayed saving
         this._saveTimer = null;
-        this.saveState();
+        if (this._pendingWrite) {
+          this.saveState();
+        }
         break;
       case "Session:Restore": {
+        Services.obs.removeObserver(this, "Session:Restore");
         if (aData) {
           // Be ready to handle any restore failures by making sure we have a valid tab opened
           let window = Services.wm.getMostRecentWindow("navigator:browser");
           let restoreCleanup = {
             observe: function (aSubject, aTopic, aData) {
               Services.obs.removeObserver(restoreCleanup, "sessionstore-windows-restored");
 
               if (window.BrowserApp.tabs.length == 0) {
@@ -194,16 +199,23 @@ SessionStore.prototype = {
           let data = JSON.parse(aData);
           this.restoreLastSession(data.sessionString);
         } else {
           // Not doing a restore; just send restore message
           Services.obs.notifyObservers(null, "sessionstore-windows-restored", "");
         }
         break;
       }
+      case "application-background":
+        // We receive this notification when Android's onPause callback is
+        // executed. After onPause, the application may be terminated at any
+        // point without notice; therefore, we must synchronously write out any
+        // pending save state to ensure that this data does not get lost.
+        this.flushPendingState();
+        break;
     }
   },
 
   handleEvent: function ss_handleEvent(aEvent) {
     let window = aEvent.currentTarget.ownerDocument.defaultView;
     switch (aEvent.type) {
       case "TabOpen": {
         let browser = aEvent.target;
@@ -216,21 +228,28 @@ SessionStore.prototype = {
         this.onTabRemove(window, browser);
         break;
       }
       case "TabSelect": {
         let browser = aEvent.target;
         this.onTabSelect(window, browser);
         break;
       }
-      case "pageshow": {
+      case "DOMTitleChanged": {
         let browser = aEvent.currentTarget;
-        // Top-level changes only
-        if (aEvent.originalTarget == browser.contentDocument)
-          this.onTabLoad(window, browser, aEvent.persisted);
+
+        // Handle only top-level DOMTitleChanged event
+        if (browser.contentDocument !== aEvent.originalTarget)
+          return;
+
+        // Use DOMTitleChanged to detect page loads over alternatives.
+        // onLocationChange happens too early, so we don't have the page title
+        // yet; pageshow happens too late, so we could lose session data if the
+        // browser were killed.
+        this.onTabLoad(window, browser);
         break;
       }
     }
   },
 
   onWindowOpen: function ss_onWindowOpen(aWindow) {
     // Return if window has already been initialized
     if (aWindow && aWindow.__SSID && this._windows[aWindow.__SSID])
@@ -286,24 +305,24 @@ SessionStore.prototype = {
     let tabs = aWindow.BrowserApp.tabs;
     for (let i = 0; i < tabs.length; i++)
       this.onTabRemove(aWindow, tabs[i].browser, true);
 
     delete aWindow.__SSID;
   },
 
   onTabAdd: function ss_onTabAdd(aWindow, aBrowser, aNoNotification) {
-    aBrowser.addEventListener("pageshow", this, true);
+    aBrowser.addEventListener("DOMTitleChanged", this, true);
     if (!aNoNotification)
       this.saveStateDelayed();
     this._updateCrashReportURL(aWindow);
   },
 
   onTabRemove: function ss_onTabRemove(aWindow, aBrowser, aNoNotification) {
-    aBrowser.removeEventListener("pageshow", this, true);
+    aBrowser.removeEventListener("DOMTitleChanged", this, true);
 
     // If this browser is being restored, skip any session save activity
     if (aBrowser.__SS_restore)
       return;
 
     delete aBrowser.__SS_data;
 
     if (!aNoNotification)
@@ -322,53 +341,47 @@ SessionStore.prototype = {
 
       this._windows[aWindow.__SSID].closedTabs.unshift(data);
       let length = this._windows[aWindow.__SSID].closedTabs.length;
       if (length > this._maxTabsUndo)
         this._windows[aWindow.__SSID].closedTabs.splice(this._maxTabsUndo, length - this._maxTabsUndo);
     }
   },
 
-  onTabLoad: function ss_onTabLoad(aWindow, aBrowser, aPersisted) {
+  onTabLoad: function ss_onTabLoad(aWindow, aBrowser) {
     // If this browser is being restored, skip any session save activity
     if (aBrowser.__SS_restore)
       return;
 
     // Ignore a transient "about:blank"
     if (!aBrowser.canGoBack && aBrowser.currentURI.spec == "about:blank")
       return;
 
     let history = aBrowser.sessionHistory;
 
-    if (aPersisted && aBrowser.__SS_data) {
-      // Loading from the cache; just update the index
-      aBrowser.__SS_data.index = history.index + 1;
-      this.saveStateDelayed();
-    } else {
-      // Serialize the tab data
-      let entries = [];
-      let index = history.index + 1;
-      for (let i = 0; i < history.count; i++) {
-        let historyEntry = history.getEntryAtIndex(i, false);
-        // Don't try to restore wyciwyg URLs
-        if (historyEntry.URI.schemeIs("wyciwyg")) {
-          // Adjust the index to account for skipped history entries
-          if (i <= history.index)
-            index--;
-          continue;
-        }
-        let entry = this._serializeHistoryEntry(historyEntry);
-        entries.push(entry);
+    // Serialize the tab data
+    let entries = [];
+    let index = history.index + 1;
+    for (let i = 0; i < history.count; i++) {
+      let historyEntry = history.getEntryAtIndex(i, false);
+      // Don't try to restore wyciwyg URLs
+      if (historyEntry.URI.schemeIs("wyciwyg")) {
+        // Adjust the index to account for skipped history entries
+        if (i <= history.index)
+          index--;
+        continue;
       }
-      let data = { entries: entries, index: index };
+      let entry = this._serializeHistoryEntry(historyEntry);
+      entries.push(entry);
+    }
+    let data = { entries: entries, index: index };
 
-      delete aBrowser.__SS_data;
-      this._collectTabData(aWindow, aBrowser, data);
-      this.saveStateNow();
-    }
+    delete aBrowser.__SS_data;
+    this._collectTabData(aWindow, aBrowser, data);
+    this.saveStateDelayed();
 
     this._updateCrashReportURL(aWindow);
   },
 
   onTabSelect: function ss_onTabSelect(aWindow, aBrowser) {
     if (this._loadState != STATE_RUNNING)
       return;
 
@@ -393,34 +406,44 @@ SessionStore.prototype = {
   saveStateDelayed: function ss_saveStateDelayed() {
     if (!this._saveTimer) {
       // Interval until the next disk operation is allowed
       let minimalDelay = this._lastSaveTime + this._interval - Date.now();
 
       // If we have to wait, set a timer, otherwise saveState directly
       let delay = Math.max(minimalDelay, 2000);
       if (delay > 0) {
+        this._pendingWrite++;
         this._saveTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
         this._saveTimer.init(this, delay, Ci.nsITimer.TYPE_ONE_SHOT);
       } else {
         this.saveState();
       }
     }
   },
 
-  saveStateNow: function ss_saveStateNow() {
+  saveState: function ss_saveState() {
+    this._pendingWrite++;
+    this._saveState(true);
+  },
+
+  // Immediately and synchronously writes any pending state to disk.
+  flushPendingState: function ss_flushPendingState() {
+    if (this._pendingWrite) {
+      this._saveState(false);
+    }
+  },
+
+  _saveState: function ss_saveState(aAsync) {
     // Kill any queued timer and save immediately
     if (this._saveTimer) {
       this._saveTimer.cancel();
       this._saveTimer = null;
     }
-    this.saveState();
-  },
 
-  saveState: function ss_saveState() {
     let data = this._getCurrentState();
     let normalData = { windows: [] };
     let privateData = { windows: [] };
 
     for (let winIndex = 0; winIndex < data.windows.length; ++winIndex) {
       let win = data.windows[winIndex];
       let normalWin = {};
       for (let prop in win) {
@@ -439,17 +462,17 @@ SessionStore.prototype = {
         savedWin.tabs.push(tab);
         if (win.selected == i + 1) {
           savedWin.selected = savedWin.tabs.length;
         }
       }
     }
 
     // Write only non-private data to disk
-    this._writeFile(this._sessionFile, JSON.stringify(normalData));
+    this._writeFile(this._sessionFile, JSON.stringify(normalData), aAsync);
 
     // If we have private data, send it to Java; otherwise, send null to
     // indicate that there is no private data
     this._sendMessageToJava({
       type: "PrivateBrowsing:Data",
       session: (privateData.windows[0].tabs.length > 0) ? JSON.stringify(privateData) : null
     });
 
@@ -515,30 +538,47 @@ SessionStore.prototype = {
     let windowsEnum = Services.wm.getEnumerator("navigator:browser");
     while (windowsEnum.hasMoreElements()) {
       let window = windowsEnum.getNext();
       if (window.__SSID && !window.closed)
         aFunc.call(this, window);
     }
   },
 
-  _writeFile: function ss_writeFile(aFile, aData) {
+  _writeFile: function ss_writeFile(aFile, aData, aAsync) {
     let stateString = Cc["@mozilla.org/supports-string;1"].createInstance(Ci.nsISupportsString);
     stateString.data = aData;
     Services.obs.notifyObservers(stateString, "sessionstore-state-write", "");
 
     // Don't touch the file if an observer has deleted all state data
     if (!stateString.data)
       return;
 
-    // Asynchronously copy the data to the file.
-    let array = new TextEncoder().encode(aData);
-    OS.File.writeAtomic(aFile.path, array, { tmpPath: aFile.path + ".tmp" }).then(function onSuccess() {
-      Services.obs.notifyObservers(null, "sessionstore-state-write-complete", "");
-    });
+    if (aAsync) {
+      let array = new TextEncoder().encode(aData);
+      let pendingWrite = this._pendingWrite;
+      OS.File.writeAtomic(aFile.path, array, { tmpPath: aFile.path + ".tmp" }).then(function onSuccess() {
+        // Make sure this._pendingWrite is the same value it was before we
+        // fired off the async write. If the count is different, another write
+        // is pending, so we shouldn't reset this._pendingWrite yet.
+        if (pendingWrite === this._pendingWrite)
+          this._pendingWrite = 0;
+        Services.obs.notifyObservers(null, "sessionstore-state-write-complete", "");
+      }.bind(this));
+    } else {
+      this._pendingWrite = 0;
+      let foStream = Cc["@mozilla.org/network/file-output-stream;1"].
+                     createInstance(Ci.nsIFileOutputStream);
+      foStream.init(aFile, 0x02 | 0x08 | 0x20, 0666, 0);
+      let converter = Cc["@mozilla.org/intl/converter-output-stream;1"].
+                      createInstance(Ci.nsIConverterOutputStream);
+      converter.init(foStream, "UTF-8", 0, 0);
+      converter.writeString(aData);
+      converter.close();
+    }
   },
 
   _updateCrashReportURL: function ss_updateCrashReportURL(aWindow) {
 #ifdef MOZ_CRASHREPORTER
     if (!aWindow.BrowserApp.selectedBrowser)
       return;
 
     try {