Bug 730340 - Don't use expando properties for storing data on places nodes. Part 3 - cache livemark-info object in the controller for each view instead of setting _feedURI and _siteURI exapndos on livemark nodes. Also fix a bug I introduced in the previous check for this bug, in PVB_nodeHistoryDetailsChanged. r=mak

This commit is contained in:
Asaf Romano 2012-06-02 16:54:25 +03:00
parent ac0bc073d7
commit 8f87a1a12b
4 changed files with 100 additions and 63 deletions

View File

@ -213,7 +213,7 @@ PlacesViewBase.prototype = {
if (!resultNode.containerOpen)
return;
if (resultNode._feedURI) {
if (this.controller.hasCachedLivemarkInfo(resultNode)) {
this._setEmptyPopupStatus(aPopup, false);
aPopup._built = true;
this._populateLivemarkPopup(aPopup);
@ -315,10 +315,9 @@ PlacesViewBase.prototype = {
#endif
// Set an expando on the node, controller will use it to build
// its metadata.
aPlacesNode._feedURI = aLivemark.feedURI;
aPlacesNode._siteURI = aLivemark.siteURI;
this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark);
}
}
}.bind(this)
);
}
@ -365,8 +364,9 @@ PlacesViewBase.prototype = {
_setLivemarkSiteURIMenuItem:
function PVB__setLivemarkSiteURIMenuItem(aPopup) {
let siteUrl = aPopup._placesNode._siteURI ? aPopup._placesNode._siteURI.spec
: null;
let livemarkInfo = this.controller.getCachedLivemarkInfo(aPopup._placesNode);
let siteUrl = livemarkInfo && livemarkInfo.siteURI ?
livemarkInfo.siteURI.spec : null;
if (!siteUrl && aPopup._siteURIMenuitem) {
aPopup.removeChild(aPopup._siteURIMenuitem);
aPopup._siteURIMenuitem = null;
@ -493,15 +493,13 @@ PlacesViewBase.prototype = {
PlacesUtils.livemarks.getLivemark(
{ id: aPlacesNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
// Set an expando on the node, controller will use it to build
// its metadata.
aPlacesNode._feedURI = aLivemark.feedURI;
aPlacesNode._siteURI = aLivemark.siteURI;
// Controller will use this to build the meta data for the node.
this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark);
this.invalidateContainer(aPlacesNode);
}
}).bind(this)
}.bind(this)
);
}
},
@ -533,7 +531,6 @@ PlacesViewBase.prototype = {
nodeRemoved:
function PVB_nodeRemoved(aParentPlacesNode, aPlacesNode, aIndex) {
let parentElt = this._getDOMNodeForPlacesNode(aPlacesNode);
let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
// Here we need the <menu>.
@ -573,9 +570,10 @@ PlacesViewBase.prototype = {
nodeHistoryDetailsChanged:
function PVB_nodeHistoryDetailsChanged(aPlacesNode, aTime, aCount) {
if (aPlacesNode.parent && aPlacesNode.parent._feedURI) {
if (aPlacesNode.parent &&
this.controller.hasCachedLivemarkInfo(aPlacesNode.parent)) {
// Find the node in the parent.
let popup = this._getDOMNodeForPlacesNode(aPlacesNode);
let popup = this._getDOMNodeForPlacesNode(aPlacesNode.parent);
for (let child = popup._startMarker.nextSibling;
child != popup._endMarker;
child = child.nextSibling) {
@ -652,11 +650,11 @@ PlacesViewBase.prototype = {
}
PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
let shouldInvalidate = !aPlacesNode._feedURI;
aPlacesNode._feedURI = aLivemark.feedURI;
aPlacesNode._siteURI = aLivemark.siteURI;
let shouldInvalidate =
!this.controller.hasCachedLivemarkInfo(aPlacesNode);
this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark);
if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED) {
aLivemark.registerForUpdates(aPlacesNode, this);
// Prioritize the current livemark.
@ -669,7 +667,7 @@ PlacesViewBase.prototype = {
aLivemark.unregisterForUpdates(aPlacesNode);
}
}
}).bind(this)
}.bind(this)
);
}
}
@ -683,7 +681,7 @@ PlacesViewBase.prototype = {
this._setLivemarkStatusMenuItem(aPopup, Ci.mozILivemark.STATUS_LOADING);
PlacesUtils.livemarks.getLivemark({ id: aPopup._placesNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
let placesNode = aPopup._placesNode;
if (!Components.isSuccessCode(aStatus) || !placesNode.containerOpen)
return;
@ -702,7 +700,7 @@ PlacesViewBase.prototype = {
else
this._getDOMNodeForPlacesNode(child).removeAttribute("visited");
}
}).bind(this)
}.bind(this)
);
},
@ -1004,10 +1002,9 @@ PlacesToolbar.prototype = {
button.setAttribute("livemark", "true");
// Set an expando on the node, controller will use it to build
// its metadata.
aChild._feedURI = aLivemark.feedURI;
aChild._siteURI = aLivemark.siteURI;
this.controller.cacheLivemarkInfo(aChild, aLivemark);
}
}
}.bind(this)
);
}
@ -1253,15 +1250,14 @@ PlacesToolbar.prototype = {
PlacesUtils.livemarks.getLivemark(
{ id: aPlacesNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
// Set an expando on the node, controller will use it to build
// its metadata.
aPlacesNode._feedURI = aLivemark.feedURI;
aPlacesNode._siteURI = aLivemark.siteURI;
this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark);
this.invalidateContainer(aPlacesNode);
}
}).bind(this)
}.bind(this)
);
}
}
@ -1681,7 +1677,8 @@ PlacesToolbar.prototype = {
// so we don't rebuild its contents whenever the popup is reopened.
// Though, we want to always close feed containers so their expiration
// status will be checked at next opening.
if (!PlacesUtils.nodeIsFolder(placesNode) || placesNode._feedURI) {
if (!PlacesUtils.nodeIsFolder(placesNode) ||
this.controller.hasCachedLivemarkInfo(placesNode)) {
placesNode.containerOpen = false;
}
}
@ -1787,7 +1784,8 @@ PlacesMenu.prototype = {
// so we don't rebuild its contents whenever the popup is reopened.
// Though, we want to always close feed containers so their expiration
// status will be checked at next opening.
if (!PlacesUtils.nodeIsFolder(placesNode) || placesNode._feedURI)
if (!PlacesUtils.nodeIsFolder(placesNode) ||
this.controller.hasCachedLivemarkInfo(placesNode))
placesNode.containerOpen = false;
// The autoopened attribute is set for folders which have been

View File

@ -79,6 +79,8 @@ function PlacesController(aView) {
XPCOMUtils.defineLazyGetter(this, "profileName", function () {
return Services.dirsvc.get("ProfD", Ci.nsIFile).leafName;
});
this._cachedLivemarkInfoObjects = new WeakMap();
}
PlacesController.prototype = {
@ -176,7 +178,7 @@ PlacesController.prototype = {
case "placesCmd_reload":
// Livemark containers
var selectedNode = this._view.selectedNode;
return selectedNode && !!selectedNode._feedURI;
return selectedNode && this.hasCachedLivemarkInfo(selectedNode);
case "placesCmd_sortBy:name":
var selectedNode = this._view.selectedNode;
return selectedNode &&
@ -469,7 +471,7 @@ PlacesController.prototype = {
if (parentNode) {
if (PlacesUtils.nodeIsTagQuery(parentNode))
nodeData["tagChild"] = true;
else if (parentNode._feedURI)
else if (this.hasCachedLivemarkInfo(parentNode))
nodeData["livemarkChild"] = true;
}
}
@ -1038,8 +1040,9 @@ PlacesController.prototype = {
addData(PlacesUtils.TYPE_X_MOZ_PLACE, i);
// Drop the feed uri for livemark containers
if (node._feedURI) {
addURIData(i, node._feedURI.spec);
let livemarkInfo = this.getCachedLivemarkInfo(node);
if (livemarkInfo) {
addURIData(i, livemarkInfo.feedURI.spec);
}
else if (node.uri) {
addURIData(i);
@ -1113,7 +1116,8 @@ PlacesController.prototype = {
if (PlacesUtils.nodeIsFolder(node))
copiedFolders.push(node);
let overrideURI = node._feedURI ? node._feedURI.spec : null;
let livemarkInfo = this.getCachedLivemarkInfo(node);
let overrideURI = livemarkInfo ? livemarkInfo.feedURI.spec : null;
let resolveShortcuts = !PlacesControllerDragHelper.canMoveNode(node);
contents.forEach(function (content) {
@ -1281,7 +1285,39 @@ PlacesController.prototype = {
}
if (insertedNodeIds.length > 0)
this._view.selectItems(insertedNodeIds, false);
}
},
/**
* Cache the livemark info for a node. This allows the controller and the
* views to treat the given node as a livemark.
* @param aNode
* a places result node.
* @param aLivemarkInfo
* a mozILivemarkInfo object.
*/
cacheLivemarkInfo: function PC_cacheLivemarkInfo(aNode, aLivemarkInfo) {
this._cachedLivemarkInfoObjects.set(aNode, aLivemarkInfo);
},
/**
* Returns whether or not there's cached mozILivemarkInfo object for a node.
* @param aNode
* a places result node.
* @return true if there's a cached mozILivemarkInfo object for
* aNode, false otherwise.
*/
hasCachedLivemarkInfo: function PC_hasCachedLivemarkInfo(aNode)
this._cachedLivemarkInfoObjects.has(aNode),
/**
* Returns the cached livemark info for a node, if set by cacheLivemarkInfo,
* null otherwise.
* @param aNode
* a places result node.
* @return the mozILivemarkInfo object for aNode, if set, null otherwise.
*/
getCachedLivemarkInfo: function PC_getCachedLivemarkInfo(aNode)
this._cachedLivemarkInfoObjects.get(aNode, null)
};
/**

View File

@ -98,17 +98,18 @@
callback = new Function("aContainer", onOpenFlatContainer);
}
let treeView = new PlacesTreeView(this.flatList, callback);
if (!this._controller) {
this._controller = new PlacesController(this);
this.controllers.appendController(this._controller);
}
let treeView = new PlacesTreeView(this.flatList, callback, this._controller);
// Observer removal is done within the view itself. When the tree
// goes away, treeboxobject calls view.setTree(null), which then
// calls removeObserver.
result.addObserver(treeView, false);
this.view = treeView;
if (!this._controller) {
this._controller = new PlacesController(this);
this.controllers.appendController(this._controller);
}
this._cachedInsertionPoint = undefined;
]]></body>
</method>

View File

@ -2,7 +2,7 @@
* 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/. */
function PlacesTreeView(aFlatList, aOnOpenFlatContainer) {
function PlacesTreeView(aFlatList, aOnOpenFlatContainer, aController) {
this._tree = null;
this._result = null;
this._selection = null;
@ -10,6 +10,7 @@ function PlacesTreeView(aFlatList, aOnOpenFlatContainer) {
this._rows = [];
this._flatList = aFlatList;
this._openContainerCallback = aOnOpenFlatContainer;
this._controller = aController;
}
PlacesTreeView.prototype = {
@ -92,7 +93,7 @@ PlacesTreeView.prototype = {
*/
_isPlainContainer: function PTV__isPlainContainer(aContainer) {
// Livemarks are always plain containers.
if (aContainer._feedURI)
if (this._controller.hasCachedLivemarkInfo(aContainer))
return true;
// We don't know enough about non-query containers.
@ -299,7 +300,7 @@ PlacesTreeView.prototype = {
// Recursively do containers.
if (!this._flatList &&
curChild instanceof Ci.nsINavHistoryContainerResultNode &&
!curChild._feedURI) {
!this._controller.hasCachedLivemarkInfo(curChild)) {
let resource = this._getResourceForNode(curChild);
let isopen = resource != null &&
PlacesUIUtils.localStore.HasAssertion(resource,
@ -789,7 +790,7 @@ PlacesTreeView.prototype = {
_populateLivemarkContainer: function PTV__populateLivemarkContainer(aNode) {
PlacesUtils.livemarks.getLivemark({ id: aNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
let placesNode = aNode;
// Need to check containerOpen since getLivemark is async.
if (!Components.isSuccessCode(aStatus) || !placesNode.containerOpen)
@ -800,7 +801,7 @@ PlacesTreeView.prototype = {
let child = children[i];
this.nodeInserted(placesNode, child, i);
}
}).bind(this));
}.bind(this));
},
nodeTitleChanged: function PTV_nodeTitleChanged(aNode, aNewTitle) {
@ -818,7 +819,7 @@ PlacesTreeView.prototype = {
nodeHistoryDetailsChanged:
function PTV_nodeHistoryDetailsChanged(aNode, aUpdatedVisitDate,
aUpdatedVisitCount) {
if (aNode.parent && aNode.parent._feedURI) {
if (aNode.parent && this._controller.hasCachedLivemarkInfo(aNode.parent)) {
// Find the node in the parent.
let parentRow = this._flatList ? 0 : this._getRowForNode(aNode.parent);
for (let i = parentRow; i < this._rows.length; i++) {
@ -851,9 +852,9 @@ PlacesTreeView.prototype = {
else if (aAnno == PlacesUtils.LMANNO_FEEDURI) {
PlacesUtils.livemarks.getLivemark(
{ id: aNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
aNode._feedURI = aLivemark.feedURI;
this._controller.cacheLivemarkInfo(aNode, aLivemark);
let properties = this._cellProperties.get(aNode, null);
if (properties)
properties.push(this._getAtomFor("livemark"));
@ -861,7 +862,7 @@ PlacesTreeView.prototype = {
// The livemark attribute is set as a cell property on the title cell.
this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE);
}
}).bind(this)
}.bind(this)
);
}
},
@ -887,10 +888,11 @@ PlacesTreeView.prototype = {
}
PlacesUtils.livemarks.getLivemark({ id: aNode.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
let shouldInvalidate = !aNode._feedURI;
aNode._feedURI = aLivemark.feedURI;
let shouldInvalidate =
!this._controller.hasCachedLivemarkInfo(aNode);
this._controller.cacheLivemarkInfo(aNode, aLivemark);
if (aNewState == Components.interfaces.nsINavHistoryContainerResultNode.STATE_OPENED) {
aLivemark.registerForUpdates(aNode, this);
// Prioritize the current livemark.
@ -903,7 +905,7 @@ PlacesTreeView.prototype = {
aLivemark.unregisterForUpdates(aNode);
}
}
}).bind(this)
}.bind(this)
);
}
},
@ -999,7 +1001,7 @@ PlacesTreeView.prototype = {
}
}
if (aContainer._feedURI) {
if (this._controller.hasCachedLivemarkInfo(aContainer)) {
let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions;
if (!queryOptions.excludeItems) {
this._populateLivemarkContainer(aContainer);
@ -1169,20 +1171,20 @@ PlacesTreeView.prototype = {
}
else if (nodeType == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER ||
nodeType == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT) {
if (node._feedURI) {
if (this._controller.hasCachedLivemarkInfo(node)) {
properties.push(this._getAtomFor("livemark"));
}
else {
PlacesUtils.livemarks.getLivemark(
{ id: node.itemId },
(function (aStatus, aLivemark) {
function (aStatus, aLivemark) {
if (Components.isSuccessCode(aStatus)) {
node._feedURI = aLivemark.feedURI;
this._controller.cacheLivemarkInfo(node, aLivemark);
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);
}
}).bind(this)
}.bind(this)
);
}
}
@ -1198,7 +1200,7 @@ PlacesTreeView.prototype = {
else if (PlacesUtils.nodeIsURI(node)) {
properties.push(this._getAtomFor(PlacesUIUtils.guessUrlSchemeForUI(node.uri)));
if (node.parent._feedURI) {
if (this._controller.hasCachedLivemarkInfo(node.parent)) {
properties.push(this._getAtomFor("livemarkItem"));
if (node.accessCount) {
properties.push(this._getAtomFor("visited"));
@ -1253,7 +1255,7 @@ PlacesTreeView.prototype = {
return true;
let node = this._rows[aRow];
if (node._feedURI) {
if (this._controller.hasCachedLivemarkInfo(node)) {
let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions;
return queryOptions.excludeItems;
}
@ -1503,7 +1505,7 @@ PlacesTreeView.prototype = {
}
// Persist containers open status, but never persist livemarks.
if (!node._feedURI) {
if (!this._controller.hasCachedLivemarkInfo(node)) {
let resource = this._getResourceForNode(node);
if (resource) {
const openLiteral = PlacesUIUtils.RDF.GetResource("http://home.netscape.com/NC-rdf#open");