From 279be7bfeec8f794ec42479752944c154dc312be Mon Sep 17 00:00:00 2001 From: Birunthan Mohanathas Date: Wed, 16 Apr 2014 13:56:52 -0400 Subject: [PATCH] Bug 992670 - Make all child insertions/removals non-temporary in nsNavHistoryContainerResultNode. r=mak --- .../components/places/nsNavHistoryResult.cpp | 79 +++++++------------ .../components/places/nsNavHistoryResult.h | 6 +- 2 files changed, 32 insertions(+), 53 deletions(-) diff --git a/toolkit/components/places/nsNavHistoryResult.cpp b/toolkit/components/places/nsNavHistoryResult.cpp index 64c8b0fc10f..5bf58268815 100644 --- a/toolkit/components/places/nsNavHistoryResult.cpp +++ b/toolkit/components/places/nsNavHistoryResult.cpp @@ -1300,23 +1300,17 @@ nsNavHistoryContainerResultNode::FindChildURI(const nsACString& aSpec, * This does the work of adding a child to the container. The child can be * either a container or or a single item that may even be collapsed with the * adjacent ones. - * - * Some inserts are "temporary" meaning that they are happening immediately - * after a temporary remove. We do this when movings elements when they - * change to keep them in the proper sorting position. In these cases, we - * don't need to recompute any statistics. */ nsresult nsNavHistoryContainerResultNode::InsertChildAt(nsNavHistoryResultNode* aNode, - int32_t aIndex, - bool aIsTemporary) + int32_t aIndex) { nsNavHistoryResult* result = GetResult(); NS_ENSURE_STATE(result); aNode->mParent = this; aNode->mIndentLevel = mIndentLevel + 1; - if (!aIsTemporary && aNode->IsContainer()) { + if (aNode->IsContainer()) { // need to update all the new item's children nsNavHistoryContainerResultNode* container = aNode->GetAsContainer(); container->mResult = result; @@ -1327,21 +1321,19 @@ nsNavHistoryContainerResultNode::InsertChildAt(nsNavHistoryResultNode* aNode, return NS_ERROR_OUT_OF_MEMORY; // Update our stats and notify the result's observers. - if (!aIsTemporary) { - mAccessCount += aNode->mAccessCount; - if (mTime < aNode->mTime) - mTime = aNode->mTime; - if (!mParent || mParent->AreChildrenVisible()) { - NOTIFY_RESULT_OBSERVERS(result, - NodeHistoryDetailsChanged(TO_ICONTAINER(this), - mTime, - mAccessCount)); - } - - nsresult rv = ReverseUpdateStats(aNode->mAccessCount); - NS_ENSURE_SUCCESS(rv, rv); + mAccessCount += aNode->mAccessCount; + if (mTime < aNode->mTime) + mTime = aNode->mTime; + if (!mParent || mParent->AreChildrenVisible()) { + NOTIFY_RESULT_OBSERVERS(result, + NodeHistoryDetailsChanged(TO_ICONTAINER(this), + mTime, + mAccessCount)); } + nsresult rv = ReverseUpdateStats(aNode->mAccessCount); + NS_ENSURE_SUCCESS(rv, rv); + // Update tree if we are visible. Note that we could be here and not // expanded, like when there is a bookmark folder being updated because its // parent is visible. @@ -1358,12 +1350,12 @@ nsNavHistoryContainerResultNode::InsertChildAt(nsNavHistoryResultNode* aNode, */ nsresult nsNavHistoryContainerResultNode::InsertSortedChild( - nsNavHistoryResultNode* aNode, - bool aIsTemporary, bool aIgnoreDuplicates) + nsNavHistoryResultNode* aNode, + bool aIgnoreDuplicates) { if (mChildren.Count() == 0) - return InsertChildAt(aNode, 0, aIsTemporary); + return InsertChildAt(aNode, 0); SortComparator comparator = GetSortingComparator(GetSortType()); if (comparator) { @@ -1373,7 +1365,7 @@ nsNavHistoryContainerResultNode::InsertSortedChild( // level. Doing this twice shouldn't be a large performance penalty because // when we are inserting new containers, they typically contain only one // item (because we've browsed a new page). - if (!aIsTemporary && aNode->IsContainer()) { + if (aNode->IsContainer()) { // need to update all the new item's children nsNavHistoryContainerResultNode* container = aNode->GetAsContainer(); container->mResult = mResult; @@ -1389,9 +1381,9 @@ nsNavHistoryContainerResultNode::InsertSortedChild( if (aIgnoreDuplicates && itemExists) return NS_OK; - return InsertChildAt(aNode, position, aIsTemporary); + return InsertChildAt(aNode, position); } - return InsertChildAt(aNode, mChildren.Count(), aIsTemporary); + return InsertChildAt(aNode, mChildren.Count()); } /** @@ -1437,15 +1429,9 @@ nsNavHistoryContainerResultNode::EnsureItemPosition(uint32_t aIndex) { * This does all the work of removing a child from this container, including * updating the tree if necessary. Note that we do not need to be open for * this to work. - * - * Some removes are "temporary" meaning that they'll just get inserted again. - * We do this for resorting. In these cases, we don't need to recompute any - * statistics, and we shouldn't notify those container that they are being - * removed. */ nsresult -nsNavHistoryContainerResultNode::RemoveChildAt(int32_t aIndex, - bool aIsTemporary) +nsNavHistoryContainerResultNode::RemoveChildAt(int32_t aIndex) { NS_ASSERTION(aIndex >= 0 && aIndex < mChildren.Count(), "Invalid index"); @@ -1453,13 +1439,10 @@ nsNavHistoryContainerResultNode::RemoveChildAt(int32_t aIndex, nsRefPtr oldNode = mChildren[aIndex]; // Update stats. - uint32_t oldAccessCount = 0; - if (!aIsTemporary) { - MOZ_ASSERT(mAccessCount >= mChildren[aIndex]->mAccessCount, - "Invalid access count while updating!"); - oldAccessCount = mAccessCount; - mAccessCount -= mChildren[aIndex]->mAccessCount; - } + MOZ_ASSERT(mAccessCount >= mChildren[aIndex]->mAccessCount, + "Invalid access count while updating!"); + uint32_t oldAccessCount = mAccessCount; + mAccessCount -= mChildren[aIndex]->mAccessCount; // Remove it from our list and notify the result's observers. mChildren.RemoveObjectAt(aIndex); @@ -1469,11 +1452,9 @@ nsNavHistoryContainerResultNode::RemoveChildAt(int32_t aIndex, NodeRemoved(this, oldNode, aIndex)); } - if (!aIsTemporary) { - nsresult rv = ReverseUpdateStats(mAccessCount - oldAccessCount); - NS_ENSURE_SUCCESS(rv, rv); - oldNode->OnRemoving(); - } + nsresult rv = ReverseUpdateStats(mAccessCount - oldAccessCount); + NS_ENSURE_SUCCESS(rv, rv); + oldNode->OnRemoving(); return NS_OK; } @@ -2586,7 +2567,7 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI, rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node)); NS_ENSURE_SUCCESS(rv, rv); if (history->EvaluateQueryForNode(mQueries, mOptions, node)) { - rv = InsertSortedChild(node, true); + rv = InsertSortedChild(node); NS_ENSURE_SUCCESS(rv, rv); } } @@ -2799,7 +2780,7 @@ nsNavHistoryQueryResultNode::NotifyIfTagsChanged(nsIURI* aURI) rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node)); NS_ENSURE_SUCCESS(rv, rv); if (history->EvaluateQueryForNode(mQueries, mOptions, node)) { - rv = InsertSortedChild(node, true); + rv = InsertSortedChild(node); NS_ENSURE_SUCCESS(rv, rv); } } @@ -3634,7 +3615,7 @@ nsNavHistoryFolderResultNode::OnItemAdded(int64_t aItemId, } // insert at sorted position - return InsertSortedChild(node, false); + return InsertSortedChild(node); } diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index bac24871bb4..6fbbce3f982 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -576,14 +576,12 @@ public: int32_t FindChild(nsNavHistoryResultNode* aNode) { return mChildren.IndexOf(aNode); } - nsresult InsertChildAt(nsNavHistoryResultNode* aNode, int32_t aIndex, - bool aIsTemporary = false); + nsresult InsertChildAt(nsNavHistoryResultNode* aNode, int32_t aIndex); nsresult InsertSortedChild(nsNavHistoryResultNode* aNode, - bool aIsTemporary = false, bool aIgnoreDuplicates = false); bool EnsureItemPosition(uint32_t aIndex); - nsresult RemoveChildAt(int32_t aIndex, bool aIsTemporary = false); + nsresult RemoveChildAt(int32_t aIndex); void RecursiveFindURIs(bool aOnlyOne, nsNavHistoryContainerResultNode* aContainer,