Bug 918024 - Remove the synchronous fallback for reading in Session Store initialization. r=Yoric, f=ttaubert

This commit is contained in:
Steven MacLeod
2013-11-19 14:57:46 -05:00
parent 0ecec6d58d
commit 2f938f6bbb
11 changed files with 47 additions and 170 deletions

View File

@@ -549,7 +549,7 @@ nsBrowserContentHandler.prototype = {
}
var overridePage = "";
var haveUpdateSession = false;
var willRestoreSession = false;
try {
// Read the old value of homepage_override.mstone before
// needHomepageOverride updates it, so that we can later add it to the
@@ -568,11 +568,15 @@ nsBrowserContentHandler.prototype = {
overridePage = Services.urlFormatter.formatURLPref("startup.homepage_welcome_url");
break;
case OVERRIDE_NEW_MSTONE:
// Check whether we have a session to restore. If we do, we assume
// that this is an "update" session.
// Check whether we will restore a session. If we will, we assume
// that this is an "update" session. This does not take crashes
// into account because that requires waiting for the session file
// to be read. If a crash occurs after updating, before restarting,
// we may open the startPage in addition to restoring the session.
var ss = Components.classes["@mozilla.org/browser/sessionstartup;1"]
.getService(Components.interfaces.nsISessionStartup);
haveUpdateSession = ss.doRestore();
willRestoreSession = ss.isAutomaticRestoreEnabled();
overridePage = Services.urlFormatter.formatURLPref("startup.homepage_override_url");
if (prefb.prefHasUserValue("app.update.postupdate"))
overridePage = getPostUpdateOverridePage(overridePage);
@@ -600,7 +604,7 @@ nsBrowserContentHandler.prototype = {
startPage = "";
// Only show the startPage if we're not restoring an update session.
if (overridePage && startPage && !haveUpdateSession)
if (overridePage && startPage && !willRestoreSession)
return overridePage + "|" + startPage;
return overridePage || startPage || "about:blank";

View File

@@ -10,7 +10,7 @@
* - and allows to restore everything into one window.
*/
[scriptable, uuid(51f4b9f0-f3d2-11e2-bb62-2c24dd830245)]
[scriptable, uuid(6c79d4c1-f071-4c5c-a7fb-676adb144584)]
interface nsISessionStartup: nsISupports
{
/**
@@ -23,12 +23,18 @@ interface nsISessionStartup: nsISupports
readonly attribute jsval state;
/**
* Determines whether there is a pending session restore and makes sure that
* we're initialized before returning. If we're not yet this will read the
* session file synchronously.
* Determines whether there is a pending session restore. Should only be
* called after initialization has completed.
*/
boolean doRestore();
/**
* Determines whether automatic session restoration is enabled for this
* launch of the browser. This does not include crash restoration, and will
* return false if restoration will only be caused by a crash.
*/
boolean isAutomaticRestoreEnabled();
/**
* Returns whether we will restore a session that ends up replacing the
* homepage. The browser uses this to not start loading the homepage if

View File

@@ -38,16 +38,10 @@ Cu.import("resource://gre/modules/AsyncShutdown.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
"resource://gre/modules/TelemetryStopwatch.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
"resource://gre/modules/NetUtil.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
"resource://gre/modules/FileUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Task",
"resource://gre/modules/Task.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "Telemetry",
"@mozilla.org/base/telemetry;1", "nsITelemetry");
XPCOMUtils.defineLazyModuleGetter(this, "Deprecated",
"resource://gre/modules/Deprecated.jsm");
this.SessionFile = {
/**
@@ -56,15 +50,6 @@ this.SessionFile = {
read: function () {
return SessionFileInternal.read();
},
/**
* Read the contents of the session file, synchronously.
*/
syncRead: function () {
Deprecated.warning(
"syncRead is deprecated and will be removed in a future version",
"https://bugzilla.mozilla.org/show_bug.cgi?id=532150")
return SessionFileInternal.syncRead();
},
/**
* Write the contents of the session file, asynchronously.
*/
@@ -161,60 +146,6 @@ let SessionFileInternal = {
*/
_isClosed: false,
/**
* Utility function to safely read a file synchronously.
* @param aPath
* A path to read the file from.
* @returns string if successful, undefined otherwise.
*/
readAuxSync: function (aPath) {
let text;
try {
let file = new FileUtils.File(aPath);
let chan = NetUtil.newChannel(file);
let stream = chan.open();
text = NetUtil.readInputStreamToString(stream, stream.available(),
{charset: "utf-8"});
} catch (e if e.result == Components.results.NS_ERROR_FILE_NOT_FOUND) {
// Ignore exceptions about non-existent files.
} catch (ex) {
// Any other error.
Cu.reportError(ex);
} finally {
return text;
}
},
/**
* Read the sessionstore file synchronously.
*
* This function is meant to serve as a fallback in case of race
* between a synchronous usage of the API and asynchronous
* initialization.
*
* In case if sessionstore.js file does not exist or is corrupted (something
* happened between backup and write), attempt to read the sessionstore.bak
* instead.
*/
syncRead: function () {
// Start measuring the duration of the synchronous read.
TelemetryStopwatch.start("FX_SESSION_RESTORE_SYNC_READ_FILE_MS");
// First read the sessionstore.js.
let text = this.readAuxSync(this.path);
if (typeof text === "undefined") {
// If sessionstore.js does not exist or is corrupted, read sessionstore.bak.
text = this.readAuxSync(this.backupPath);
}
// Finish the telemetry probe and return an empty string.
TelemetryStopwatch.finish("FX_SESSION_RESTORE_SYNC_READ_FILE_MS");
text = text || "";
// The worker needs to know the initial state read from
// disk so that writeLoadStateOnceAfterStartup() works.
SessionWorker.post("setInitialState", [text]);
return text;
},
read: function () {
return SessionWorker.post("read").then(msg => {
this._recordTelemetry(msg.telemetry);

View File

@@ -73,30 +73,6 @@ let Agent = {
// The path to sessionstore.bak
backupPath: OS.Path.join(OS.Constants.Path.profileDir, "sessionstore.bak"),
/**
* This method is only intended to be called by SessionFile.syncRead() and
* can be removed when we're not supporting synchronous SessionStore
* initialization anymore. When sessionstore.js is read from disk
* synchronously the state string must be supplied to the worker manually by
* calling this method.
*/
setInitialState: function (aState) {
// SessionFile.syncRead() should not be called after startup has finished.
// Thus we also don't support any setInitialState() calls after we already
// wrote the loadState to disk.
if (this.hasWrittenLoadStateOnce) {
throw new Error("writeLoadStateOnceAfterStartup() must only be called once.");
}
// Initial state might have been filled by read() already but yet we might
// be called by SessionFile.syncRead() before SessionStore.jsm had a chance
// to call writeLoadStateOnceAfterStartup(). It's safe to ignore
// setInitialState() calls if this happens.
if (!this.initialState) {
this.initialState = aState;
}
},
/**
* Read the session from disk.
* In case sessionstore.js does not exist, attempt to read sessionstore.bak.

View File

@@ -48,6 +48,9 @@ XPCOMUtils.defineLazyModuleGetter(this, "SessionFile",
const STATE_RUNNING_STR = "running";
// 'browser.startup.page' preference value to resume the previous session.
const BROWSER_STARTUP_RESUME_SESSION = 3;
function debug(aMsg) {
aMsg = ("SessionStartup: " + aMsg).replace(/\S{80}/g, "$&\n");
Services.console.logStringMessage(aMsg);
@@ -138,7 +141,7 @@ SessionStartup.prototype = {
let doResumeSessionOnce = Services.prefs.getBoolPref("browser.sessionstore.resume_session_once");
let doResumeSession = doResumeSessionOnce ||
Services.prefs.getIntPref("browser.startup.page") == 3;
Services.prefs.getIntPref("browser.startup.page") == BROWSER_STARTUP_RESUME_SESSION;
// If this is a normal restore then throw away any previous session
if (!doResumeSessionOnce)
@@ -226,9 +229,9 @@ SessionStartup.prototype = {
},
/**
* Determines whether there is a pending session restore and makes sure that
* we're initialized before returning. If we're not yet this will read the
* session file synchronously.
* Determines whether there is a pending session restore. Should only be
* called after initialization has completed.
* @throws Error if initialization is not complete yet.
* @returns bool
*/
doRestore: function sss_doRestore() {
@@ -236,6 +239,18 @@ SessionStartup.prototype = {
return this._willRestore();
},
/**
* Determines whether automatic session restoration is enabled for this
* launch of the browser. This does not include crash restoration. In
* particular, if session restore is configured to restore only in case of
* crash, this method returns false.
* @returns bool
*/
isAutomaticRestoreEnabled: function () {
return Services.prefs.getBoolPref("browser.sessionstore.resume_session_once") ||
Services.prefs.getIntPref("browser.startup.page") == BROWSER_STARTUP_RESUME_SESSION;
},
/**
* Determines whether there is a pending session restore.
* @returns bool
@@ -275,20 +290,12 @@ SessionStartup.prototype = {
return this._sessionType;
},
// Ensure that initialization is complete.
// If initialization is not complete yet, fall back to a synchronous
// initialization and kill ongoing asynchronous initialization
// Ensure that initialization is complete. If initialization is not complete
// yet, something is attempting to use the old synchronous initialization,
// throw an error.
_ensureInitialized: function sss__ensureInitialized() {
try {
if (this._initialized) {
// Initialization is complete, nothing else to do
return;
}
let contents = SessionFile.syncRead();
this._onSessionFileRead(contents);
} catch(ex) {
debug("ensureInitialized: could not read session " + ex + ", " + ex.stack);
throw ex;
if (!this._initialized) {
throw new Error("Session Store is not initialized.");
}
},

View File

@@ -104,11 +104,6 @@ function testReadBackup() {
let ssDataRead = yield SessionFile.read();
is(ssDataRead, gSSData, "SessionFile.read read sessionstore.js correctly.");
// Read sessionstore.js with SessionFile.syncRead.
ssDataRead = SessionFile.syncRead();
is(ssDataRead, gSSData,
"SessionFile.syncRead read sessionstore.js correctly.");
// Remove sessionstore.js to test fallback onto sessionstore.bak.
yield OS.File.remove(path);
ssExists = yield OS.File.exists(path);
@@ -119,11 +114,6 @@ function testReadBackup() {
is(ssDataRead, gSSBakData,
"SessionFile.read read sessionstore.bak correctly.");
// Read sessionstore.bak with SessionFile.syncRead.
ssDataRead = SessionFile.syncRead();
is(ssDataRead, gSSBakData,
"SessionFile.syncRead read sessionstore.bak correctly.");
nextTest(testBackupUnchanged);
}

View File

@@ -1,15 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
// Test nsISessionStartup.sessionType in the following scenario:
// - no sessionstore.js;
// - the session store has not been loaded yet, so we have to trigger
// synchronous fallback
function run_test() {
do_get_profile();
let startup = Cc["@mozilla.org/browser/sessionstartup;1"].
getService(Ci.nsISessionStartup);
do_check_eq(startup.sessionType, Ci.nsISessionStartup.NO_SESSION);
}

View File

@@ -1,17 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
// Test nsISessionStartup.sessionType in the following scenario:
// - valid sessionstore.js;
// - the session store has not been loaded yet, so we have to trigger
// synchronous fallback
function run_test() {
let profd = do_get_profile();
let source = do_get_file("data/sessionstore_valid.js");
source.copyTo(profd, "sessionstore.js");
let startup = Cc["@mozilla.org/browser/sessionstartup;1"].
getService(Ci.nsISessionStartup);
do_check_eq(startup.sessionType, Ci.nsISessionStartup.DEFER_SESSION);
}

View File

@@ -6,7 +6,5 @@ support-files = data/sessionstore_valid.js
[test_backup.js]
[test_backup_once.js]
[test_startup_nosession_sync.js]
[test_startup_nosession_async.js]
[test_startup_session_sync.js]
[test_startup_session_async.js]

View File

@@ -118,6 +118,10 @@ function test()
{
waitForExplicitFinish();
// Reset the startup page pref since it may have been set by other tests
// and we will assume it is default.
Services.prefs.clearUserPref('browser.startup.page');
if (gPrefService.prefHasUserValue(PREF_MSTONE)) {
gOriginalMStone = gPrefService.getCharPref(PREF_MSTONE);
}

View File

@@ -2804,13 +2804,6 @@
"extended_statistics_ok": true,
"description": "Session restore: Time to read the session data from the file on disk (ms)"
},
"FX_SESSION_RESTORE_SYNC_READ_FILE_MS": {
"kind": "exponential",
"high": "3000",
"n_buckets": 10,
"extended_statistics_ok": true,
"description": "Session restore: Time to read the session data from the file on disk, using the synchronous fallback (ms)"
},
"FX_SESSION_RESTORE_WRITE_FILE_MS": {
"kind": "exponential",
"high": "3000",