[Australis] Bug 987177 - invalidate wrapper's node reference, r=jaws, a=sylvestre
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 26 Mar 2014 00:28:58 +0000
changeset 192373 573846297dbae08634b89e7596cea0eb3b683681
parent 192372 33e7fd745c1b3db5f2d6e1c9f4cb8cd4abf1a134
child 192374 230efbf7f58d8f728797a95e1dd74be5a814b70e
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, sylvestre
bugs987177
milestone30.0a2
[Australis] Bug 987177 - invalidate wrapper's node reference, r=jaws, a=sylvestre
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_987177_xul_wrapper_updating.js
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -3425,17 +3425,17 @@ function XULWidgetGroupWrapper(aWidgetId
 
     let instance = aWindow.document.getElementById(aWidgetId);
     if (!instance) {
       // Toolbar palettes aren't part of the document, so elements in there
       // won't be found via document.getElementById().
       instance = aWindow.gNavToolbox.palette.getElementsByAttribute("id", aWidgetId)[0];
     }
 
-    let wrapper = new XULWidgetSingleWrapper(aWidgetId, instance);
+    let wrapper = new XULWidgetSingleWrapper(aWidgetId, instance, aWindow.document);
     wrapperMap.set(aWidgetId, wrapper);
     return wrapper;
   };
 
   this.__defineGetter__("areaType", function() {
     let placement = CustomizableUIInternal.getPlacementOfWidget(aWidgetId);
     if (!placement) {
       return null;
@@ -3451,42 +3451,80 @@ function XULWidgetGroupWrapper(aWidgetId
 
   Object.freeze(this);
 }
 
 /**
  * A XULWidgetSingleWrapper is a wrapper around a single instance of a XUL 
  * widget in a particular window.
  */
-function XULWidgetSingleWrapper(aWidgetId, aNode) {
+function XULWidgetSingleWrapper(aWidgetId, aNode, aDocument) {
   this.isGroup = false;
 
   this.id = aWidgetId;
   this.type = "custom";
   this.provider = CustomizableUI.PROVIDER_XUL;
 
-  this.node = aNode;
+  let weakDoc = Cu.getWeakReference(aDocument);
+  // If we keep a strong ref, the weak ref will never die, so null it out:
+  aDocument = null;
+
+  this.__defineGetter__("node", function() {
+    // If we've set this to null (further down), we're sure there's nothing to
+    // be gotten here, so bail out early:
+    if (!weakDoc) {
+      return null;
+    }
+    if (aNode) {
+      // Return the last known node if it's still in the DOM...
+      if (aNode.ownerDocument.contains(aNode)) {
+        return aNode;
+      }
+      // ... or the toolbox
+      let toolbox = aNode.ownerDocument.defaultView.gNavToolbox;
+      if (toolbox && toolbox.palette && aNode.parentNode == toolbox.palette) {
+        return aNode;
+      }
+      // If it isn't, clear the cached value and fall through to the "slow" case:
+      aNode = null;
+    }
+
+    let doc = weakDoc.get();
+    if (doc) {
+      // Store locally so we can cache the result:
+      aNode = CustomizableUIInternal.findWidgetInWindow(aWidgetId, doc.defaultView);
+      return aNode;
+    }
+    // The weakref to the document is dead, we're done here forever more:
+    weakDoc = null;
+    return null;
+  });
 
   this.__defineGetter__("anchor", function() {
     let anchorId;
     // First check for an anchor for the area:
     let placement = CustomizableUIInternal.getPlacementOfWidget(aWidgetId);
     if (placement) {
       anchorId = gAreas.get(placement.area).get("anchor");
     }
-    if (!anchorId) {
-      anchorId = aNode.getAttribute("cui-anchorid");
+
+    let node = this.node;
+    if (!anchorId && node) {
+      anchorId = node.getAttribute("cui-anchorid");
     }
 
-    return anchorId ? aNode.ownerDocument.getElementById(anchorId)
-                    : aNode;
+    return (anchorId && node) ? node.ownerDocument.getElementById(anchorId) : node;
   });
 
   this.__defineGetter__("overflowed", function() {
-    return aNode.getAttribute("overflowedItem") == "true";
+    let node = this.node;
+    if (!node) {
+      return false;
+    }
+    return node.getAttribute("overflowedItem") == "true";
   });
 
   Object.freeze(this);
 }
 
 const LAZY_RESIZE_INTERVAL_MS = 200;
 
 function OverflowableToolbar(aToolbarNode) {
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -78,9 +78,10 @@ skip-if = os == "linux"
 [browser_976792_insertNodeInWindow.js]
 skip-if = os == "linux"
 
 [browser_978084_dragEnd_after_move.js]
 [browser_980155_add_overflow_toolbar.js]
 [browser_981418-widget-onbeforecreated-handler.js]
 [browser_985815_propagate_setToolbarVisibility.js]
 [browser_981305_separator_insertion.js]
+[browser_987177_xul_wrapper_updating.js]
 [browser_panel_toggle.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_987177_xul_wrapper_updating.js
@@ -0,0 +1,74 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+const BUTTONID = "test-XUL-wrapper-widget";
+add_task(function() {
+  let btn = createDummyXULButton(BUTTONID, "XUL btn");
+  gNavToolbox.palette.appendChild(btn);
+  let groupWrapper = CustomizableUI.getWidget(BUTTONID);
+  ok(groupWrapper, "Should get a group wrapper");
+  let singleWrapper = groupWrapper.forWindow(window);
+  ok(singleWrapper, "Should get a single wrapper");
+  is(singleWrapper.node, btn, "Node should be in the wrapper");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, btn, "Button should be that instance.");
+
+  CustomizableUI.addWidgetToArea(BUTTONID, CustomizableUI.AREA_NAVBAR);
+
+  let otherSingleWrapper = groupWrapper.forWindow(window);
+  is(singleWrapper, otherSingleWrapper, "Should get the same wrapper after adding the node to the navbar.");
+  is(singleWrapper.node, btn, "Node should be in the wrapper");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, btn, "Button should be that instance.");
+
+  CustomizableUI.removeWidgetFromArea(BUTTONID);
+
+  otherSingleWrapper = groupWrapper.forWindow(window);
+  isnot(singleWrapper, otherSingleWrapper, "Shouldn't get the same wrapper after removing it from the navbar.");
+  singleWrapper = otherSingleWrapper;
+  is(singleWrapper.node, btn, "Node should be in the wrapper");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, btn, "Button should be that instance.");
+
+  btn.remove();
+  otherSingleWrapper = groupWrapper.forWindow(window);
+  is(singleWrapper, otherSingleWrapper, "Should get the same wrapper after physically removing the node.");
+  is(singleWrapper.node, null, "Wrapper's node should be null now that it's left the DOM.");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, null, "That instance should be null.");
+
+  btn = createDummyXULButton(BUTTONID, "XUL btn");
+  gNavToolbox.palette.appendChild(btn);
+  otherSingleWrapper = groupWrapper.forWindow(window);
+  is(singleWrapper, otherSingleWrapper, "Should get the same wrapper after readding the node.");
+  is(singleWrapper.node, btn, "Node should be in the wrapper");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, btn, "Button should be that instance.");
+
+  CustomizableUI.addWidgetToArea(BUTTONID, CustomizableUI.AREA_NAVBAR);
+
+  otherSingleWrapper = groupWrapper.forWindow(window);
+  is(singleWrapper, otherSingleWrapper, "Should get the same wrapper after adding the node to the navbar.");
+  is(singleWrapper.node, btn, "Node should be in the wrapper");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, btn, "Button should be that instance.");
+
+  CustomizableUI.removeWidgetFromArea(BUTTONID);
+
+  otherSingleWrapper = groupWrapper.forWindow(window);
+  isnot(singleWrapper, otherSingleWrapper, "Shouldn't get the same wrapper after removing it from the navbar.");
+  singleWrapper = otherSingleWrapper;
+  is(singleWrapper.node, btn, "Node should be in the wrapper");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, btn, "Button should be that instance.");
+
+  btn.remove();
+  otherSingleWrapper = groupWrapper.forWindow(window);
+  is(singleWrapper, otherSingleWrapper, "Should get the same wrapper after physically removing the node.");
+  is(singleWrapper.node, null, "Wrapper's node should be null now that it's left the DOM.");
+  is(groupWrapper.instances.length, 1, "There should be 1 instance on the group wrapper");
+  is(groupWrapper.instances[0].node, null, "That instance should be null.");
+});