Backed out changeset a58a8a698e3d (bug 1059257) for XPC Test Failures
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Tue, 16 Sep 2014 14:24:33 +0200
changeset 228671 04349a090473eacae9d44e3feafbf6a5acd05e7a
parent 228670 8b581c66bdedacfe4dfd22ae573018656f3ce9ba
child 228672 b7762059815bd5ac63afe004b267bdb3f7ef099d
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1059257
milestone35.0a1
backs outa58a8a698e3d715ea883304f380b60bd81729c4e
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
Backed out changeset a58a8a698e3d (bug 1059257) for XPC Test Failures
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/tests/unit/test_async_transactions.js
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -58,17 +58,17 @@ this.EXPORTED_SYMBOLS = ["PlacesTransact
  *  - feedURI: an nsIURI object, holding the url for a live bookmark.
  *  - siteURI: an nsIURI object, holding the url for the site with which
  *             a live bookmark is associated.
  *  - GUID, parentGUID, newParentGUID: a valid places GUID string.
  *  - title: a string
  *  - index, newIndex: the position of an item in its containing folder,
  *    starting from 0.
  *    integer and PlacesUtils.bookmarks.DEFAULT_INDEX
- *  - annotation: see PlacesUtils.setAnnotationsForItem
+ *  - annotationObject: see PlacesUtils.setAnnotationsForItem
  *  - annotations: an array of annotation objects as above.
  *  - tags: an array of strings.
  *
  * Batching transactions
  * ---------------------
  * Sometimes it is useful to "batch" or "merge" transactions. For example,
  * "Bookmark All Tabs" may be implemented as one NewFolder transaction followed
  * by numerous NewBookmark transactions - all to be undone or redone in a single
@@ -583,166 +583,134 @@ DefineTransaction.isAnnotationObject = f
     if (Object.keys(obj).every( (k) => validKeys.indexOf(k) != -1 ))
       return true;
   }
   return false;
 };
 
 DefineTransaction.inputProps = new Map();
 DefineTransaction.defineInputProps =
