Bug 1206133 - Fix for Jetpack bugs caused by the popuppositioned patch. r=gabor
authorKirk Steuber <ksteuber@mozilla.com>
Fri, 10 Jun 2016 10:59:38 +0200
changeset 315937 9401602eabf337e087f9723eba1d5d1869d7ba6c
parent 315936 bca4534616adca94e6e9b5dd85f18919768e174a
child 315938 06f3f82059c82c6958a27f5c478325f22fdd4a7f
push id20634
push usercbook@mozilla.com
push dateFri, 30 Sep 2016 10:10:13 +0000
treeherderfx-team@afe79b010d13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs1206133
milestone52.0a1
Bug 1206133 - Fix for Jetpack bugs caused by the popuppositioned patch. r=gabor MozReview-Commit-ID: 7SQIMcaNoAl
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();
 };