Backed out changesets e128ceec086c and 7e31deb02ba8 (bug 992670) for mochitest-bc and xpcshell crashes.
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 16 Apr 2014 15:16:30 -0400
changeset 197339 c60b059c47ca40af1e2f21aaaa23da263ed0a39b
parent 197338 7acce569bd84bc5b264c28df67c6066f527a27fb
child 197340 8a13590ce482280ea38a5c886e9812d918ed6848
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs992670
milestone31.0a1
backs oute128ceec086cf203af55501ac688534c1c9610f8
7e31deb02ba88d389eae04f5a528d0696e22c714
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changesets e128ceec086c and 7e31deb02ba8 (bug 992670) for mochitest-bc and xpcshell crashes.
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/places/nsNavHistoryResult.h
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -1295,100 +1295,108 @@ nsNavHistoryContainerResultNode::FindChi
   }
   return nullptr;
 }
 
 /**
  * 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)
+                                               int32_t aIndex,
+                                               bool aIsTemporary)
 {
   nsNavHistoryResult* result = GetResult();
   NS_ENSURE_STATE(result);
 
   aNode->mParent = this;
   aNode->mIndentLevel = mIndentLevel + 1;
-  if (aNode->IsContainer()) {
+  if (!aIsTemporary && aNode->IsContainer()) {
     // need to update all the new item's children
     nsNavHistoryContainerResultNode* container = aNode->GetAsContainer();
     container->mResult = result;
     container->FillStats();
   }
 
   if (!mChildren.InsertObjectAt(aNode, aIndex))
     return NS_ERROR_OUT_OF_MEMORY;
 
   // Update our stats and notify the result's observers.
-  mAccessCount += aNode->mAccessCount;
-  if (mTime < aNode->mTime)
-    mTime = aNode->mTime;
-  if (!mParent || mParent->AreChildrenVisible()) {
-    NOTIFY_RESULT_OBSERVERS(result,
-                            NodeHistoryDetailsChanged(TO_ICONTAINER(this),
-                                                      mTime,
-                                                      mAccessCount));
+  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);
   }
 
-  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.
   if (AreChildrenVisible())
     NOTIFY_RESULT_OBSERVERS(result, NodeInserted(this, aNode, aIndex));
 
   return NS_OK;
 }
 
 
 /**
  * This locates the proper place for insertion according to the current sort
  * and calls InsertChildAt
  */
 nsresult
 nsNavHistoryContainerResultNode::InsertSortedChild(
-    nsNavHistoryResultNode* aNode,
-    bool aIgnoreDuplicates)
+    nsNavHistoryResultNode* aNode, 
+    bool aIsTemporary, bool aIgnoreDuplicates)
 {
 
   if (mChildren.Count() == 0)
-    return InsertChildAt(aNode, 0);
+    return InsertChildAt(aNode, 0, aIsTemporary);
 
   SortComparator comparator = GetSortingComparator(GetSortType());
   if (comparator) {
     // When inserting a new node, it must have proper statistics because we use
     // them to find the correct insertion point.  The insert function will then
     // recompute these statistics and fill in the proper parents and hierarchy
     // 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 (aNode->IsContainer()) {
+    if (!aIsTemporary && aNode->IsContainer()) {
       // need to update all the new item's children
       nsNavHistoryContainerResultNode* container = aNode->GetAsContainer();
       container->mResult = mResult;
       container->FillStats();
     }
 
     nsAutoCString sortingAnnotation;
     GetSortingAnnotation(sortingAnnotation);
     bool itemExists;
     uint32_t position = FindInsertionPoint(aNode, comparator, 
                                            sortingAnnotation.get(), 
                                            &itemExists);
     if (aIgnoreDuplicates && itemExists)
       return NS_OK;
 
-    return InsertChildAt(aNode, position);
+    return InsertChildAt(aNode, position, aIsTemporary);
   }
-  return InsertChildAt(aNode, mChildren.Count());
+  return InsertChildAt(aNode, mChildren.Count(), aIsTemporary);
 }
 
 /**
  * This checks if the item at aIndex is located correctly given the sorting
  * move.  If it's not, the item is moved, and the result's observers are
  * notified.
  *
  * @return true if the item position has been changed, false otherwise.
@@ -1424,42 +1432,52 @@ nsNavHistoryContainerResultNode::EnsureI
 
   return true;
 }
 
 /**
  * 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)
+nsNavHistoryContainerResultNode::RemoveChildAt(int32_t aIndex,
+                                               bool aIsTemporary)
 {
   NS_ASSERTION(aIndex >= 0 && aIndex < mChildren.Count(), "Invalid index");
 
   // Hold an owning reference to keep from expiring while we work with it.
   nsRefPtr<nsNavHistoryResultNode> oldNode = mChildren[aIndex];
 
   // Update stats.
-  MOZ_ASSERT(mAccessCount >= mChildren[aIndex]->mAccessCount,
-             "Invalid access count while updating!");
-  uint32_t oldAccessCount = mAccessCount;
-  mAccessCount -= mChildren[aIndex]->mAccessCount;
+  uint32_t oldAccessCount = 0;
+  if (!aIsTemporary) {
+    oldAccessCount = mAccessCount;
+    mAccessCount -= mChildren[aIndex]->mAccessCount;
+    NS_ASSERTION(mAccessCount >= 0, "Invalid access count while updating!");
+  }
 
   // Remove it from our list and notify the result's observers.
   mChildren.RemoveObjectAt(aIndex);
   if (AreChildrenVisible()) {
     nsNavHistoryResult* result = GetResult();
     NOTIFY_RESULT_OBSERVERS(result,
                             NodeRemoved(this, oldNode, aIndex));
   }
 
-  nsresult rv = ReverseUpdateStats(mAccessCount - oldAccessCount);
-  NS_ENSURE_SUCCESS(rv, rv);
-  oldNode->OnRemoving();
+  if (!aIsTemporary) {
+    nsresult rv = ReverseUpdateStats(mAccessCount - oldAccessCount);
+    NS_ENSURE_SUCCESS(rv, rv);
+    oldNode->OnRemoving();
+  }
   return NS_OK;
 }
 
 
 /**
  * Searches for matches for the given URI.  If aOnlyOne is set, it will
  * terminate as soon as it finds a single match.  This would be used when there
  * are URI results so there will only ever be one copy of any URI.
@@ -2562,17 +2580,17 @@ nsNavHistoryQueryResultNode::OnTitleChan
       // This could be a new node matching the query, thus we could need
       // to add it to the result.
       nsRefPtr<nsNavHistoryResultNode> node;
       nsNavHistory* history = nsNavHistory::GetHistoryService();
       NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
       rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node));
       NS_ENSURE_SUCCESS(rv, rv);
       if (history->EvaluateQueryForNode(mQueries, mOptions, node)) {
-        rv = InsertSortedChild(node);
+        rv = InsertSortedChild(node, true);
         NS_ENSURE_SUCCESS(rv, rv);
       }
     }
     for (int32_t i = 0; i < matches.Count(); ++i) {
       // For each matched node we check if it passes the query filter, if not
       // we remove the node from the result, otherwise we'll update the title
       // later.
       nsNavHistoryResultNode* node = matches[i];
@@ -2775,17 +2793,17 @@ nsNavHistoryQueryResultNode::NotifyIfTag
   RecursiveFindURIs(onlyOneEntry, this, spec, &matches);
 
   if (matches.Count() == 0 && mHasSearchTerms && !mRemovingURI) {
     // A new tag has been added, it's possible it matches our query.
     NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
     rv = history->URIToResultNode(aURI, mOptions, getter_AddRefs(node));
     NS_ENSURE_SUCCESS(rv, rv);
     if (history->EvaluateQueryForNode(mQueries, mOptions, node)) {
-      rv = InsertSortedChild(node);
+      rv = InsertSortedChild(node, true);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
   for (int32_t i = 0; i < matches.Count(); ++i) {
     nsNavHistoryResultNode* node = matches[i];
     // Force a tags update before checking the node.
     node->mTags.SetIsVoid(true);
@@ -3610,17 +3628,17 @@ nsNavHistoryFolderResultNode::OnItemAdde
 
   if (aItemType == nsINavBookmarksService::TYPE_SEPARATOR ||
       GetSortType() == nsINavHistoryQueryOptions::SORT_BY_NONE) {
     // insert at natural bookmarks position
     return InsertChildAt(node, aIndex);
   }
 
   // insert at sorted position
-  return InsertSortedChild(node);
+  return InsertSortedChild(node, false);
 }
 
 
 NS_IMETHODIMP
 nsNavHistoryFolderResultNode::OnItemRemoved(int64_t aItemId,
                                             int64_t aParentFolder,
                                             int32_t aIndex,
                                             uint16_t aItemType,
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -571,22 +571,24 @@ public:
     return FindChildURI(spec, aNodeIndex);
   }
   nsNavHistoryResultNode* FindChildURI(const nsACString& aSpec,
                                        uint32_t* aNodeIndex);
   // returns the index of the given node, -1 if not found
   int32_t FindChild(nsNavHistoryResultNode* aNode)
     { return mChildren.IndexOf(aNode); }
 
-  nsresult InsertChildAt(nsNavHistoryResultNode* aNode, int32_t aIndex);
+  nsresult InsertChildAt(nsNavHistoryResultNode* aNode, int32_t aIndex,
+                         bool aIsTemporary = false);
   nsresult InsertSortedChild(nsNavHistoryResultNode* aNode,
+                             bool aIsTemporary = false,
                              bool aIgnoreDuplicates = false);
   bool EnsureItemPosition(uint32_t aIndex);
 
-  nsresult RemoveChildAt(int32_t aIndex);
+  nsresult RemoveChildAt(int32_t aIndex, bool aIsTemporary = false);
 
   void RecursiveFindURIs(bool aOnlyOne,
                          nsNavHistoryContainerResultNode* aContainer,
                          const nsCString& aSpec,
                          nsCOMArray<nsNavHistoryResultNode>* aMatches);
   bool UpdateURIs(bool aRecursive, bool aOnlyOne, bool aUpdateSort,
                   const nsCString& aSpec,
                   nsresult (*aCallback)(nsNavHistoryResultNode*, const void*,