Bug 629814 - Async history API should ignore place ids
authorShawn Wilsher <me@shawnwilsher.com>
Thu, 03 Feb 2011 14:09:37 -0800
changeset 61908 1cb679417a8ba3f4f5e13c7bc880a9c70944f3f8
parent 61907 ab87d34e89fa00423266c8177ee3eb1811e08f61
child 61909 aebd2f355859be18ff73b6462d5ce9abf0ab9ce2
push id18532
push usersdwilsh@shawnwilsher.com
push dateThu, 03 Feb 2011 22:13:13 +0000
treeherdermozilla-central@aebd2f355859 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs629814
milestone2.0b12pre
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
Bug 629814 - Async history API should ignore place ids r=mak sr=rstrong a=softblocker
toolkit/components/places/public/mozIAsyncHistory.idl
toolkit/components/places/src/History.cpp
toolkit/components/places/tests/unit/test_async_history_api.js
--- a/toolkit/components/places/public/mozIAsyncHistory.idl
+++ b/toolkit/components/places/public/mozIAsyncHistory.idl
@@ -153,17 +153,17 @@ interface mozIAsyncHistory : nsISupports
    *        update.  This can be a single object, or an array of objects.
    * @param [optional] aCallback
    *        Callback to be notified for each visit addition, title change, or
    *        guid change.  If more than one operation is done for a given visit,
    *        only one callback will be given (i.e. title change and add visit).
    *
    * @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.
    *         - Not providing a visitDate and transitionType for each
    *           mozIVisitInfo.
    *         - Providing an invalid transitionType for a mozIVisitInfo.
    */
   [implicit_jscontext]
--- a/toolkit/components/places/src/History.cpp
+++ b/toolkit/components/places/src/History.cpp
@@ -584,26 +584,23 @@ private:
 /**
  * Checks to see if we can add aURI to history, and dispatches an error to
  * aCallback (if provided) if we cannot.
  *
  * @param aURI
  *        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.
  */
 bool
 CanAddURI(nsIURI* aURI,
           const nsCString& aGUID = EmptyCString(),
-          PRInt64 aPlaceId = 0,
           mozIVisitInfoCallback* aCallback = NULL)
 {
   nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(navHistory, false);
 
   PRBool canAdd;
   nsresult rv = navHistory->CanAddURI(aURI, &canAdd);
   if (NS_SUCCEEDED(rv) && canAdd) {
@@ -614,17 +611,16 @@ CanAddURI(nsIURI* aURI,
   if (aCallback) {
     // NotifyCompletion does not hold a strong reference to the callback, so we
     // have to manage it by AddRefing now and then releasing it after the event
     // has run.
     NS_ADDREF(aCallback);
 
     VisitData place(aURI);
     place.guid = aGUID;
-    place.placeId = aPlaceId;
     nsCOMPtr<nsIRunnable> event =
       new NotifyCompletion(aCallback, place, NS_ERROR_INVALID_ARG);
     (void)NS_DispatchToMainThread(event);
 
     // Also dispatch an event to release our reference to the callback after
     // NotifyCompletion has run.
     nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
     (void)NS_ProxyRelease(mainThread, aCallback, PR_TRUE);
@@ -1850,45 +1846,36 @@ History::UpdatePlaces(const jsval& aPlac
 
   nsTArray<VisitData> visitData;
   for (jsuint i = 0; i < infosLength; i++) {
     JSObject* info;
     nsresult rv = GetJSObjectFromArray(aCtx, infos, i, &info);
     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;
       GetStringFromJSObject(aCtx, info, "guid", fatGUID);
       if (fatGUID.IsVoid()) {
         guid.SetIsVoid(PR_TRUE);
       }
       else {
         guid = NS_ConvertUTF16toUTF8(fatGUID);
       }
     }
 
     // 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);
     NS_ENSURE_ARG(guid.IsVoid() || isValidGUID);
 
     nsString title;
     GetStringFromJSObject(aCtx, info, "title", title);
 
@@ -1913,17 +1900,16 @@ History::UpdatePlaces(const jsval& aPlac
     // Check each visit, and build our array of VisitData objects.
     visitData.SetCapacity(visitData.Length() + visitsLength);
     for (jsuint j = 0; j < visitsLength; j++) {
       JSObject* visit;
       rv = GetJSObjectFromArray(aCtx, visits, j, &visit);
       NS_ENSURE_SUCCESS(rv, rv);
 
       VisitData& data = *visitData.AppendElement(VisitData(uri));
-      data.placeId = placeId;
       data.title = title;
       data.guid = guid;
 
       // We must have a date and a transaction type!
       rv = GetIntFromJSObject(aCtx, visit, "visitDate", &data.visitTime);
       NS_ENSURE_SUCCESS(rv, rv);
       PRUint32 transitionType;
       rv = GetIntFromJSObject(aCtx, visit, "transitionType", &transitionType);
--- a/toolkit/components/places/tests/unit/test_async_history_api.js
+++ b/toolkit/components/places/tests/unit/test_async_history_api.js
@@ -193,47 +193,16 @@ function test_invalid_places_throws()
     catch (e) {
       do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
     }
   }
 
   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.
   let place = {
     guid: "BAD_GUID",
     uri: NetUtil.newURI(TEST_DOMAIN + "test_invalid_guid_throws"),
     visits: [
       new VisitInfo(),
@@ -267,45 +236,40 @@ function test_no_visits_throws()
     NetUtil.newURI(TEST_DOMAIN + "test_no_id_or_guid_no_visits_throws");
   const TEST_GUID = "_RANDOMGUID_";
   const TEST_PLACEID = 2;
 
   let log_test_conditions = function(aPlace) {
     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);
   };
 
   // Loop through every possible case.  Note that we don't actually care about
   // the case where we have no uri, place id, or guid (covered by another test),
   // but it is easier to just make sure it too throws than to exclude it.
   let place = { };
   for (let uri = 1; uri >= 0; uri--) {
     place.uri = uri ? TEST_URI : undefined;
 
     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);
         }
       }
     }
   }
 
   run_next_test();
 }
 
@@ -889,17 +853,16 @@ function test_title_change_notifies()
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Runner
 
 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,
   test_add_visit_no_transitionType_throws,
   test_add_visit_invalid_transitionType_throws,
   test_non_addable_uri_errors,
   test_observer_topic_dispatched_when_complete,
   test_add_visit,