Bug 1286526 - [webext] Fix shutdown issues on empty addon with a valid manifest. r=aswan draft
authorLuca Greco <lgreco@mozilla.com>
Wed, 13 Jul 2016 13:12:59 +0200
changeset 392958 4b8fcc9be4e9f0e9d4735e90c58f8f923421a02f
parent 392957 39215aa3dfecff2746cd2e6c1be2054ae0c3daf0
child 392959 5d4c9e9ae90a2824eef4cc6ed175e175e43d1125
push id24157
push userluca.greco@alcacoop.it
push dateTue, 26 Jul 2016 16:01:12 +0000
reviewersaswan
bugs1286526
milestone50.0a1
Bug 1286526 - [webext] Fix shutdown issues on empty addon with a valid manifest. r=aswan MozReview-Commit-ID: 30MryAPYBv0
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ext-alarms.js
toolkit/components/extensions/ext-notifications.js
toolkit/components/extensions/test/mochitest/chrome.ini
toolkit/components/extensions/test/mochitest/test_chrome_ext_shutdown_cleanup.html
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -478,54 +478,65 @@ let ParentAPIManager = {
     findPathInObject(context.apiObj, data.path)[data.name].removeListener(listener);
   },
 };
 
 ParentAPIManager.init();
 
 // For extensions that have called setUninstallURL(), send an event
 // so the browser can display the URL.
-let UninstallObserver = {
+var UninstallObserver = {
   initialized: false,
 
   init: function() {
     if (!this.initialized) {
       AddonManager.addAddonListener(this);
       this.initialized = true;
     }
   },
 
+  uninit: function() {
+    if (this.initialized) {
+      AddonManager.removeAddonListener(this);
+      this.initialized = false;
+    }
+  },
+
   onUninstalling: function(addon) {
     let extension = GlobalManager.extensionMap.get(addon.id);
     if (extension) {
       Management.emit("uninstall", extension);
     }
   },
 };
 
 // Responsible for loading extension APIs into the right globals.
 GlobalManager = {
   // Map[extension ID -> Extension]. Determines which extension is
   // responsible for content under a particular extension ID.
   extensionMap: new Map(),
+  initialized: false,
 
   init(extension) {
     if (this.extensionMap.size == 0) {
       Services.obs.addObserver(this, "content-document-global-created", false);
       UninstallObserver.init();
+      this.initialized = true;
     }
 
     this.extensionMap.set(extension.id, extension);
   },
 
   uninit(extension) {
     this.extensionMap.delete(extension.id);
 
-    if (this.extensionMap.size == 0) {
+    if (this.extensionMap.size == 0 && this.initialized) {
       Services.obs.removeObserver(this, "content-document-global-created");
+      UninstallObserver.uninit();
+      this.initialized = false;
     }
   },
 
   getExtension(extensionId) {
     return this.extensionMap.get(extensionId);
   },
 
   injectInObject(extension, context, defaultCallback, dest, namespaces = null) {
--- a/toolkit/components/extensions/ext-alarms.js
+++ b/toolkit/components/extensions/ext-alarms.js
@@ -78,21 +78,23 @@ Alarm.prototype = {
 
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("startup", (type, extension) => {
   alarmsMap.set(extension, new Map());
   alarmCallbacksMap.set(extension, new Set());
 });
 
 extensions.on("shutdown", (type, extension) => {
-  for (let alarm of alarmsMap.get(extension).values()) {
-    alarm.clear();
+  if (alarmsMap.has(extension)) {
+    for (let alarm of alarmsMap.get(extension).values()) {
+      alarm.clear();
+    }
+    alarmsMap.delete(extension);
+    alarmCallbacksMap.delete(extension);
   }
-  alarmsMap.delete(extension);
-  alarmCallbacksMap.delete(extension);
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 extensions.registerSchemaAPI("alarms", (extension, context) => {
   return {
     alarms: {
       create: function(name, alarmInfo) {
         name = name || "";
--- a/toolkit/components/extensions/ext-notifications.js
+++ b/toolkit/components/extensions/ext-notifications.js
@@ -76,20 +76,22 @@ Notification.prototype = {
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("startup", (type, extension) => {
   let map = new Map();
   EventEmitter.decorate(map);
   notificationsMap.set(extension, map);
 });
 
 extensions.on("shutdown", (type, extension) => {
-  for (let notification of notificationsMap.get(extension).values()) {
-    notification.clear();
+  if (notificationsMap.has(extension)) {
+    for (let notification of notificationsMap.get(extension).values()) {
+      notification.clear();
+    }
+    notificationsMap.delete(extension);
   }
-  notificationsMap.delete(extension);
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 var nextId = 0;
 
 extensions.registerSchemaAPI("notifications", (extension, context) => {
   return {
     notifications: {
--- a/toolkit/components/extensions/test/mochitest/chrome.ini
+++ b/toolkit/components/extensions/test/mochitest/chrome.ini
@@ -17,17 +17,18 @@ skip-if = (toolkit == 'android') # andro
 [test_chrome_ext_eventpage_warning.html]
 [test_chrome_ext_native_messaging.html]
 skip-if = os == "android"  # native messaging is not supported on android
 [test_chrome_ext_contentscript_unrecognizedprop_warning.html]
 skip-if = (os == 'android') # browser.tabs is undefined. Bug 1258975 on android.
 [test_chrome_ext_trustworthy_origin.html]
 [test_chrome_ext_webnavigation_resolved_urls.html]
 skip-if = (os == 'android') # browser.tabs is undefined. Bug 1258975 on android.
+[test_chrome_ext_shutdown_cleanup.html]
 [test_chrome_native_messaging_paths.html]
 skip-if = os != "mac" && os != "linux"
 [test_ext_cookies_expiry.html]
 skip-if = buildapp == 'b2g'
 [test_ext_cookies_permissions.html]
 skip-if = buildapp == 'b2g'
 [test_ext_jsversion.html]
 skip-if = buildapp == 'b2g'
-[test_ext_schema.html]
+[test_ext_schema.html]
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_shutdown_cleanup.html
@@ -0,0 +1,59 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>WebExtension test</title>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" href="chrome://mochikit/contents/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://testing-common/TestUtils.jsm");
+
+const {
+ GlobalManager,
+ UninstallObserver,
+} = Cu.import("resource://gre/modules/Extension.jsm");
+
+/* eslint-disable mozilla/balanced-listeners */
+
+add_task(function* testShutdownCleanup() {
+  is(GlobalManager.initialized, false,
+     "GlobalManager start as not initialized");
+  is(UninstallObserver.initialized, false,
+     "UninstallObserver start as not initialized");
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background: "new " + function() {
+      browser.test.notifyPass("background page loaded");
+    },
+  });
+
+  yield extension.startup();
+
+  yield extension.awaitFinish("background page loaded");
+
+  is(GlobalManager.initialized, true,
+     "GlobalManager has been initialized once an extension is started");
+  is(UninstallObserver.initialized, true,
+     "UninstallObserver has been initialized once an extension is started");
+
+  yield extension.unload();
+
+  is(GlobalManager.initialized, false,
+     "GlobalManager has been uninitialized once all the webextensions have been stopped");
+  is(UninstallObserver.initialized, false,
+     "UninstallObserver has been uninitialized once all the webextensions have been stopped");
+});
+</script>
+
+</body>
+</html>