Bug 1501375 - Clear primed listeners on extension shutdown r=aswan
authorRob Wu <rob@robwu.nl>
Wed, 20 Feb 2019 21:53:08 +0000
changeset 518754 3a5c6e81192dbdb0a8adfb20e00181d3de5540f7
parent 518753 2090ee542b58d0610623717a02a1a3dea1ceb1e0
child 518755 04b242fc5a70c63cf5276adc0a4e314d9e4de592
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1501375
milestone67.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 1501375 - Clear primed listeners on extension shutdown r=aswan In D9959 and D10954, the implementation of background page machinery was updated so that the `build` method of `BackgroundPage` is guaranteed to return eventually, without error. Even if the load of the background page was interrupted by an extension shut down. When an extension shuts down during start-up, primed listeners have likely not been re-registered. To prevent interruption of startup from causing the loss of persistent listeners, this commit changes the implementation such that persistent listeners are only updated if the background page was fully loaded. Otherwise listeners are cleared from memory, but not unregistered. Depends on D10954 Differential Revision: https://phabricator.services.mozilla.com/D17700
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/parent/ext-backgroundPage.js
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -2152,30 +2152,35 @@ class EventManager {
     let listener = extension.persistentListeners.get(module).get(event).get(key);
     if (listener.primed) {
       listener.primed = null;
     }
   }
 
   // Remove any primed listeners that were not re-registered.
   // This function is called after the background page has started.
-  static clearPrimedListeners(extension) {
+  // The removed listeners are removed from the set of saved listeners, unless
+  // `clearPersistent` is false. If false, the listeners are cleared from
+  // memory, but not removed from the extension's startup data.
+  static clearPrimedListeners(extension, clearPersistent = true) {
     for (let [module, moduleEntry] of extension.persistentListeners) {
       for (let [event, listeners] of moduleEntry) {
         for (let [key, listener] of listeners) {
           let {primed} = listener;
           if (!primed) {
             continue;
           }
 
           for (let evt of primed.pendingEvents) {
             evt.reject(new Error("listener not re-registered"));
           }
 
-          EventManager.clearPersistentListener(extension, module, event, key);
+          if (clearPersistent) {
+            EventManager.clearPersistentListener(extension, module, event, key);
+          }
           primed.unregister();
         }
       }
     }
   }
 
   // Record the fact that there is a listener for the given event in
   // the given extension.  `args` is an Array containing any extra
--- a/toolkit/components/extensions/parent/ext-backgroundPage.js
+++ b/toolkit/components/extensions/parent/ext-backgroundPage.js
@@ -92,18 +92,26 @@ this.backgroundPage = class extends Exte
 
     if (extension.startupReason !== "APP_STARTUP" || !DELAYED_STARTUP) {
       return this.build();
     }
 
     EventManager.primeListeners(extension);
 
     extension.once("start-background-page", async () => {
+      if (!this.extension) {
+        // Extension was shut down. Don't build the background page.
+        // Primed listeners have been cleared in onShutdown.
+        return;
+      }
       await this.build();
-      EventManager.clearPrimedListeners(extension);
+      // |this.extension| may be null if the extension was shut down.
+      // In that case, we still want to clear the primed listeners,
+      // but not update the persistent listeners in the startupData.
+      EventManager.clearPrimedListeners(extension, !!this.extension);
     });
 
     // 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
@@ -116,11 +124,13 @@ this.backgroundPage = class extends Exte
     ExtensionParent.browserStartupPromise.then(() => {
       extension.emit("start-background-page");
     });
   }
 
   onShutdown() {
     if (this.bgPage) {
       this.bgPage.shutdown();
+    } else {
+      EventManager.clearPrimedListeners(this.extension, false);
     }
   }
 };