Bug 1447551 Part 1: Fix some issues with persistent EventManagers r=kmag
authorAndrew Swan <aswan@mozilla.com>
Fri, 20 Apr 2018 16:09:13 -0700
changeset 469208 49295192e8a854f21854f70d658f04cb51e8aa36
parent 469207 43fb3472db5f3c1e811697993316cfd4e434cfdb
child 469209 129e90aed47a91b44a0b8ac3b3c44b0b8b9c72ca
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1447551
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1447551 Part 1: Fix some issues with persistent EventManagers r=kmag - Un-lazify the startup promises in ext-toolkit.js since the manifest background property is handled asynchronously, so it races with startup and can miss the relevant events if it loses the race. - Ensure that persistent events don't cause breakage when the background-delayed-startup preference is set to false. - Add a wakeup() method to the fire object provided to primed listeners. This method returns a Promise that resolves when the extension background page has started. Events that need to do some work in the context of the extension can wait on the result of wakeup(), then continue processing after the background page is started, using fire.[a?]sync as normal. MozReview-Commit-ID: HiYOguVdEQK
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/parent/ext-backgroundPage.js
toolkit/components/extensions/parent/ext-toolkit.js
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -44,16 +44,19 @@ var {
   getConsole,
   getInnerWindowID,
   getUniqueId,
   getWinUtils,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyGetter(this, "console", getConsole);
 
+XPCOMUtils.defineLazyPreferenceGetter(this, "DELAYED_BG_STARTUP",
+                                      "extensions.webextensions.background-delayed-startup");
+
 var ExtensionCommon;
 
 /**
  * A sentinel class to indicate that an array of values should be
  * treated as an array when used as a promise resolution value, but as a
  * spread expression (...args) when passed to a callback.
  */
 class SpreadArgs extends Array {
@@ -1699,16 +1702,22 @@ class EventManager {
       let {context, name, register, inputHandling = false, persistent = null} = params;
       this.context = context;
       this.name = name;
       this.register = register;
       this.inputHandling = inputHandling;
       this.persistent = persistent;
     }
 
+    // Don't bother with persistent event handling if delayed background
+    // startup is not enabled.
+    if (!DELAYED_BG_STARTUP) {
+      this.persistent = null;
+    }
+
     this.unregister = new Map();
     this.remove = new Map();
 
     if (this.persistent) {
       if (this.context.viewType !== "background") {
         this.persistent = null;
       }
       if (AppConstants.DEBUG) {
@@ -1790,24 +1799,30 @@ class EventManager {
 
     for (let [module, moduleEntry] of extension.persistentListeners) {
       let api = extension.apiManager.getAPI(module, extension, "addon_parent");
       for (let [event, eventEntry] of moduleEntry) {
         for (let listener of eventEntry.values()) {
           let primed = {pendingEvents: []};
           listener.primed = primed;
 
-          let wakeup = (...args) => new Promise((resolve, reject) => {
+          let wakeup = () => new Promise(resolve => {
+            extension.once("startup", resolve);
+            extension.emit("background-page-event");
+          });
+
+          let fireEvent = (...args) => new Promise((resolve, reject) => {
             primed.pendingEvents.push({args, resolve, reject});
             extension.emit("background-page-event");
           });
 
           let fire = {
-            sync: wakeup,
-            async: wakeup,
+            wakeup,
+            sync: fireEvent,
+            async: fireEvent,
           };
 
           let {unregister, convert} = api.primeListener(extension, event, fire, listener.params);
           Object.assign(primed, {unregister, convert});
         }
       }
     }
   }
@@ -1917,25 +1932,32 @@ class EventManager {
     if (this.persistent) {
       recordStartupData = true;
       let {module, event} = this.persistent;
 
       let key = uneval(args);
       EventManager._initPersistentListeners(extension);
       let listener = extension.persistentListeners
                               .get(module).get(event).get(key);
-      if (listener) {
-        let {primed} = listener;
-        listener.primed = null;
 
-        primed.convert(fire);
-        unregister = primed.unregister;
+      if (listener) {
+        // If extensions.webextensions.background-delayed-startup is disabled,
+        // we can have stored info here but no primed listener.  This check
+        // can be removed if/when we make delayed background startup the only
+        // supported setting.
+        let {primed} = listener;
+        if (primed) {
+          listener.primed = null;
 
-        for (let evt of primed.pendingEvents) {
-          evt.resolve(fire.async(...evt.args));
+          primed.convert(fire, this.context);
+          unregister = primed.unregister;
+
+          for (let evt of primed.pendingEvents) {
+            evt.resolve(fire.async(...evt.args));
+          }
         }
 
         recordStartupData = false;
         this.remove.set(callback, () => {
           EventManager.clearPersistentListener(extension, module, event, uneval(args));
         });
       }
     }
--- a/toolkit/components/extensions/parent/ext-backgroundPage.js
+++ b/toolkit/components/extensions/parent/ext-backgroundPage.js
@@ -74,17 +74,16 @@ this.backgroundPage = class extends Exte
 
     // There are two ways to start the background page:
     // 1. If a primed event fires, then start the background page as
     //    soon as we have painted a browser window.  Note that we have
     //    to touch browserPaintedPromise here to initialize the listener
     //    or else we can miss it if the event occurs after the first
     //    window is painted but before #2
     // 2. After all windows have been restored.
-    void browserPaintedPromise;
     extension.once("background-page-event", async () => {
       await browserPaintedPromise;
       extension.emit("start-background-page");
     });
 
     browserStartupPromise.then(() => {
       extension.emit("start-background-page");
     });
--- a/toolkit/components/extensions/parent/ext-toolkit.js
+++ b/toolkit/components/extensions/parent/ext-toolkit.js
@@ -70,28 +70,17 @@ global.getContainerForCookieStoreId = fu
 };
 
 global.isValidCookieStoreId = function(storeId) {
   return isDefaultCookieStoreId(storeId) ||
          isPrivateCookieStoreId(storeId) ||
          isContainerCookieStoreId(storeId);
 };
 
-function makeEventPromise(name, event) {
-  Object.defineProperty(global, name, {
-    get() {
-      let promise = ExtensionUtils.promiseObserved(event);
-      Object.defineProperty(global, name, {value: promise});
-      return promise;
-    },
-    configurable: true,
-    enumerable: true,
-  });
+function makeStartupPromise(event) {
+  return ExtensionUtils.promiseObserved(event).then(() => {});
 }
 
 // browserPaintedPromise and browserStartupPromise are promises that
 // resolve after the first browser window is painted and after browser
 // windows have been restored, respectively.
-// These promises must be referenced during startup to be valid -- if the
-// first reference happens after the corresponding event has occurred,
-// the Promise will never resolve.
-makeEventPromise("browserPaintedPromise", "browser-delayed-startup-finished");
-makeEventPromise("browserStartupPromise", "sessionstore-windows-restored");
+global.browserPaintedPromise = makeStartupPromise("browser-delayed-startup-finished");
+global.browserStartupPromise = makeStartupPromise("sessionstore-windows-restored");