Bug 392401 EXPIRE_HISTORY expiration policy should be disallowed for item annotations (r=mano)
authordietrich@mozilla.com
Sun, 19 Aug 2007 13:02:48 -0700
changeset 4797 a59ffaa491b8140e5491035ecf355ef4be67b21c
parent 4796 168e5b87c86bf532ff081db740c4917ef0f89939
child 4798 41ebb2448c027fb37b01f41a06bd48f44f3463fe
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmano
bugs392401
milestone1.9a8pre
Bug 392401 EXPIRE_HISTORY expiration policy should be disallowed for item annotations (r=mano)
toolkit/components/places/src/nsAnnotationService.cpp
toolkit/components/places/tests/unit/test_annotations.js
toolkit/components/places/tests/unit/test_expiration.js
--- a/toolkit/components/places/src/nsAnnotationService.cpp
+++ b/toolkit/components/places/src/nsAnnotationService.cpp
@@ -406,16 +406,19 @@ NS_IMETHODIMP
 nsAnnotationService::SetItemAnnotation(PRInt64 aItemId,
                                        const nsACString& aName,
                                        nsIVariant* aValue,
                                        PRInt32 aFlags,
                                        PRUint16 aExpiration)
 {
   NS_ENSURE_ARG(aValue);
 
+  if (aExpiration == EXPIRE_WITH_HISTORY)
+    return NS_ERROR_INVALID_ARG;
+
   PRUint16 dataType;
   nsresult rv = aValue->GetDataType(&dataType);
   NS_ENSURE_SUCCESS(rv, rv);
 
   switch (dataType) {
     case nsIDataType::VTYPE_INT8:
     case nsIDataType::VTYPE_UINT8:
     case nsIDataType::VTYPE_INT16:
@@ -500,16 +503,19 @@ nsAnnotationService::SetPageAnnotationSt
 
 NS_IMETHODIMP
 nsAnnotationService::SetItemAnnotationString(PRInt64 aItemId,
                                              const nsACString& aName,
                                              const nsAString& aValue,
                                              PRInt32 aFlags,
                                              PRUint16 aExpiration)
 {
+  if (aExpiration == EXPIRE_WITH_HISTORY)
+    return NS_ERROR_INVALID_ARG;
+
   nsresult rv = SetAnnotationStringInternal(aItemId, PR_TRUE, aName, aValue,
                                             aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   CallSetForItemObservers(aItemId, aName);
   return NS_OK;
 }
 
@@ -573,16 +579,19 @@ nsAnnotationService::SetPageAnnotationIn
 
 NS_IMETHODIMP
 nsAnnotationService::SetItemAnnotationInt32(PRInt64 aItemId,
                                             const nsACString& aName,
                                             PRInt32 aValue,
                                             PRInt32 aFlags,
                                             PRUint16 aExpiration)
 {
+  if (aExpiration == EXPIRE_WITH_HISTORY)
+    return NS_ERROR_INVALID_ARG;
+
   nsresult rv = SetAnnotationInt32Internal(aItemId, PR_TRUE, aName, aValue,
                                            aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   CallSetForItemObservers(aItemId, aName);
   return NS_OK;
 }
 
@@ -646,16 +655,19 @@ nsAnnotationService::SetPageAnnotationIn
 
 NS_IMETHODIMP
 nsAnnotationService::SetItemAnnotationInt64(PRInt64 aItemId,
                                             const nsACString& aName,
                                             PRInt64 aValue,
                                             PRInt32 aFlags,
                                             PRUint16 aExpiration)
 {
+  if (aExpiration == EXPIRE_WITH_HISTORY)
+    return NS_ERROR_INVALID_ARG;
+
   nsresult rv = SetAnnotationInt64Internal(aItemId, PR_TRUE, aName, aValue,
                                            aFlags, aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
 
   CallSetForItemObservers(aItemId, aName);
   return NS_OK;
 }
 
@@ -719,16 +731,19 @@ nsAnnotationService::SetPageAnnotationDo
 
 NS_IMETHODIMP
 nsAnnotationService::SetItemAnnotationDouble(PRInt64 aItemId,
                                              const nsACString& aName,
                                              double aValue,
                                              PRInt32 aFlags,
                                              PRUint16 aExpiration)
 {
+  if (aExpiration == EXPIRE_WITH_HISTORY)
+    return NS_ERROR_INVALID_ARG;
+
   nsresult rv = SetAnnotationDoubleInternal(aItemId, PR_TRUE, aName, aValue,
                                             aFlags, aExpiration);                      
   NS_ENSURE_SUCCESS(rv, rv);
 
   CallSetForItemObservers(aItemId, aName);
   return NS_OK;
 }
 
@@ -799,16 +814,19 @@ NS_IMETHODIMP
 nsAnnotationService::SetItemAnnotationBinary(PRInt64 aItemId,
                                              const nsACString& aName,
                                              const PRUint8 *aData,
                                              PRUint32 aDataLen,
                                              const nsACString& aMimeType,
                                              PRInt32 aFlags,
                                              PRUint16 aExpiration)
 {
+  if (aExpiration == EXPIRE_WITH_HISTORY)
+    return NS_ERROR_INVALID_ARG;
+
   nsresult rv = SetAnnotationBinaryInternal(aItemId, PR_TRUE, aName, aData,
                                             aDataLen, aMimeType, aFlags,
                                             aExpiration);
   NS_ENSURE_SUCCESS(rv, rv);
   CallSetForItemObservers(aItemId, aName);
   return NS_OK;
 }
 
--- a/toolkit/components/places/tests/unit/test_annotations.js
+++ b/toolkit/components/places/tests/unit/test_annotations.js
@@ -328,10 +328,19 @@ function run_test() {
   for each (var id in invalidIds) {
     try {
       annosvc.setItemAnnotation(id, "foo", "bar", 0, 0);
       do_throw("setItemAnnotation* should throw for invalid item id: " + id)
     }
     catch(ex) { }
   }
 
+  // setting an annotation with EXPIRE_HISTORY for an item should throw
+  var itemId = bmsvc.insertBookmark(bmsvc.bookmarksRoot, testURI, -1, "");
+  try {
+    annosvc.setItemAnnotation(itemId, "foo", "bar", 0, annosvc.EXPIRE_WITH_HISTORY);
+    do_throw("setting an item annotation with EXPIRE_HISTORY should throw");
+  }
+  catch(ex) {
+  }
+
   annosvc.removeObserver(annoObserver);
 }
--- a/toolkit/components/places/tests/unit/test_expiration.js
+++ b/toolkit/components/places/tests/unit/test_expiration.js
@@ -167,34 +167,28 @@ function run_test() {
   */
   histsvc.addVisit(testURI, Date.now(), 0, histsvc.TRANSITION_TYPED, false, 0);
   annosvc.setPageAnnotation(testURI, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_NEVER);
   annosvc.setItemAnnotation(bookmark, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_NEVER);
   histsvc.removeAllPages();
   // anno should still be there
   do_check_eq(annosvc.getPageAnnotation(testURI, testAnnoName), testAnnoVal);
   do_check_eq(annosvc.getItemAnnotation(bookmark, testAnnoName), testAnnoVal);
+  annosvc.removeItemAnnotation(bookmark, testAnnoName);
 
   /*
   test anno expiration (expire with history)
   */
   histsvc.addVisit(testURI, Date.now(), 0, histsvc.TRANSITION_TYPED, false, 0);
   annosvc.setPageAnnotation(testURI, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_WITH_HISTORY);
-  annosvc.setItemAnnotation(bookmark, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_WITH_HISTORY);
   histsvc.removeAllPages();
   try {
     annosvc.getPageAnnotation(testURI, testAnnoName);
     do_throw("page still had expire_with_history anno");
   } catch(ex) {}
-  try {
-    annosvc.getItemAnnotation(bookmark, testAnnoName);
-  } catch(ex) {
-    do_throw("bookmark lost it's expire_with_history anno when history was cleared!");
-  }
-  annosvc.removeItemAnnotation(bookmark, testAnnoName);
 
   /*
   test anno expiration (days)
     - add a bookmark, set an anno on the bookmarked uri 
     - manually tweak moz_annos.dateAdded, making it expired
     - add another entry (to kick off expiration)
     - try to get the anno (should fail. maybe race here? is there a way to determine
       if the page has been added, so we know that expiration is done?)