Bug 1447551 Part 1: Fix some issues with persistent EventManagers r=kmag
☠☠ backed out by 9d3390130f27 ☠ ☠
authorAndrew Swan <aswan@mozilla.com>
Fri, 20 Apr 2018 16:09:13 -0700
changeset 468962 b6f7de464d7f6fc953aaa4022c6c48cf879fcb10
parent 468961 544e2832e78306fc04c7cbaea81a40b0189ae155
child 468963 82d178d9966ddd126ec1cf3c0ff90173c9ad4a80
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
@@ -45,16 +45,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 {
@@ -1700,16 +1703,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) {
@@ -1791,24 +1800,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});
         }
       }
     }
   }
@@ -1918,25 +1933,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,13 @@ 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,
-  });
-}
-
 // 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 = ExtensionUtils.promiseObserved("browser-delayed-startup-finished");
+global.browserStartupPromise = ExtensionUtils.promiseObserved("sessionstore-windows-restored");