Bug 325092, r=bryner. Vacuuming history doesn't work.

browser/components/places/src/nsNavHistory.cpp

@@ -1060,15 +1060,17 @@ nsNavHistory::VacuumDB(PRTime aTimeAgo, PRBool aCompress)
   rv = nsFaviconService::VacuumFavicons(mDBConn);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  transaction.Commit();
+
   // compress the tables
   if (aCompress) {
-    rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("VACUUM moz_history"));
+#ifdef DEBUG
-    NS_ENSURE_SUCCESS(rv, rv);
+    PRBool inProgress = PR_FALSE;
-    rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("VACUUM moz_historyvisit"));
+    rv = mDBConn->GetTransactionInProgress(&inProgress);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ASSERTION(NS_SUCCEEDED(rv), "Can't get transaction status");
-    rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("VACUUM moz_anno"));
+    NS_ASSERTION(! inProgress, "You must not have a transaction in progress to vacuum!");
-    NS_ENSURE_SUCCESS(rv, rv);
+#endif
-    rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("VACUUM moz_favicon"));
+    rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("VACUUM"));
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -1411,7 +1413,7 @@ nsNavHistory::GetHasHistoryEntries(PRBool* aHasEntries)
 {
   nsCOMPtr<mozIStorageStatement> dbSelectStatement;
   nsresult rv = mDBConn->CreateStatement(
-      NS_LITERAL_CSTRING("SELECT url FROM moz_history LIMIT 1"),
+      NS_LITERAL_CSTRING("SELECT visit_id FROM moz_historyvisit LIMIT 1"),
       getter_AddRefs(dbSelectStatement));
   NS_ENSURE_SUCCESS(rv, rv);
   return dbSelectStatement->ExecuteStep(aHasEntries);
@@ -1998,41 +2000,21 @@ nsNavHistory::GetLastPageVisited(nsACString & aLastPageVisited)
 
 // nsNavHistory::GetCount
 //
-//    Finds the total number of history items.
+//    This function is used in legacy code to see if there is any history to
-//
+//    clear. Counting the actual number of history entries is very slow, so
-//    This function is useless, please don't use it. It's also very slow
+//    we just see if there are any and return 0 or 1, which is enough to make
-//    since in sqlite, count enumerates all results to see how many there are.
+//    all the code that uses this function happy.
-//
-//    If you want to see if there is any history, use HasHistoryEntries
 
 NS_IMETHODIMP
 nsNavHistory::GetCount(PRUint32 *aCount)
 {
-  NS_WARNING("Don't use history.count: it is slow and useless. Try hasHistoryEntries.");
+  PRBool hasEntries = PR_FALSE;
-
+  nsresult rv = GetHasHistoryEntries(&hasEntries);
-  nsCOMPtr<mozIStorageStatement> dbSelectStatement;
+  if (hasEntries)
-  nsresult rv = mDBConn->CreateStatement(
+    *aCount = 1;
-      NS_LITERAL_CSTRING("SELECT count(url) FROM moz_history"),
-      getter_AddRefs(dbSelectStatement));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  PRBool moreResults;
-  rv = dbSelectStatement->ExecuteStep(&moreResults);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (!moreResults) {
-    // huh? count() should always return one result
-    return NS_ERROR_FAILURE;
-  }
-
-  PRInt32 countSigned;
-  rv = dbSelectStatement->GetInt32(0, &countSigned);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (countSigned < 0)
-    *aCount = 0;
   else
-    *aCount = NS_STATIC_CAST(PRUint32, countSigned);
+    *aCount = 0;
-  return NS_OK;
+  return rv;
 }
 
 

browser/components/places/src/nsNavHistoryResult.cpp

@@ -2221,16 +2221,14 @@ nsNavHistoryQueryResultNode::OnItemChanged(nsIURI* aBookmark,
                                             const nsACString& aProperty,
                                             const nsAString& aValue)
 {
-  if (mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS)
+  NS_NOTREACHED("Everything observers should not get OnItemChanged, but should get the corresponding history notifications instead");
-    return Refresh();
   return NS_OK;
 }
 NS_IMETHODIMP
 nsNavHistoryQueryResultNode::OnItemVisited(nsIURI* aBookmark, PRInt64 aVisitId,
                                             PRTime aTime)
 {
-  if (mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS)
+  NS_NOTREACHED("Everything observers should not get OnItemVisited, but should get OnVisit instead");
-    return Refresh();
   return NS_OK;
 }
 NS_IMETHODIMP
@@ -2761,8 +2759,24 @@ nsNavHistoryFolderResultNode::OnItemVisited(nsIURI* aBookmark, PRInt64 aVisitId,
     mTime = aTime;
   ReverseUpdateStats(mAccessCount - oldAccessCount);
 
-  if (mVisibleIndex >= 0)
+  // update sorting if necessary
+  PRUint32 sortType = GetSortType();
+  if (sortType == nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_ASCENDING ||
+      sortType == nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING ||
+      sortType == nsINavHistoryQueryOptions::SORT_BY_DATE_ASCENDING ||
+      sortType == nsINavHistoryQueryOptions::SORT_BY_DATE_DESCENDING) {
+    PRInt32 childIndex = FindChild(node);
+    NS_ASSERTION(childIndex >= 0, "Could not find child we just got a reference to");
+    if (childIndex >= 0) {
+      SortComparator comparator = GetSortingComparator(GetSortType()); 
+      nsCOMPtr<nsINavHistoryURIResultNode> nodeLock(node);
+      RemoveChildAt(childIndex, PR_TRUE);
+      InsertChildAt(node, FindInsertionPoint(node, comparator), PR_TRUE);
+    }
+  } else if (mVisibleIndex >= 0) {
+    // no sorting changed, just redraw the row if visible
     result->RowChanged(node->mVisibleIndex);
+  }
   return NS_OK;
 }
 
@@ -4422,6 +4436,7 @@ nsNavHistoryResult::RowReplaced(PRInt32 aVisibleIndex,
 NS_IMETHODIMP
 nsNavHistoryResult::OnBeginUpdateBatch()
 {
+  ENUMERATE_HISTORY_OBSERVERS(OnBeginUpdateBatch());
   return NS_OK;
 }
 
@@ -4431,6 +4446,7 @@ nsNavHistoryResult::OnBeginUpdateBatch()
 NS_IMETHODIMP
 nsNavHistoryResult::OnEndUpdateBatch()
 {
+  ENUMERATE_HISTORY_OBSERVERS(OnEndUpdateBatch());
   return NS_OK;
 }
 
@@ -4444,6 +4460,7 @@ nsNavHistoryResult::OnItemAdded(nsIURI *aBookmark,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aFolder,
       OnItemAdded(aBookmark, aFolder, aIndex));
+  ENUMERATE_HISTORY_OBSERVERS(OnItemAdded(aBookmark, aFolder, aIndex));
   return NS_OK;
 }
 
@@ -4456,6 +4473,7 @@ nsNavHistoryResult::OnItemRemoved(nsIURI *aBookmark,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aFolder,
       OnItemRemoved(aBookmark, aFolder, aIndex));
+  ENUMERATE_HISTORY_OBSERVERS(OnItemRemoved(aBookmark, aFolder, aIndex));
   return NS_OK;
 }
 
