Skip to content

Commit

Permalink
Bug 1651844 - Ensure that readyPromise is resolved when startup() exi…
Browse files Browse the repository at this point in the history
…ts early r=rpl

This is necessary because otherwise callers of policy.readyPromise can
get stuck when an extension fails to start up.

Differential Revision: https://phabricator.services.mozilla.com/D83143
  • Loading branch information
Rob--W committed Jul 21, 2020
1 parent 938eda0 commit f75d9d8
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 2 deletions.
6 changes: 4 additions & 2 deletions dom/chrome-webidl/WebExtensionPolicy.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,11 @@ interface WebExtensionPolicy {
* This may be used to delay operations, such as loading extension pages,
* which depend on extensions being fully initialized.
*
* Note: This will always be either a Promise<WebExtensionPolicy> or null,
* Note: This will always be either a Promise<WebExtensionPolicy?> or null,
* but the WebIDL grammar does not allow us to specify a nullable Promise
* type.
*
* Note: This could resolve to null when the startup was interrupted.
*/
readonly attribute object? readyPromise;

Expand Down Expand Up @@ -270,5 +272,5 @@ dictionary WebExtensionInit {
sequence<DOMString>? backgroundScripts = null;
DOMString? backgroundWorkerScript = null;

Promise<WebExtensionPolicy> readyPromise;
Promise<WebExtensionPolicy?> readyPromise;
};
5 changes: 5 additions & 0 deletions toolkit/components/extensions/Extension.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -2396,6 +2396,8 @@ class Extension extends ExtensionData {
async startup() {
this.state = "Startup";

// readyPromise is resolved with the policy upon success,
// and with null if startup was interrupted.
let resolveReadyPromise;
let readyPromise = new Promise(resolve => {
resolveReadyPromise = resolve;
Expand Down Expand Up @@ -2560,6 +2562,9 @@ class Extension extends ExtensionData {
throw e;
} finally {
ExtensionTelemetry.extensionStartup.stopwatchFinish(this);
// Mark readyPromise as resolved in case it has not happened before,
// e.g. due to an early return or an error.
resolveReadyPromise(null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ add_task(async function extension_startup_early_error() {

let startupPromise = extension.startup();

let policy = WebExtensionPolicy.getByID(EXTENSION_ID);
ok(policy, "WebExtensionPolicy instantiated at startup");
let readyPromise = policy.readyPromise;
ok(readyPromise, "WebExtensionPolicy.readyPromise is set");

await Assert.rejects(
startupPromise,
/dummy error/,
Expand All @@ -32,4 +37,10 @@ add_task(async function extension_startup_early_error() {
null,
"WebExtensionPolicy should be unregistered"
);

Assert.equal(
await readyPromise,
null,
"policy.readyPromise should be resolved with null"
);
});

0 comments on commit f75d9d8

Please sign in to comment.