Bug 1206133 - Fix for Jetpack bugs caused by the popuppositioned patch. r=gabor
☠☠ backed out by d2deda115569 ☠ ☠
authorKirk Steuber <ksteuber@mozilla.com>
Fri, 10 Jun 2016 10:59:38 +0200
changeset 313460 2b0a8096c6e0986af6aba3f7f13be49c8e1b7d6e
parent 313459 0c9c5b508ccd245bdd091f311a62bffffcef3898
child 313461 b99b5f17840568617f01a3e78d8f078283c9e371
push id30685
push userphilringnalda@gmail.com
push dateSun, 11 Sep 2016 05:38:20 +0000
treeherdermozilla-central@7e873393cc11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs1206133
milestone51.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 1206133 - Fix for Jetpack bugs caused by the popuppositioned patch. r=gabor MozReview-Commit-ID: 8vesVdT8sfh
addon-sdk/source/lib/sdk/panel.js
addon-sdk/source/test/test-panel.js
--- a/addon-sdk/source/lib/sdk/panel.js
+++ b/addon-sdk/source/lib/sdk/panel.js
@@ -8,17 +8,17 @@
 module.metadata = {
   "stability": "stable",
   "engines": {
     "Firefox": "*",
     "SeaMonkey": "*"
   }
 };
 
-const { Ci } = require("chrome");
+const { Cu, Ci } = require("chrome");
 const { setTimeout } = require('./timers');
 const { Class } = require("./core/heritage");
 const { merge } = require("./util/object");
 const { WorkerHost } = require("./content/utils");
 const { Worker } = require("./deprecated/sync-worker");
 const { Disposable } = require("./core/disposable");
 const { WeakReference } = require('./core/reference');
 const { contract: loaderContract } = require("./content/loader");
@@ -106,42 +106,83 @@ var workers = new WeakMap();
 var styles = new WeakMap();
 
 const viewFor = (panel) => views.get(panel);
 const modelFor = (panel) => models.get(panel);
 const panelFor = (view) => panels.get(view);
 const workerFor = (panel) => workers.get(panel);
 const styleFor = (panel) => styles.get(panel);
 
