Bug 730340 - Don't use expando properties for storing data on places nodes. part 1 - use a WeakMap for mapping places nodes to DOM nodes. part 2 - use a WeakMap for cellProperties in treeView.js; stop caching _plainContainer. r=mak

This commit is contained in:
Asaf Romano 2012-06-01 23:03:00 +03:00
Родитель 1b458bce34
Коммит a41ee513d9
2 изменённых файлов: 93 добавлений и 120 удалений

Просмотреть файл

@ -68,18 +68,37 @@ PlacesViewBase.prototype = {
if (val) {
this._resultNode = val.root;
this._rootElt._placesNode = this._resultNode;
this._resultNode._DOMElement = this._rootElt;
this._domNodes = new WeakMap();
this._domNodes.set(this._resultNode, this._rootElt);
// This calls _rebuild through invalidateContainer.
this._resultNode.containerOpen = true;
}
else {
this._resultNode = null;
delete this._domNodes;
}
return val;
},
/**
* Gets the DOM node used for the given places node.
*
* @param aPlacesNode
* a places result node.
* @throws if there is no DOM node set for aPlacesNode.
*/
_getDOMNodeForPlacesNode:
function PVB__getDOMNodeForPlacesNode(aPlacesNode) {
let node = this._domNodes.get(aPlacesNode, null);
if (!node) {
throw new Error("No DOM node set for aPlacesNode.\nnode.type: " +
aPlacesNode.type + ". node.parent: " + aPlacesNode);
}
return node;
},
get controller() this._controller,
get selType() "single",
@ -254,7 +273,8 @@ PlacesViewBase.prototype = {
_createMenuItemForPlacesNode:
function PVB__createMenuItemForPlacesNode(aPlacesNode) {
delete aPlacesNode._DOMElement;
this._domNodes.delete(aPlacesNode);
let element;
let type = aPlacesNode.type;
if (type == Ci.nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR) {
@ -316,7 +336,7 @@ PlacesViewBase.prototype = {
element.appendChild(popup);
element.className = "menu-iconic bookmark-item";
aPlacesNode._DOMElement = popup;
this._domNodes.set(aPlacesNode, popup);
}
else
throw "Unexpected node";
@ -329,8 +349,8 @@ PlacesViewBase.prototype = {
}
element._placesNode = aPlacesNode;
if (!aPlacesNode._DOMElement)
aPlacesNode._DOMElement = element;
if (!this._domNodes.has(aPlacesNode))
this._domNodes.set(aPlacesNode, element);
return element;
},
@ -412,23 +432,20 @@ PlacesViewBase.prototype = {
}
},
toggleCutNode: function PVB_toggleCutNode(aNode, aValue) {
let elt = aNode._DOMElement;
if (elt) {
// We may get the popup for menus, but we need the menu itself.
if (elt.localName == "menupopup")
elt = elt.parentNode;
if (aValue)
elt.setAttribute("cutting", "true");
else
elt.removeAttribute("cutting");
}
toggleCutNode: function PVB_toggleCutNode(aPlacesNode, aValue) {
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// We may get the popup for menus, but we need the menu itself.
if (elt.localName == "menupopup")
elt = elt.parentNode;
if (aValue)
elt.setAttribute("cutting", "true");
else
elt.removeAttribute("cutting");
},
nodeURIChanged: function PVB_nodeURIChanged(aPlacesNode, aURIString) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -438,9 +455,7 @@ PlacesViewBase.prototype = {
},
nodeIconChanged: function PVB_nodeIconChanged(aPlacesNode) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// There's no UI representation for the root node, thus there's nothing to
// be done when the icon changes.
@ -460,9 +475,7 @@ PlacesViewBase.prototype = {
nodeAnnotationChanged:
function PVB_nodeAnnotationChanged(aPlacesNode, aAnno) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// All livemarks have a feedURI, so use it as our indicator of a livemark
// being modified.
@ -495,9 +508,7 @@ PlacesViewBase.prototype = {
nodeTitleChanged:
function PVB_nodeTitleChanged(aPlacesNode, aNewTitle) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// There's no UI representation for the root node, thus there's
// nothing to be done when the title changes.
@ -521,13 +532,9 @@ PlacesViewBase.prototype = {
nodeRemoved:
function PVB_nodeRemoved(aParentPlacesNode, aPlacesNode, aIndex) {
let parentElt = aParentPlacesNode._DOMElement;
let elt = aPlacesNode._DOMElement;
let parentElt = this._getDOMNodeForPlacesNode(aPlacesNode);
if (!parentElt)
throw "aParentPlacesNode must have _DOMElement set";
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -546,14 +553,9 @@ PlacesViewBase.prototype = {
nodeReplaced:
function PVB_nodeReplaced(aParentPlacesNode, aOldPlacesNode, aNewPlacesNode, aIndex) {
let parentElt = aParentPlacesNode._DOMElement;
if (!parentElt)
throw "aParentPlacesNode node must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
if (parentElt._built) {
let elt = aOldPlacesNode._DOMElement;
if (!elt)
throw "aOldPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aOldPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -573,7 +575,7 @@ PlacesViewBase.prototype = {
function PVB_nodeHistoryDetailsChanged(aPlacesNode, aTime, aCount) {
if (aPlacesNode.parent && aPlacesNode.parent._feedURI) {
// Find the node in the parent.
let popup = aPlacesNode.parent._DOMElement;
let popup = this._getDOMNodeForPlacesNode(aPlacesNode);
for (let child = popup._startMarker.nextSibling;
child != popup._endMarker;
child = child.nextSibling) {
@ -597,10 +599,7 @@ PlacesViewBase.prototype = {
nodeInserted:
function PVB_nodeInserted(aParentPlacesNode, aPlacesNode, aIndex) {
let parentElt = aParentPlacesNode._DOMElement;
if (!parentElt)
throw "aParentPlacesNode node must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
if (!parentElt._built)
return;
@ -619,9 +618,7 @@ PlacesViewBase.prototype = {
// use this notification when the item in question is moved from one
// folder to another. Instead, it calls nodeRemoved and nodeInserted
// for the two folders. Thus, we can assume old-parent == new-parent.
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -632,10 +629,7 @@ PlacesViewBase.prototype = {
if (elt == this._rootElt)
return;
let parentElt = aNewParentPlacesNode._DOMElement;
if (!parentElt)
throw "aNewParentPlacesNode node must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aNewParentPlacesNode);
if (parentElt._built) {
// Move the node.
parentElt.removeChild(elt);
@ -704,19 +698,16 @@ PlacesViewBase.prototype = {
let child = children[i];
this.nodeInserted(placesNode, child, i);
if (child.accessCount)
child._DOMElement.setAttribute("visited", true);
this._getDOMNodeForPlacesNode(child).setAttribute("visited", true);
else
child._DOMElement.removeAttribute("visited");
this._getDOMNodeForPlacesNode(child).removeAttribute("visited");
}
}).bind(this)
);
},
invalidateContainer: function PVB_invalidateContainer(aPlacesNode) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
elt._built = false;
// If the menupopup is open we should live-update it.
@ -981,7 +972,8 @@ PlacesToolbar.prototype = {
_insertNewItem:
function PT__insertNewItem(aChild, aBefore) {
delete aChild._DOMElement;
this._domNodes.delete(aChild);
let type = aChild.type;
let button;
if (type == Ci.nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR) {
@ -1027,7 +1019,7 @@ PlacesToolbar.prototype = {
popup.setAttribute("context", "placesContext");
#endif
aChild._DOMElement = popup;
this._domNodes.set(aChild, popup);
}
else if (PlacesUtils.nodeIsURI(aChild)) {
button.setAttribute("scheme",
@ -1036,8 +1028,8 @@ PlacesToolbar.prototype = {
}
button._placesNode = aChild;
if (!aChild._DOMElement)
aChild._DOMElement = button;
if (!this._domNodes.has(aChild))
this._domNodes.set(aChild, button);
if (aBefore)
this._rootElt.insertBefore(button, aBefore);
@ -1180,10 +1172,7 @@ PlacesToolbar.prototype = {
nodeInserted:
function PT_nodeInserted(aParentPlacesNode, aPlacesNode, aIndex) {
let parentElt = aParentPlacesNode._DOMElement;
if (!parentElt)
throw "aParentPlacesNode node must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
if (parentElt == this._rootElt) {
let children = this._rootElt.childNodes;
this._insertNewItem(aPlacesNode,
@ -1197,13 +1186,8 @@ PlacesToolbar.prototype = {
nodeRemoved:
function PT_nodeRemoved(aParentPlacesNode, aPlacesNode, aIndex) {
let parentElt = aParentPlacesNode._DOMElement;
let elt = aPlacesNode._DOMElement;
if (!parentElt)
throw "aParentPlacesNode node must have _DOMElement set";
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -1222,17 +1206,12 @@ PlacesToolbar.prototype = {
function PT_nodeMoved(aPlacesNode,
aOldParentPlacesNode, aOldIndex,
aNewParentPlacesNode, aNewIndex) {
let parentElt = aNewParentPlacesNode._DOMElement;
if (!parentElt)
throw "aNewParentPlacesNode node must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aNewParentPlacesNode);
if (parentElt == this._rootElt) {
// Container is on the toolbar.
// Move the element.
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -1257,10 +1236,7 @@ PlacesToolbar.prototype = {
nodeAnnotationChanged:
function PT_nodeAnnotationChanged(aPlacesNode, aAnno) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
if (elt == this._rootElt)
return;
@ -1296,9 +1272,7 @@ PlacesToolbar.prototype = {
},
nodeTitleChanged: function PT_nodeTitleChanged(aPlacesNode, aNewTitle) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// There's no UI representation for the root node, thus there's
// nothing to be done when the title changes.
@ -1320,14 +1294,9 @@ PlacesToolbar.prototype = {
nodeReplaced:
function PT_nodeReplaced(aParentPlacesNode,
aOldPlacesNode, aNewPlacesNode, aIndex) {
let parentElt = aParentPlacesNode._DOMElement;
if (!parentElt)
throw "aParentPlacesNode node must have _DOMElement set";
let parentElt = this._getDOMNodeForPlacesNode(aParentPlacesNode);
if (parentElt == this._rootElt) {
let elt = aOldPlacesNode._DOMElement;
if (!elt)
throw "aOldPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aOldPlacesNode);
// Here we need the <menu>.
if (elt.localName == "menupopup")
@ -1348,10 +1317,7 @@ PlacesToolbar.prototype = {
},
invalidateContainer: function PT_invalidateContainer(aPlacesNode) {
let elt = aPlacesNode._DOMElement;
if (!elt)
throw "aPlacesNode must have _DOMElement set";
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
if (elt == this._rootElt) {
// Container is the toolbar itself.
this._rebuild();

Просмотреть файл

@ -91,30 +91,26 @@ PlacesTreeView.prototype = {
* @return true if aContainer is a plain container, false otherwise.
*/
_isPlainContainer: function PTV__isPlainContainer(aContainer) {
if (aContainer._plainContainer !== undefined)
return aContainer._plainContainer;
// Livemarks are always plain containers.
if (aContainer._feedURI)
return aContainer._plainContainer = true;
return true;
// We don't know enough about non-query containers.
if (!(aContainer instanceof Ci.nsINavHistoryQueryResultNode))
return aContainer._plainContainer = false;
return false;
switch (aContainer.queryOptions.resultType) {
case Ci.nsINavHistoryQueryOptions.RESULTS_AS_DATE_QUERY:
case Ci.nsINavHistoryQueryOptions.RESULTS_AS_SITE_QUERY:
case Ci.nsINavHistoryQueryOptions.RESULTS_AS_DATE_SITE_QUERY:
case Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY:
return aContainer._plainContainer = false;
return false;
}
// If it's a folder, it's not a plain container.
let nodeType = aContainer.type;
return aContainer._plainContainer =
(nodeType != Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER &&
nodeType != Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT);
return nodeType != Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER &&
nodeType != Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT;
},
/**
@ -828,7 +824,7 @@ PlacesTreeView.prototype = {
for (let i = parentRow; i < this._rows.length; i++) {
let child = this.nodeForTreeIndex(i);
if (child.uri == aNode.uri) {
delete child._cellProperties;
this._cellProperties.delete(child);
this._invalidateCellValue(child, this.COLUMN_TYPE_TITLE);
break;
}
@ -858,9 +854,10 @@ PlacesTreeView.prototype = {
(function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
aNode._feedURI = aLivemark.feedURI;
if (aNode._cellProperties) {
aNode._cellProperties.push(this._getAtomFor("livemark"));
}
let properties = this._cellProperties.get(aNode, null);
if (properties)
properties.push(this._getAtomFor("livemark"));
// The livemark attribute is set as a cell property on the title cell.
this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE);
}
@ -1084,8 +1081,16 @@ PlacesTreeView.prototype = {
this._rootNode.containerOpen = false;
}
this._result = val;
this._rootNode = val ? val.root : null;
if (val) {
this._result = val;
this._rootNode = this._result.root;
this._cellProperties = new WeakMap();
}
else if (this._result) {
delete this._result;
delete this._rootNode;
delete this._cellProperties;
}
// If the tree is not set yet, setTree will call finishInit.
if (this._tree && val)
@ -1147,8 +1152,9 @@ PlacesTreeView.prototype = {
aProperties.AppendElement(this._getAtomFor("cutting"));
}
if (!node._cellProperties) {
let properties = new Array();
let properties = this._cellProperties.get(node, null);
if (!properties) {
properties = [];
let itemId = node.itemId;
let nodeType = node.type;
if (PlacesUtils.containerTypes.indexOf(nodeType) != -1) {
@ -1172,7 +1178,7 @@ PlacesTreeView.prototype = {
(function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
node._feedURI = aLivemark.feedURI;
node._cellProperties.push(this._getAtomFor("livemark"));
properties.push(this._getAtomFor("livemark"));
// The livemark attribute is set as a cell property on the title cell.
this._invalidateCellValue(node, this.COLUMN_TYPE_TITLE);
}
@ -1200,10 +1206,11 @@ PlacesTreeView.prototype = {
}
}
node._cellProperties = properties;
this._cellProperties.set(node, properties);
}
for (let property of properties) {
aProperties.AppendElement(property);
}
for (let i = 0; i < node._cellProperties.length; i++)
aProperties.AppendElement(node._cellProperties[i]);
},
getColumnProperties: function(aColumn, aProperties) { },