Bug 992670 - Make all child insertions/removals non-temporary in nsNavHistoryContainerResultNode. r=mak

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<nsNavHistoryResultNode> 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);
 }
 
 

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,