-// Utility function takes `panel` instance and makes sure it will be
-// automatically hidden as soon as other panel is shown.
-var setupAutoHide = new function() {
-  let refs = new WeakMap();
+function getPanelFromWeakRef(weakRef) {
+  if (!weakRef) {
+    return null;
+  }
+  let panel = weakRef.get();
+  if (!panel) {
+    return null;
+  }
+  if (isDisposed(panel)) {
+    return null;
+  }
+  return panel;
+}
 
-  return function setupAutoHide(panel) {
-    // Create system event listener that reacts to any panel showing and
-    // hides given `panel` if it's not the one being shown.
-    function listener({subject}) {
-      // It could be that listener is not GC-ed in the same cycle as
-      // panel in such case we remove listener manually.
-      let view = viewFor(panel);
-      if (!view) systemEvents.off("popupshowing", listener);
-      else if (subject !== view) panel.hide();
+var SinglePanelManager = {
+  visiblePanel: null,
+  enqueuedPanel: null,
+  enqueuedPanelCallback: null,
+  // Calls |callback| with no arguments when the panel may be shown.
+  requestOpen: function(panelToOpen, callback) {
+    let currentPanel = getPanelFromWeakRef(SinglePanelManager.visiblePanel);
+    if (currentPanel || SinglePanelManager.enqueuedPanel) {
+      SinglePanelManager.enqueuedPanel = Cu.getWeakReference(panelToOpen);
+      SinglePanelManager.enqueuedPanelCallback = callback;
+      if (currentPanel && currentPanel.isShowing) {
+        currentPanel.hide();
+      }
+    } else {
+      SinglePanelManager.notifyPanelCanOpen(panelToOpen, callback);
     }
-
-    // system event listener is intentionally weak this way we'll allow GC
-    // to claim panel if it's no longer referenced by an add-on code. This also
-    // helps minimizing cleanup required on unload.
-    systemEvents.on("popupshowing", listener);
-    // To make sure listener is not claimed by GC earlier than necessary we
-    // associate it with `panel` it's associated with. This way it won't be
-    // GC-ed earlier than `panel` itself.
-    refs.set(panel, listener);
+  },
+  notifyPanelCanOpen: function(panel, callback) {
+    let view = viewFor(panel);
+    // Can't pass an arrow function as the event handler because we need to be
+    // able to call |removeEventListener| later.
+    view.addEventListener("popuphidden", SinglePanelManager.onVisiblePanelHidden, true);
+    view.addEventListener("popupshown", SinglePanelManager.onVisiblePanelShown, false);
+    SinglePanelManager.enqueuedPanel = null;
+    SinglePanelManager.enqueuedPanelCallback = null;
+    SinglePanelManager.visiblePanel = Cu.getWeakReference(panel);
+    callback();
+  },
+  onVisiblePanelShown: function(event) {
+    let panel = panelFor(event.target);
+    if (SinglePanelManager.enqueuedPanel) {
+      // Another panel started waiting for |panel| to close before |panel| was
+      // even done opening.
+      panel.hide();
+    }
+  },
+  onVisiblePanelHidden: function(event) {
+    let view = event.target;
+    let panel = panelFor(view);
+    let currentPanel = getPanelFromWeakRef(SinglePanelManager.visiblePanel);
+    if (currentPanel && currentPanel != panel) {
+      return;
+    }
+    SinglePanelManager.visiblePanel = null;
+    view.removeEventListener("popuphidden", SinglePanelManager.onVisiblePanelHidden, true);
+    view.removeEventListener("popupshown", SinglePanelManager.onVisiblePanelShown, false);
+    let nextPanel = getPanelFromWeakRef(SinglePanelManager.enqueuedPanel);
+    let nextPanelCallback = SinglePanelManager.enqueuedPanelCallback;
+    if (nextPanel) {
+      SinglePanelManager.notifyPanelCanOpen(nextPanel, nextPanelCallback);
+    }
   }
-}
+};
 
 const Panel = Class({
   implements: [
     // Generate accessors for the validated properties that update model on
     // set and return values from model on get.
     panelContract.properties(modelFor),
     EventTarget,
     Disposable,
@@ -173,18 +214,16 @@ const Panel = Class({
     views.set(this, view);
 
     // Load panel content.
     domPanel.setURL(view, model.contentURL);
 
     // Allow context menu
     domPanel.allowContextMenu(view, model.contextMenu);
 
-    setupAutoHide(this);
-
     // Setup listeners.
     setListeners(this, options);
     let worker = new Worker(stripListeners(options));
     workers.set(this, worker);
 
     // pipe events from worker to a panel.
     pipe(worker, this);
   },
@@ -257,45 +296,47 @@ const Panel = Class({
 
   /* Public API: Panel.isShowing */
   get isShowing() {
     return !isDisposed(this) && domPanel.isOpen(viewFor(this));
   },
 
   /* Public API: Panel.show */
   show: function show(options={}, anchor) {
-    if (options instanceof Ci.nsIDOMElement) {
-      [anchor, options] = [options, null];
-    }
+    SinglePanelManager.requestOpen(this, () => {
+      if (options instanceof Ci.nsIDOMElement) {
+        [anchor, options] = [options, null];
+      }
 
-    if (anchor instanceof Ci.nsIDOMElement) {
-      console.warn(
-        "Passing a DOM node to Panel.show() method is an unsupported " +
-        "feature that will be soon replaced. " +
-        "See: https://bugzilla.mozilla.org/show_bug.cgi?id=878877"
-      );
-    }
+      if (anchor instanceof Ci.nsIDOMElement) {
+        console.warn(
+          "Passing a DOM node to Panel.show() method is an unsupported " +
+          "feature that will be soon replaced. " +
+          "See: https://bugzilla.mozilla.org/show_bug.cgi?id=878877"
+        );
+      }
 
-    let model = modelFor(this);
-    let view = viewFor(this);
-    let anchorView = getNodeView(anchor || options.position || model.position);
+      let model = modelFor(this);
+      let view = viewFor(this);
+      let anchorView = getNodeView(anchor || options.position || model.position);
 
-    options = merge({
-      position: model.position,
-      width: model.width,
-      height: model.height,
-      defaultWidth: model.defaultWidth,
-      defaultHeight: model.defaultHeight,
-      focus: model.focus,
-      contextMenu: model.contextMenu
-    }, displayContract(options));
+      options = merge({
+        position: model.position,
+        width: model.width,
+        height: model.height,
+        defaultWidth: model.defaultWidth,
+        defaultHeight: model.defaultHeight,
+        focus: model.focus,
+        contextMenu: model.contextMenu
+      }, displayContract(options));
 
-    if (!isDisposed(this))
-      domPanel.show(view, options, anchorView);
-
+      if (!isDisposed(this)) {
+        domPanel.show(view, options, anchorView);
+      }
+    });
     return this;
   },
 
   /* Public API: Panel.hide */
   hide: function hide() {
     // Quit immediately if panel is disposed or there is no state change.
     domPanel.close(viewFor(this));
 
--- a/addon-sdk/source/test/test-panel.js
+++ b/addon-sdk/source/test/test-panel.js
@@ -292,18 +292,21 @@ exports["test Hide Before Show"] = funct
       hideCalled = true;
     }
   });
   panel1.show();
   panel1.hide();
 
   let panel2 = Panel({
     onShow: function () {
-      assert.ok(!showCalled, 'should not emit show');
-      assert.ok(!hideCalled, 'should not emit hide');
+      if (showCalled) {
+        assert.ok(hideCalled, 'should not emit show without also emitting hide');
+      } else {
+        assert.ok(!hideCalled, 'should not emit hide without also emitting show');
+      }
       panel1.destroy();
       panel2.destroy();
       done();
     },
   });
   panel2.show();
 };