Bug 395066 Hang when trying to clear browsing history (r=sspitzer, a=mconnor)

toolkit/components/places/src/nsNavHistoryExpire.cpp

@@ -532,28 +532,19 @@ nsresult
 nsNavHistoryExpire::EraseAnnotations(mozIStorageConnection* aConnection,
     const nsTArray<nsNavHistoryExpireRecord>& aRecords)
 {
-  nsresult rv;
-  nsCOMPtr<nsIAnnotationService> annotationService = do_GetService("@mozilla.org/browser/annotation-service;1", &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCOMPtr<mozIStorageStatement> statement;
-  rv = aConnection->CreateStatement(NS_LITERAL_CSTRING(
-    "DELETE FROM moz_annos WHERE id in ("
-      "SELECT a.id from moz_annos a JOIN moz_places p on a.place_id = p.id "
-      "WHERE p.url = ?1 AND a.expiration != ?2)"),
-    getter_AddRefs(statement));
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // remove annotations for the set of records passed in
+  nsCString placeIds;
   for (PRUint32 i = 0; i < aRecords.Length(); i ++) {
-    // delete annotations (except EXPIRE_NEVER)
+    if (!placeIds.IsEmpty())
-    rv = statement->BindUTF8StringParameter(0, aRecords[i].uri);
+      placeIds.AppendLiteral(", ");
-    NS_ENSURE_SUCCESS(rv, rv);
+    placeIds.AppendInt(aRecords[i].placeID);
-    rv = statement->BindInt32Parameter(1, nsIAnnotationService::EXPIRE_NEVER);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = statement->Execute();
-    NS_ENSURE_SUCCESS(rv, rv);
   }
+  
+  nsresult rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DELETE FROM moz_annos WHERE place_id in (") +
+      placeIds + NS_LITERAL_CSTRING(") AND expiration != ") +
+      nsPrintfCString("%d", nsIAnnotationService::EXPIRE_NEVER));
+  NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 

toolkit/components/places/tests/unit/test_expiration.js

@@ -135,20 +135,35 @@ function run_test() {
   but doesn't remove bookmarks or EXPIRE_NEVER annotations.
   */
   var removeAllTestURI = uri("http://removeallpages.com");
+  var removeAllTestURINever = uri("http://removeallpagesnever.com");
   histsvc.addVisit(removeAllTestURI, Date.now(), 0, histsvc.TRANSITION_TYPED, false, 0);
+  var bmURI = uri("http://bookmarked");
+  bmsvc.insertBookmark(bmsvc.bookmarksRoot, bmURI, bmsvc.DEFAULT_INDEX, "foo");
+  //bhist.addPageWithDetails(placeURI, "place uri", Date.now());
+  var placeURI = uri("place:folder=23");
+  bhist.addPageWithDetails(placeURI, "place uri", Date.now());
   annosvc.setPageAnnotation(removeAllTestURI, testAnnoName + "Hist", testAnnoVal, 0, annosvc.EXPIRE_WITH_HISTORY);
-  annosvc.setPageAnnotation(removeAllTestURI, testAnnoName + "Never", testAnnoVal, 0, annosvc.EXPIRE_NEVER);
+  annosvc.setPageAnnotation(removeAllTestURINever, testAnnoName + "Never", testAnnoVal, 0, annosvc.EXPIRE_NEVER);
   bhist.removeAllPages();
   try {
-    annosvc.getPageAnnotation(testAnnoName + "Hist");
+    annosvc.getPageAnnotation(removeAllTestURI, testAnnoName + "Hist");
-    do_throw("nsIBrowserHistory.removePagesFromHost() didn't remove an EXPIRE_WITH_HISTORY annotation");
+    do_throw("nsIBrowserHistory.removeAllPages() didn't remove an EXPIRE_WITH_HISTORY annotation");
   } catch(ex) {}
+  // test that the moz_places record was removed for this URI
+  do_check_eq(histsvc.getPageTitle(removeAllTestURI), null);
   try {
-    do_check_eq(annosvc.getPageAnnotation(removeAllTestURI, testAnnoName + "Never"), testAnnoVal);
+    do_check_eq(annosvc.getPageAnnotation(removeAllTestURINever, testAnnoName + "Never"), testAnnoVal);
-    annosvc.removePageAnnotation(removeAllTestURI, testAnnoName + "Never");
+    annosvc.removePageAnnotation(removeAllTestURINever, testAnnoName + "Never");
   } catch(ex) {
     do_throw("nsIBrowserHistory.removeAllPages deleted EXPIRE_NEVER annos!");
   }
+  // test that the moz_places record was not removed for EXPIRE_NEVER anno
+  do_check_neq(histsvc.getPageTitle(removeAllTestURINever), null);
+  // for place URI
+  do_check_neq(histsvc.getPageTitle(placeURI), null);
+  // for bookmarked URI
+  do_check_neq(histsvc.getPageTitle(bmURI), null);
+  
 
   /*
   test age-based history and anno expiration via the browser.history_expire_days pref.