Bug 629814 - Async history API should ignore place ids

r=mak
sr=rstrong
a=softblocker

toolkit/components/places/public/mozIAsyncHistory.idl

@@ -158,7 +158,7 @@ interface mozIAsyncHistory : nsISupports
    *
    * @throws NS_ERROR_INVALID_ARG
    *         - Passing in NULL for aPlaceInfo.
-   *         - Not providing at least one valid guid, placeId, or uri for all
+   *         - Not providing at least one valid guid, or uri for all
    *           mozIPlaceInfo object[s].
    *         - Not providing an array or nothing for the visits property of
    *           mozIPlaceInfo.

toolkit/components/places/src/History.cpp

@@ -589,8 +589,6 @@ private:
  *        The URI to check.
  * @param [optional] aGUID
  *        The guid of the URI to check.  This is passed back to the callback.
- * @param [optional] aPlaceId
- *        The placeId of the URI to check.  This is passed back to the callback.
  * @param [optional] aCallback
  *        The callback to notify if the URI cannot be added to history.
  * @return true if the URI can be added to history, false otherwise.
@@ -598,7 +596,6 @@ private:
 bool
 CanAddURI(nsIURI* aURI,
           const nsCString& aGUID = EmptyCString(),
-          PRInt64 aPlaceId = 0,
           mozIVisitInfoCallback* aCallback = NULL)
 {
   nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
@@ -619,7 +616,6 @@ CanAddURI(nsIURI* aURI,
 
     VisitData place(aURI);
     place.guid = aGUID;
-    place.placeId = aPlaceId;
     nsCOMPtr<nsIRunnable> event =
       new NotifyCompletion(aCallback, place, NS_ERROR_INVALID_ARG);
     (void)NS_DispatchToMainThread(event);
@@ -1855,15 +1851,6 @@ History::UpdatePlaces(const jsval& aPlaceInfos,
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIURI> uri = GetURIFromJSObject(aCtx, info, "uri");
-    PRInt64 placeId;
-    rv = GetIntFromJSObject(aCtx, info, "placeId", &placeId);
-    if (rv == NS_ERROR_INVALID_ARG) {
-      placeId = 0;
-    }
-    else {
-      NS_ENSURE_SUCCESS(rv, rv);
-      NS_ENSURE_ARG(placeId > 0);
-    }
     nsCString guid;
     {
       nsString fatGUID;
@@ -1878,12 +1865,12 @@ History::UpdatePlaces(const jsval& aPlaceInfos,
 
     // Make sure that any uri we are given can be added to history, and if not,
     // skip it (CanAddURI will notify our callback for us).
-    if (uri && !CanAddURI(uri, guid, placeId, aCallback)) {
+    if (uri && !CanAddURI(uri, guid, aCallback)) {
       continue;
     }
 
-    // We must have at least one of uri, valid id, or guid.
-    NS_ENSURE_ARG(uri || placeId > 0 || !guid.IsVoid());
+    // We must have at least one of uri or guid.
+    NS_ENSURE_ARG(uri || !guid.IsVoid());
 
     // If we were given a guid, make sure it is valid.
     bool isValidGUID = IsValidGUID(guid);
@@ -1918,7 +1905,6 @@ History::UpdatePlaces(const jsval& aPlaceInfos,
       NS_ENSURE_SUCCESS(rv, rv);
 
       VisitData& data = *visitData.AppendElement(VisitData(uri));
-      data.placeId = placeId;
       data.title = title;
       data.guid = guid;
 

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

@@ -198,37 +198,6 @@ function test_invalid_places_throws()
   run_next_test();
 }
 
-function test_invalid_id_throws()
-{
-  // First check invalid id "0".
-  let place = {
-    placeId: 0,
-    uri: NetUtil.newURI(TEST_DOMAIN + "test_invalid_id_throws"),
-    visits: [
-      new VisitInfo(),
-    ],
-  };
-  try {
-    gHistory.updatePlaces(place);
-    do_throw("Should have thrown!");
-  }
-  catch (e) {
-    do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
-  }
-
-  // Now check negative id.
-  place.placeId = -5;
-  try {
-    gHistory.updatePlaces(place);
-    do_throw("Should have thrown!");
-  }
-  catch (e) {
-    do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
-  }
-
-  run_next_test();
-}
-
 function test_invalid_guid_throws()
 {
   // First check invalid length guid.
@@ -272,7 +241,6 @@ function test_no_visits_throws()
     let str = "Testing place with " +
       (aPlace.uri ? "uri" : "no uri") + ", " +
       (aPlace.guid ? "guid" : "no guid") + ", " +
-      (aPlace.placeId ? "placeId" : "no placeId") + ", " +
       (aPlace.visits ? "visits array" : "no visits array");
     do_log_info(str);
   };
@@ -287,20 +255,16 @@ function test_no_visits_throws()
     for (let guid = 1; guid >= 0; guid--) {
       place.guid = guid ? TEST_GUID : undefined;
 
-      for (let placeId = 1; placeId >= 0; placeId--) {
-        place.placeId = placeId ? TEST_PLACEID : undefined;
+      for (let visits = 1; visits >= 0; visits--) {
+        place.visits = visits ? [] : undefined;
 
-        for (let visits = 1; visits >= 0; visits--) {
-          place.visits = visits ? [] : undefined;
-
-          log_test_conditions(place);
-          try {
-            gHistory.updatePlaces(place);
-            do_throw("Should have thrown!");
-          }
-          catch (e) {
-            do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
-          }
+        log_test_conditions(place);
+        try {
+          gHistory.updatePlaces(place);
+          do_throw("Should have thrown!");
+        }
+        catch (e) {
+          do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
         }
       }
     }
@@ -894,7 +858,6 @@ let gTests = [
   test_interface_exists,
   test_invalid_uri_throws,
   test_invalid_places_throws,
-  test_invalid_id_throws,
   test_invalid_guid_throws,
   test_no_visits_throws,
   test_add_visit_no_date_throws,