Bug 1250784: Part 2 - [webext] Fix some issues that cause noisy tests. r=Mossop
MozReview-Commit-ID: 93NMinJpWRo
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4634,28 +4634,37 @@ var CombinedStopReload = {
if (this._timer) {
clearTimeout(this._timer);
this._timer = 0;
}
}
};
var TabsProgressListener = {
+ // Keep track of which browsers we've started load timers for, since
+ // we won't see STATE_START events for pre-rendered tabs.
+ _startedLoadTimer: new WeakSet(),
+
onStateChange: function (aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) {
// Collect telemetry data about tab load times.
if (aWebProgress.isTopLevel && (!aRequest.originalURI || aRequest.originalURI.spec.scheme != "about")) {
if (aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW) {
if (aStateFlags & Ci.nsIWebProgressListener.STATE_START) {
+ this._startedLoadTimer.add(aBrowser);
TelemetryStopwatch.start("FX_PAGE_LOAD_MS", aBrowser);
Services.telemetry.getHistogramById("FX_TOTAL_TOP_VISITS").add(true);
- } else if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) {
+ } else if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
+ this._startedLoadTimer.has(aBrowser)) {
+ this._startedLoadTimer.delete(aBrowser);
TelemetryStopwatch.finish("FX_PAGE_LOAD_MS", aBrowser);
}
} else if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
- aStatus == Cr.NS_BINDING_ABORTED) {
+ aStatus == Cr.NS_BINDING_ABORTED &&
+ this._startedLoadTimer.has(aBrowser)) {
+ this._startedLoadTimer.delete(aBrowser);
TelemetryStopwatch.cancel("FX_PAGE_LOAD_MS", aBrowser);
}
}
// Attach a listener to watch for "click" events bubbling up from error
// pages and other similar pages (like about:newtab). This lets us fix bugs
// like 401575 which require error page UI to do privileged things, without
// letting error pages have any privilege themselves.
--- a/browser/components/extensions/ext-bookmarks.js
+++ b/browser/components/extensions/ext-bookmarks.js
@@ -90,17 +90,17 @@ extensions.registerSchemaAPI("bookmarks"
for (let id of list) {
let bookmark = yield Bookmarks.fetch({guid: id});
if (!bookmark) {
throw new Error("Bookmark not found");
}
bookmarks.push(convert(bookmark));
}
return bookmarks;
- });
+ }).catch(error => Promise.reject({message: error.message}));
},
getChildren: function(id) {
// TODO: We should optimize this.
return getTree(id, true);
},
getTree: function() {
@@ -138,17 +138,18 @@ extensions.registerSchemaAPI("bookmarks"
if (bookmark.parentId !== null) {
info.parentGuid = bookmark.parentId;
} else {
info.parentGuid = Bookmarks.unfiledGuid;
}
try {
- return Bookmarks.insert(info).then(convert);
+ return Bookmarks.insert(info).then(convert)
+ .catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
move: function(id, destination) {
let info = {
guid: id,
@@ -157,17 +158,18 @@ extensions.registerSchemaAPI("bookmarks"
if (destination.parentId !== null) {
info.parentGuid = destination.parentId;
}
if (destination.index !== null) {
info.index = destination.index;
}
try {
- return Bookmarks.update(info).then(convert);
+ return Bookmarks.update(info).then(convert)
+ .catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
update: function(id, changes) {
let info = {
guid: id,
@@ -176,42 +178,45 @@ extensions.registerSchemaAPI("bookmarks"
if (changes.title !== null) {
info.title = changes.title;
}
if (changes.url !== null) {
info.url = changes.url;
}
try {
- return Bookmarks.update(info).then(convert);
+ return Bookmarks.update(info).then(convert)
+ .catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
remove: function(id) {
let info = {
guid: id,
};
// The API doesn't give you the old bookmark at the moment
try {
- return Bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true}).then(result => {});
+ return Bookmarks.remove(info, {preventRemovalOfNonEmptyFolders: true}).then(result => {})
+ .catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
removeTree: function(id) {
let info = {
guid: id,
};
try {
- return Bookmarks.remove(info).then(result => {});
+ return Bookmarks.remove(info).then(result => {})
+ .catch(error => Promise.reject({message: error.message}));
} catch (e) {
return Promise.reject({message: `Invalid bookmark: ${JSON.stringify(info)}`});
}
},
},
};
});
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -126,14 +126,16 @@ function clickPageAction(extension, win
EventUtils.synthesizeMouseAtCenter(elem, {}, win);
return new Promise(SimpleTest.executeSoon);
}
function closePageAction(extension, win = window) {
let node = getPageActionPopup(extension, win);
if (node) {
- node.hidePopup();
+ return promisePopupShown(node).then(() => {
+ node.hidePopup();
+ });
}
return Promise.resolve();
}
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -392,22 +392,17 @@ GlobalManager = {
if (callback === null) {
callback = defaultCallback;
}
let promise;
try {
promise = findPath(path)[name](...args);
} catch (e) {
- if (e instanceof context.cloneScope.Error) {
- promise = Promise.reject(e);
- } else {
- Cu.reportError(e);
- promise = Promise.reject({message: "An unexpected error occurred"});
- }
+ promise = Promise.reject(e);
}
return context.wrapPromise(promise || Promise.resolve(), callback);
},
shouldInject(path, name) {
return findPath(path) != null;
},
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -197,35 +197,52 @@ class BaseContext {
}
set lastError(val) {
this.checkedLastError = false;
this._lastError = val;
}
/**
+ * Normalizes the given error object for use by the target scope. If
+ * the target is an error object which belongs to that scope, it is
+ * returned as-is. If it is an ordinary object with a `message`
+ * property, it is converted into an error belonging to the target
+ * scope. If it is an Error object which does *not* belong to the
+ * clone scope, it is reported, and converted to an unexpected
+ * exception error.
+ */
+ normalizeError(error) {
+ if (error instanceof this.cloneScope.Error) {
+ return error;
+ }
+ if (!instanceOf(error, "Object")) {
+ Cu.reportError(error);
+ error = {message: "An unexpected error occurred"};
+ }
+ return new this.cloneScope.Error(error.message);
+ }
+
+ /**
* Sets the value of `.lastError` to `error`, calls the given
* callback, and reports an error if the value has not been checked
* when the callback returns.
*
* @param {object} error An object with a `message` property. May
* optionally be an `Error` object belonging to the target scope.
* @param {function} callback The callback to call.
* @returns {*} The return value of callback.
*/
withLastError(error, callback) {
- if (!(error instanceof this.cloneScope.Error)) {
- error = new this.cloneScope.Error(error.message);
- }
- this.lastError = error;
+ this.lastError = this.normalizeError(error);
try {
return callback();
} finally {
if (!this.checkedLastError) {
- Cu.reportError(`Unchecked lastError value: ${error}`);
+ Cu.reportError(`Unchecked lastError value: ${this.lastError}`);
}
this.lastError = null;
}
}
/**
* Wraps the given promise so it can be safely returned to extension
* code in this context.
@@ -273,20 +290,17 @@ class BaseContext {
runSafeSyncWithoutClone(callback);
});
});
} else {
return new this.cloneScope.Promise((resolve, reject) => {
promise.then(
value => { runSafe(resolve, value); },
value => {
- if (!(value instanceof this.cloneScope.Error)) {
- value = new this.cloneScope.Error(value.message);
- }
- runSafeSyncWithoutClone(reject, value);
+ runSafeSyncWithoutClone(reject, this.normalizeError(value));
});
});
}
}
unload() {
MessageChannel.abortResponses({
extensionId: this.extension.id,