@@ -4468,6 +4486,7 @@ nsNavHistoryResult::OnItemMoved(nsIURI *aBookmark, PRInt64 aFolder,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aFolder,
       OnItemMoved(aBookmark, aFolder, aOldIndex, aNewIndex));
+  ENUMERATE_HISTORY_OBSERVERS(OnItemMoved(aBookmark, aFolder, aOldIndex, aNewIndex));
   return NS_OK;
 
 }
@@ -4494,6 +4513,11 @@ nsNavHistoryResult::OnItemChanged(nsIURI *aBookmark,
   }
   if (folders)
     nsMemory::Free(folders);
+
+  // Note: we do NOT call history observers in this case. This notification is
+  // the same as other history notification, except that here we know the item
+  // is a bookmark. History observers will handle the history notification
+  // instead.
   return NS_OK;
 }
 
@@ -4508,6 +4532,7 @@ nsNavHistoryResult::OnItemVisited(nsIURI* aBookmark, PRInt64 aVisitId,
   nsNavBookmarks* bookmarkService = nsNavBookmarks::GetBookmarksService();
   NS_ENSURE_TRUE(bookmarkService, NS_ERROR_OUT_OF_MEMORY);
 
+  // find all the folders to notify about this item
   PRUint32 folderCount;
   PRInt64* folders;
   rv = bookmarkService->GetBookmarkFolders(aBookmark, &folderCount, &folders);
@@ -4518,6 +4543,10 @@ nsNavHistoryResult::OnItemVisited(nsIURI* aBookmark, PRInt64 aVisitId,
   }
   if (folders)
     nsMemory::Free(folders);
+
+  // Note: we do NOT call history observers in this case. This notification is
+  // the same as OnVisit, except that here we know the item is a bookmark.
+  // History observers will handle the history notification instead.
   return NS_OK;
 }
 
@@ -4530,6 +4559,7 @@ nsNavHistoryResult::OnItemReplaced(PRInt64 aFolder,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aFolder,
       OnItemReplaced(aFolder, aItem, aNewItem));
+  ENUMERATE_HISTORY_OBSERVERS(OnItemReplaced(aFolder, aItem, aNewItem));
   return NS_OK;
 }
 
@@ -4542,6 +4572,7 @@ nsNavHistoryResult::OnFolderAdded(PRInt64 aFolder,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aParent,
       OnFolderAdded(aFolder, aParent, aIndex));
+  ENUMERATE_HISTORY_OBSERVERS(OnFolderAdded(aFolder, aParent, aIndex));
   return NS_OK;
 }
 
@@ -4554,6 +4585,7 @@ nsNavHistoryResult::OnFolderRemoved(PRInt64 aFolder,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aParent,
       OnFolderRemoved(aFolder, aParent, aIndex));
+  ENUMERATE_HISTORY_OBSERVERS(OnFolderRemoved(aFolder, aParent, aIndex));
   return NS_OK;
 }
 
@@ -4576,6 +4608,8 @@ nsNavHistoryResult::OnFolderMoved(PRInt64 aFolder,
     ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aNewParent,
         OnFolderMoved(aFolder, aOldParent, aOldIndex, aNewParent, aNewIndex));
   }
+  ENUMERATE_HISTORY_OBSERVERS(OnFolderMoved(aFolder, aOldParent, aOldIndex,
+                                            aNewParent, aNewIndex));
   return NS_OK;
 }
 
@@ -4588,6 +4622,7 @@ nsNavHistoryResult::OnFolderChanged(PRInt64 aFolder,
 {
   ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aFolder,
       OnFolderChanged(aFolder, aProperty));
+  ENUMERATE_HISTORY_OBSERVERS(OnFolderChanged(aFolder, aProperty));
   return NS_OK;
 }
 

browser/components/places/src/nsNavHistoryResult.h

@@ -495,6 +495,8 @@ public:
   nsresult RemoveChildAt(PRInt32 aIndex, PRBool aIsTemporary = PR_FALSE);
 
   PRBool CanRemoteContainersChange();
+  void ChangeTitles(nsIURI* aURI, const nsACString& aNewTitle,
+                    PRBool aRecursive, PRBool aOnlyOne);
 };