Bug 1413692 - Extensions page actions are pinned to address bar again and again. r?mixedpuppy draft
authorDrew Willcoxon <adw@mozilla.com>
Mon, 06 Nov 2017 21:19:26 -0500
changeset 693904 c0fda14d43f7192374cddd09b0392607d2d0fcb0
parent 690034 cd7217cf05a2332a8fd7b498767a07b2c31ea657
child 739188 cd76a952270207c0684746736cd7210d64b10634
push id87969
push userdwillcoxon@mozilla.com
push dateTue, 07 Nov 2017 02:19:51 +0000
reviewersmixedpuppy
bugs1413692
milestone58.0a1
Bug 1413692 - Extensions page actions are pinned to address bar again and again. r?mixedpuppy MozReview-Commit-ID: HrhYTNSwygH
browser/components/extensions/ext-pageAction.js
browser/components/extensions/test/xpcshell/test_ext_pageAction_shutdown.js
browser/components/extensions/test/xpcshell/xpcshell.ini
browser/modules/PageActions.jsm
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -86,17 +86,21 @@ this.pageAction = class extends Extensio
     }
   }
 
   onShutdown(reason) {
     pageActionMap.delete(this.extension);
 
     this.tabContext.shutdown();
 
-    if (this.browserPageAction) {
+    // Removing the browser page action causes PageActions to forget about it
+    // across app restarts, so don't remove it on app shutdown, but do remove
+    // it on all other shutdowns since there's no guarantee the action will be
+    // coming back.
+    if (reason != "APP_SHUTDOWN" && this.browserPageAction) {
       this.browserPageAction.remove();
       this.browserPageAction = null;
     }
   }
 
   // Returns the value of the property |prop| for the given tab, where
   // |prop| is one of "show", "title", "icon", "popup".
   getProperty(tab, prop) {
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/xpcshell/test_ext_pageAction_shutdown.js
@@ -0,0 +1,79 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+Cu.import("resource:///modules/PageActions.jsm");
+Cu.import("resource://testing-common/AddonTestUtils.jsm");
+
+const {
+  createAppInfo,
+  promiseShutdownManager,
+  promiseStartupManager,
+} = AddonTestUtils;
+
+AddonTestUtils.init(this);
+AddonTestUtils.overrideCertDB();
+
+createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "58");
+
+// This is copied and pasted from ExtensionPopups.jsm.  It's used as the
+// PageActions action ID.  See ext-pageAction.js.
+function makeWidgetId(id) {
+  id = id.toLowerCase();
+  // FIXME: This allows for collisions.
+  return id.replace(/[^a-z0-9_-]/g, "_");
+}
+
+// Tests that the shownInUrlbar property of the PageActions.Action object
+// backing the extension's page action persists across app restarts.
+add_task(async function testAppShutdown() {
+  let extensionData = {
+    useAddonManager: "permanent",
+    manifest: {
+      page_action: {
+        default_title: "test_ext_pageAction_shutdown.js",
+        browser_style: false,
+      },
+    },
+  };
+
+  // Simulate starting up the app.
+  PageActions.init();
+  await promiseStartupManager();
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+
+  // Get the PageAction.Action object.  Its shownInUrlbar should have been
+  // initialized to true in ext-pageAction.js, when it's created.
+  let actionID = makeWidgetId(extension.id);
+  let action = PageActions.actionForID(actionID);
+  Assert.equal(action.shownInUrlbar, true);
+
+  // Simulate restarting the app without first unloading the extension.
+  await promiseShutdownManager();
+  PageActions._reset();
+  await promiseStartupManager();
+  await extension.awaitStartup();
+
+  // Get the action.  Its shownInUrlbar should remain true.
+  action = PageActions.actionForID(actionID);
+  Assert.equal(action.shownInUrlbar, true);
+
+  // Now set its shownInUrlbar to false.
+  action.shownInUrlbar = false;
+
+  // Simulate restarting the app again without first unloading the extension.
+  await promiseShutdownManager();
+  PageActions._reset();
+  await promiseStartupManager();
+  await extension.awaitStartup();
+
+  // Get the action.  Its shownInUrlbar should remain false.
+  action = PageActions.actionForID(actionID);
+  Assert.equal(action.shownInUrlbar, false);
+
+  // Now unload the extension and quit the app.
+  await extension.unload();
+  await promiseShutdownManager();
+});
--- a/browser/components/extensions/test/xpcshell/xpcshell.ini
+++ b/browser/components/extensions/test/xpcshell/xpcshell.ini
@@ -11,15 +11,16 @@ dupe-manifest =
 # Tests which are affected by remote content or remote extensions should
 # go in one of:
 #
 #  - xpcshell-common.ini
 #    For tests which should run in all configurations.
 #  - xpcshell-remote.ini
 #    For tests which should only run with both remote extensions and remote content.
 
+[test_ext_geckoProfiler_schema.js]
 [test_ext_manifest_commands.js]
 [test_ext_manifest_omnibox.js]
 [test_ext_manifest_permissions.js]
-[test_ext_geckoProfiler_schema.js]
+[test_ext_pageAction_shutdown.js]
 [test_ext_pkcs11_management.js]
 
 [include:xpcshell-common.ini]
--- a/browser/modules/PageActions.jsm
+++ b/browser/modules/PageActions.jsm
@@ -386,16 +386,24 @@ this.PageActions = {
       } else {
         histogram.add("other");
       }
     } catch (ex) {
       Cu.reportError(ex);
     }
   },
 
+  // For tests.  See Bug 1413692.
+  _reset() {
+    PageActions._purgeUnregisteredPersistedActions();
+    PageActions._builtInActions = [];
+    PageActions._nonBuiltInActions = [];
+    PageActions._actionsByID = new Map();
+  },
+
   _storePersistedActions() {
     let json = JSON.stringify(this._persistedActions);
     Services.prefs.setStringPref(PREF_PERSISTED_ACTIONS, json);
   },
 
   _loadPersistedActions() {
     try {
       let json = Services.prefs.getStringPref(PREF_PERSISTED_ACTIONS);