Backed out changeset 2b0a8096c6e0 (bug 1206133)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 10 Sep 2016 23:40:32 +0200
changeset 354750 d2deda115569aa52d6fe70042ced70fee27cdbb4
parent 354749 8d37bc3c376e4f05944ae92f1cd0ab886d0b9895
child 354751 43725e6c61b396f0ae965f060f6ccc02a1d6a382
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1206133
milestone51.0a1
backs out2b0a8096c6e0986af6aba3f7f13be49c8e1b7d6e
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
Backed out changeset 2b0a8096c6e0 (bug 1206133)
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 { Cu, Ci } = require("chrome");
+const { 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,83 +106,42 @@ 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);
 
-function getPanelFromWeakRef(weakRef) {
-  if (!weakRef) {
-    return null;
-  }
-  let panel = weakRef.get();
-  if (!panel) {
-    return null;
-  }
-  if (isDisposed(panel)) {
-    return null;
-  }
-  return 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();
 
-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);
+  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();
     }
-  },
-  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);
-    }
+
+    // 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);
   }
-};
+}
 
 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,
@@ -214,16 +173,18 @@ 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);
   },
@@ -296,47 +257,45 @@ 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) {
-    SinglePanelManager.requestOpen(this, () => {
-      if (options instanceof Ci.nsIDOMElement) {
-        [anchor, options] = [options, null];
-      }
+    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,21 +292,18 @@ exports["test Hide Before Show"] = funct
       hideCalled = true;
     }
   });
   panel1.show();
   panel1.hide();
 
   let panel2 = Panel({
     onShow: function () {
-      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');
-      }
+      assert.ok(!showCalled, 'should not emit show');
+      assert.ok(!hideCalled, 'should not emit hide');
       panel1.destroy();
       panel2.destroy();
       done();
     },
   });
   panel2.show();
 };