-function (aNames, aValidationFunction,
-          aDefaultValue, aTransformFunction = null) {
+function (aNames, aValidationFunction, aDefaultValue) {
   for (let name of aNames) {
-    let propName = name;
-    this.inputProps.set(propName, {
-      validateValue: function (aValue) {
-        if (aValue === undefined)
-          return aDefaultValue;
-        if (!aValidationFunction(aValue))
-          throw new Error(`Invalid value for input property ${propName}`);
-        return aTransformFunction ? aTransformFunction(aValue) : aValue;
-      },
-
-      validateInput: function (aInput, aRequired) {
-        if (aRequired && !(propName in aInput))
-          throw new Error(`Required input property is missing: ${propName}`);
-        return this.validateValue(aInput[propName]);
-      },
-
-      isArrayProperty: false
+    this.inputProps.set(name, {
+      validate:     aValidationFunction,
+      defaultValue: aDefaultValue,
+      isGUIDProp:   false
     });
   }
 };
 
 DefineTransaction.defineArrayInputProp =
-function (aName, aBasePropertyName) {
-  let baseProp = this.inputProps.get(aBasePropertyName);
-  if (!baseProp)
-    throw new Error(`Unknown input property: ${aBasePropertyName}`);
-
+function (aName, aValidationFunction, aDefaultValue) {
   this.inputProps.set(aName, {
-    validateValue: function (aValue) {
-      if (aValue == undefined)
-        return [];
-
-      if (!Array.isArray(aValue))
-        throw new Error(`${aName} input property value must be an array`);
-
-      // This also takes care of abandoning the global scope of the input
-      // array (through Array.prototype).
-      return [for (e of aValue) baseProp.validateValue(e)];
-    },
-
-    // We allow setting either the array property itself (e.g. uris), or a
-    // single element of it (uri, in that example), that is then transformed
-    // into a single-element array.
-    validateInput: function (aInput, aRequired) {
-      if (aName in aInput) {
-        // It's not allowed to set both though.
-        if (aBasePropertyName in aInput) {
-          throw new Error(`It is not allowed to set both ${aName} and
-                          ${aBasePropertyName} as  input properties`);
-        }
-        let array = this.validateValue(aInput[aName]);
-        if (aRequired && array.length == 0) {
-          throw new Error(`Empty array passed for required input property:
-                           ${aName}`);
-        }
-        return array;
-      }
-      // If the property is required and it's not set as is, check if the base
-      // property is set.
-      if (aRequired && !(aBasePropertyName in aInput))
-        throw new Error(`Required input property is missing: ${aName}`);
-
-      if (aBasePropertyName in aInput)
-        return [baseProp.validateValue(aInput[aBasePropertyName])];
-
-      return [];
-    },
-
-    isArrayProperty: true
+    validate:     (v) => Array.isArray(v) && v.every(aValidationFunction),
+    defaultValue: aDefaultValue,
+    isGUIDProp:   false
   });
 };
 
-DefineTransaction.validatePropertyValue =
-function (aProp, aInput, aRequired) {
-  return this.inputProps.get(aProp).validateInput(aInput, aRequired);
-};
-
-DefineTransaction.getInputObjectForSingleValue =
-function (aInput, aRequiredProps, aOptionalProps) {
-  // The following input forms may be deduced from a single value:
-  // * a single required property with or without optional properties (the given
-  //   value is set to the required property).
-  // * a single optional property with no required properties.
-  if (aRequiredProps.length > 1 ||
-      (aRequiredProps.length == 0 && aOptionalProps.length > 1)) {
-    throw new Error("Transaction input isn't an object");
+DefineTransaction.verifyPropertyValue =
+function (aProp, aValue, aRequired) {
+  if (aValue === undefined) {
+    if (aRequired)
+      throw new Error("Required property is missing: " + aProp);
+    return this.inputProps.get(aProp).defaultValue;
   }
 
-  let propName = aRequiredProps.length == 1 ?
-                 aRequiredProps[0] : aOptionalProps[0];
-  let propValue =
-    this.inputProps.get(propName).isArrayProperty && !Array.isArray(aInput) ?
-    [aInput] : aInput;
-  return { [propName]: propValue };
+  if (!this.inputProps.get(aProp).validate(aValue))
+    throw new Error("Invalid value for property: " + aProp);
+
+  if (Array.isArray(aValue)) {
+    // The original array cannot be referenced by this module because it would
+    // then implicitly reference its global as well.
+    return Components.utils.cloneInto(aValue, {});
+  }
+
+  return aValue;
 };
 
 DefineTransaction.verifyInput =
-function (aInput, aRequiredProps = [], aOptionalProps = []) {
-  if (aRequiredProps.length == 0 && aOptionalProps.length == 0)
+function (aInput, aRequired = [], aOptional = []) {
+  if (aRequired.length == 0 && aOptional.length == 0)
     return {};
 
   // If there's just a single required/optional property, we allow passing it
   // as is, so, for example, one could do PlacesTransactions.RemoveItem(myGUID)
   // rather than PlacesTransactions.RemoveItem({ GUID: myGUID}).
   // This shortcut isn't supported for "complex" properties - e.g. one cannot
   // pass an annotation object this way (note there is no use case for this at
   // the moment anyway).
-  let input = aInput;
   let isSinglePropertyInput =
     this.isPrimitive(aInput) ||
-    Array.isArray(aInput) ||
     (aInput instanceof Components.interfaces.nsISupports);
-  if (isSinglePropertyInput) {
-    input =  this.getInputObjectForSingleValue(aInput,
-                                               aRequiredProps,
-                                               aOptionalProps);
+  let fixedInput = { };
+  if (aRequired.length > 0) {
+    if (isSinglePropertyInput) {
+      if (aRequired.length == 1) {
+        let prop = aRequired[0], value = aInput;
+        value = this.verifyPropertyValue(prop, value, true);
+        fixedInput[prop] = value;
+      }
+      else {
+        throw new Error("Transaction input isn't an object");
+      }
+    }
+    else {
+      for (let prop of aRequired) {
+        let value = this.verifyPropertyValue(prop, aInput[prop], true);
+        fixedInput[prop] = value;
+      }
+    }
   }
 
-  let fixedInput = { };
-  for (let prop of aRequiredProps) {
-    fixedInput[prop] = this.validatePropertyValue(prop, input, true);
-  }
-  for (let prop of aOptionalProps) {
-    fixedInput[prop] = this.validatePropertyValue(prop, input, false);
+  if (aOptional.length > 0) {
+    if (isSinglePropertyInput && !aRequired.length > 0) {
+      if (aOptional.length == 1) {
+        let prop = aOptional[0], value = aInput;
+        value = this.verifyPropertyValue(prop, value, true);
+        fixedInput[prop] = value;
+      }
+      else if (aInput !== null && aInput !== undefined) {
+        throw new Error("Transaction input isn't an object");
+      }
+    }
+    else {
+      for (let prop of aOptional) {
+        let value = this.verifyPropertyValue(prop, aInput[prop], false);
+        if (value !== undefined)
+          fixedInput[prop] = value;
+        else
+          fixedInput[prop] = this.defaultValues[prop];
+      }
+    }
   }
 
   return fixedInput;
 };
 
 // Update the documentation at the top of this module if you add or
 // remove properties.
 DefineTransaction.defineInputProps(["uri", "feedURI", "siteURI"],
                                    DefineTransaction.isURI, null);
 DefineTransaction.defineInputProps(["GUID", "parentGUID", "newParentGUID"],
                                    DefineTransaction.isGUID);
 DefineTransaction.defineInputProps(["title"],
                                    DefineTransaction.isStrOrNull, null);
-DefineTransaction.defineInputProps(["keyword", "postData", "tag"],
+DefineTransaction.defineInputProps(["keyword", "postData"],
                                    DefineTransaction.isStr, "");
 DefineTransaction.defineInputProps(["index", "newIndex"],
                                    DefineTransaction.isIndex,
                                    PlacesUtils.bookmarks.DEFAULT_INDEX);
-DefineTransaction.defineInputProps(["annotation"],
+DefineTransaction.defineInputProps(["annotationObject"],
                                    DefineTransaction.isAnnotationObject);
-DefineTransaction.defineArrayInputProp("uris", "uri");
-DefineTransaction.defineArrayInputProp("tags", "tag");
-DefineTransaction.defineArrayInputProp("annotations", "annotation")
+DefineTransaction.defineArrayInputProp("tags",
+                                       DefineTransaction.isStr, null);
+DefineTransaction.defineArrayInputProp("annotations",
+                                       DefineTransaction.isAnnotationObject,
+                                       null);
 
 /**
  * Internal helper for implementing the execute method of NewBookmark, NewFolder
  * and NewSeparator.
  *
  * @param aTransaction
  *        The transaction object
  * @param aParentGUID
@@ -915,28 +883,28 @@ PT.NewBookmark.prototype = Object.seal({
     return ExecuteCreateItem(this, aParentGUID,
       function (parentId, guidToRestore = "") {
         let itemId = PlacesUtils.bookmarks.insertBookmark(
           parentId, aURI, aIndex, aTitle, guidToRestore);
         if (aKeyword)
           PlacesUtils.bookmarks.setKeywordForBookmark(itemId, aKeyword);
         if (aPostData)
           PlacesUtils.setPostDataForBookmark(itemId, aPostData);
-        if (aAnnos.length)
+        if (aAnnos)
           PlacesUtils.setAnnotationsForItem(itemId, aAnnos);
-        if (aTags.length > 0) {
+        if (aTags && aTags.length > 0) {
           let currentTags = PlacesUtils.tagging.getTagsForURI(aURI);
           aTags = [t for (t of aTags) if (currentTags.indexOf(t) == -1)];
           PlacesUtils.tagging.tagURI(aURI, aTags);
         }
 
         return itemId;
       },
       function _additionalOnUndo() {
-        if (aTags.length > 0)
+        if (aTags && aTags.length > 0)
           PlacesUtils.tagging.untagURI(aURI, aTags);
       });
   }
 });
 
 /**
  * Transaction for creating a folder.
  *
@@ -948,17 +916,17 @@ PT.NewBookmark.prototype = Object.seal({
 PT.NewFolder = DefineTransaction(["parentGUID", "title"],
                                  ["index", "annotations"]);
 PT.NewFolder.prototype = Object.seal({
   execute: function (aParentGUID, aTitle, aIndex, aAnnos) {
     return ExecuteCreateItem(this,  aParentGUID,
       function(parentId, guidToRestore = "") {
         let itemId = PlacesUtils.bookmarks.createFolder(
           parentId, aTitle, aIndex, guidToRestore);
-        if (aAnnos.length > 0)
+        if (aAnnos)
           PlacesUtils.setAnnotationsForItem(itemId, aAnnos);
         return itemId;
       });
   }
 });
 
 /**
  * Transaction for creating a separator.
@@ -997,17 +965,17 @@ PT.NewLivemark.prototype = Object.seal({
   execute: function* (aFeedURI, aTitle, aParentGUID, aSiteURI, aIndex, aAnnos) {
     let livemarkInfo = { title: aTitle
                        , feedURI: aFeedURI
                        , siteURI: aSiteURI
                        , index: aIndex };
     let createItem = function* () {
       livemarkInfo.parentId = yield PlacesUtils.promiseItemId(aParentGUID);
       let livemark = yield PlacesUtils.livemarks.addLivemark(livemarkInfo);
-      if (aAnnos.length > 0)
+      if (aAnnos)
         PlacesUtils.setAnnotationsForItem(livemark.id, aAnnos);
 
       if ("dateAdded" in livemarkInfo) {
         PlacesUtils.bookmarks.setItemDateAdded(livemark.id,
                                                livemarkInfo.dateAdded);
         PlacesUtils.bookmarks.setItemLastModified(livemark.id,
                                                   livemarkInfo.lastModified);
       }
@@ -1114,44 +1082,40 @@ PT.EditURI.prototype = Object.seal({
 
         PlacesUtils.tagging.tagURI(oldURI, oldURITags);
       }
     };
   }
 });
 
 /**
- * Transaction for setting annotations for an item.
+ * Transaction for setting an annotation for an item.
  *
  * Required Input Properties: GUID, annotationObject
  */
-PT.Annotate = DefineTransaction(["GUID", "annotations"]);
-PT.Annotate.prototype = {
-  execute: function* (aGUID, aNewAnnos) {
-    let itemId = yield PlacesUtils.promiseItemId(aGUID);
-    let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId);
-    let undoAnnos = [];
-    for (let newAnno of aNewAnnos) {
-      let currentAnno = currentAnnos.find( a => a.name == newAnno.name );
-      if (!!currentAnno) {
-        undoAnnos.push(currentAnno);
-      }
-      else {
-        // An unset value removes the annotation.
-        undoAnnos.push({ name: newAnno.name });
-      }
+PT.SetItemAnnotation = DefineTransaction(["GUID", "annotationObject"]);
+PT.SetItemAnnotation.prototype = {
+  execute: function* (aGUID, aAnno) {
+    let itemId = yield PlacesUtils.promiseItemId(aGUID), oldAnno;
+    if (PlacesUtils.annotations.itemHasAnnotation(itemId, aAnno.name)) {
+      // Fill the old anno if it is set.
+      let flags = {}, expires = {};
+      PlacesUtils.annotations.getItemAnnotationInfo(itemId, aAnno.name, flags,
+                                                    expires, { });
+      let value = PlacesUtils.annotations.getItemAnnotation(itemId, aAnno.name);
+      oldAnno = { name: aAnno.name, flags: flags.value,
+                  value: value, expires: expires.value };
+    }
+    else {
+      // An unset value removes the annoation.
+      oldAnno = { name: aAnno.name };
     }
 
-    PlacesUtils.setAnnotationsForItem(itemId, aNewAnnos);
-    this.undo = () => {
-      PlacesUtils.setAnnotationsForItem(itemId, undoAnnos);
-    };
-    this.redo = () => {
-      PlacesUtils.setAnnotationsForItem(itemId, aNewAnnos);
-    };
+    PlacesUtils.setAnnotationsForItem(itemId, [aAnno]);
+    this.undo = () => { PlacesUtils.setAnnotationsForItem(itemId, [oldAnno]); };
   }
 };
 
 /**
  * Transaction for setting the keyword for a bookmark.
  *
  * Required Input Properties: GUID, keyword.
  */
@@ -1294,17 +1258,17 @@ PT.TagURI.prototype = {
  *
  * If |tags| is not set, all tags set for |uri| are removed.
  */
 PT.UntagURI = DefineTransaction(["uri"], ["tags"]);
 PT.UntagURI.prototype = {
   execute: function* (aURI, aTags) {
     let tagsSet = PlacesUtils.tagging.getTagsForURI(aURI);
 
-    if (aTags.length > 0)
+    if (aTags && aTags.length > 0)
       aTags = [t for (t of aTags) if (tagsSet.indexOf(t) != -1)];
     else
       aTags = tagsSet;
 
     PlacesUtils.tagging.untagURI(aURI, aTags);
     this.undo = () => { PlacesUtils.tagging.tagURI(aURI, aTags); };
     this.redo = () => { PlacesUtils.tagging.untagURI(aURI, aTags); };
   }
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -1142,111 +1142,55 @@ add_task(function* test_untag_uri() {
   yield PT.transact(PT.UntagURI(bm_info.uri));
   ensureTagsForURI(bm_info.uri, []);
   yield PT.undo();
   ensureTagsForURI(bm_info.uri, bm_info.tags);
   yield PT.redo();
   ensureTagsForURI(bm_info.uri, []);
 });
 
-add_task(function* test_annotate() {
+add_task(function* test_set_item_annotation() {
   let bm_info = { uri: NetUtil.newURI("http://test.item.annotation")
                 , parentGUID: yield PlacesUtils.promiseItemGUID(root) };
   let anno_info = { name: "TestAnno", value: "TestValue" };
   function ensureAnnoState(aSet) {
     ensureAnnotationsSet(bm_info.GUID,
                          [{ name: anno_info.name
                           , value: aSet ? anno_info.value : null }]);
   }
 
   bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info));
 
   observer.reset();
-  yield PT.transact(PT.Annotate({ GUID: bm_info.GUID, annotation: anno_info }));
+  yield PT.transact(PT.SetItemAnnotation({ GUID: bm_info.GUID
+                                         , annotationObject: anno_info }));
   ensureAnnoState(true);
 
   observer.reset();
   yield PT.undo();
   ensureAnnoState(false);
 
   observer.reset();
   yield PT.redo();
   ensureAnnoState(true);
 
   // Test removing the annotation by not passing the |value| property.
   observer.reset();
-  yield PT.transact(PT.Annotate({ GUID: bm_info.GUID,
-                                  annotation: { name: anno_info.name }}));
+  yield PT.transact(
+    PT.SetItemAnnotation({ GUID: bm_info.GUID
+                         , annotationObject: { name: anno_info.name }}));
   ensureAnnoState(false);
 
   observer.reset();
   yield PT.undo();
   ensureAnnoState(true);
 
   observer.reset();
   yield PT.redo();
   ensureAnnoState(false);
-
-  // Cleanup
-  yield PT.undo();
-  observer.reset();
-});
-
-add_task(function* test_annotate_multiple() {
-  let guid = yield PT.transact(PT.NewFolder(yield createTestFolderInfo()));
-  let itemId = yield PlacesUtils.promiseItemId(guid);
-
-  function AnnoObj(aName, aValue) {
-    this.name = aName;
-    this.value = aValue;
-    this.flags = 0;
-    this.expires = Ci.nsIAnnotationService.EXPIRE_NEVER;
-  }
-
-  function annos(a = null, b = null) [new AnnoObj("A", a), new AnnoObj("B", b)]
-
-  function verifyAnnoValues(a = null, b = null) {
-    let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId);
-    let expectedAnnos = [];
-    if (a !== null)
-      expectedAnnos.push(new AnnoObj("A", a));
-    if (b !== null)
-      expectedAnnos.push(new AnnoObj("B", b));
-
-    Assert.deepEqual(currentAnnos, expectedAnnos);
-  }
-
-  yield PT.transact(PT.Annotate({ GUID: guid, annotations: annos(1, 2) }));
-  verifyAnnoValues(1, 2);
-  yield PT.undo();
-  verifyAnnoValues();
-  yield PT.redo();
-  verifyAnnoValues(1, 2);
-
-  yield PT.transact(PT.Annotate({ GUID: guid
-                                , annotation: { name: "A" } }));
-  verifyAnnoValues(null, 2);
-
-  yield PT.transact(PT.Annotate({ GUID: guid
-                                , annotation: { name: "B", value: 0 } }));
-  verifyAnnoValues(null, 0);
-  yield PT.undo();
-  verifyAnnoValues(null, 2);
-  yield PT.redo();
-  verifyAnnoValues(null, 0);
-  yield PT.undo();
-  verifyAnnoValues(null, 2);
-  yield PT.undo();
-  verifyAnnoValues(1, 2);
-  yield PT.undo();
-  verifyAnnoValues();
-
-  // Cleanup
-  yield PT.undo();
-  observer.reset();
 });
 
 add_task(function* test_sort_folder_by_name() {
   let folder_info = yield createTestFolderInfo();
 
   let uri = NetUtil.newURI("http://sort.by.name/");
   let preSep =  [{ title: i, uri: uri } for (i of ["3","2","1"])];
   let sep = {};