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 469038 50a1ddd35664449c908ea326b364d367871e78a9
parent 469037 07b3234cc1c04bc56afd69ba5b49cc970dbf8189
child 469039 cbae9dd052934b6292f8e8675eb6cf7cde78333d
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,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");