Bug 1517150 - Don't generate a new QueryInterface method for every iteration in nsUpdateService.js enumerate and other fixes. r=bytesized
authorRobert Strong <robert.bugzilla@gmail.com>
Sat, 05 Jan 2019 20:36:31 -0800
changeset 509756 ad135e7ddad2fa240139653a81be39e3cc96fd0f
parent 509755 30f3633540ca3dc2bb7f6e56e2c27a2f54be639b
child 509757 27ef389fd0b9c9fc65b86d495f9fecf52e3544de
child 509758 d0a964c90dd6ed1063c19869ce571829b3cb9eb7
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbytesized
bugs1517150, 1517718
milestone66.0a1
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 1517150 - Don't generate a new QueryInterface method for every iteration in nsUpdateService.js enumerate and other fixes. r=bytesized Changes enumerate so it doesn't generate a new QueryInterface method for every iteration and uses an nsISupportsInterfacePointer. Separates the nsIUpdate and nsIUpdatePatch attributes from the nsIPropertyBag properties. Cleans up the attributes of nsIUpdate and nsIUpdatePatch Adds test for text nodes in the update xml and fixes the issue that caused bug 1517718 with the first landing
toolkit/mozapps/update/nsUpdateService.js
toolkit/mozapps/update/tests/data/sharedUpdateXML.js
toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js
--- a/toolkit/mozapps/update/nsUpdateService.js
+++ b/toolkit/mozapps/update/nsUpdateService.js
@@ -8,16 +8,26 @@
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/FileUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/UpdateTelemetry.jsm");
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 XPCOMUtils.defineLazyGlobalGetters(this, ["DOMParser", "XMLHttpRequest"]);
 
+XPCOMUtils.defineLazyModuleGetters(this, {
+  AsyncShutdown: "resource://gre/modules/AsyncShutdown.jsm",
+  CertUtils: "resource://gre/modules/CertUtils.jsm",
+  ctypes: "resource://gre/modules/ctypes.jsm",
+  DeferredTask: "resource://gre/modules/DeferredTask.jsm",
+  OS: "resource://gre/modules/osfile.jsm",
+  UpdateUtils: "resource://gre/modules/UpdateUtils.jsm",
+  WindowsRegistry: "resource://gre/modules/WindowsRegistry.jsm",
+});
+
 const UPDATESERVICE_CID = Components.ID("{B3C290A6-3943-4B89-8BBE-C01EB7B3B311}");
 
 const PREF_APP_UPDATE_ALTWINDOWTYPE        = "app.update.altwindowtype";
 const PREF_APP_UPDATE_BACKGROUNDERRORS     = "app.update.backgroundErrors";
 const PREF_APP_UPDATE_BACKGROUNDMAXERRORS  = "app.update.backgroundMaxErrors";
 const PREF_APP_UPDATE_CANCELATIONS         = "app.update.cancelations";
 const PREF_APP_UPDATE_CANCELATIONS_OSX     = "app.update.cancelations.osx";
 const PREF_APP_UPDATE_CANCELATIONS_OSX_MAX = "app.update.cancelations.osx.max";
@@ -186,26 +196,16 @@ const APPID_TO_TOPIC = {
 
 // A var is used for the delay so tests can set a smaller value.
 var gSaveUpdateXMLDelay = 2000;
 var gUpdateMutexHandle = null;
 // The permissions of the update directory should be fixed no more than once per
 // session
 var gUpdateDirPermissionFixAttempted = false;
 
-XPCOMUtils.defineLazyModuleGetters(this, {
-  AsyncShutdown: "resource://gre/modules/AsyncShutdown.jsm",
-  CertUtils: "resource://gre/modules/CertUtils.jsm",
-  ctypes: "resource://gre/modules/ctypes.jsm",
-  DeferredTask: "resource://gre/modules/DeferredTask.jsm",
-  OS: "resource://gre/modules/osfile.jsm",
-  UpdateUtils: "resource://gre/modules/UpdateUtils.jsm",
-  WindowsRegistry: "resource://gre/modules/WindowsRegistry.jsm",
-});
-
 XPCOMUtils.defineLazyGetter(this, "gLogEnabled", function aus_gLogEnabled() {
   return Services.prefs.getBoolPref(PREF_APP_UPDATE_LOG, false);
 });
 
 XPCOMUtils.defineLazyGetter(this, "gUpdateBundle", function aus_gUpdateBundle() {
   return Services.strings.createBundle(URI_UPDATES_PROPERTIES);
 });
 
@@ -1181,286 +1181,297 @@ function handleCriticalWriteResult(wrote
  * Update Patch
  * @param   patch
  *          A <patch> element to initialize this object with
  * @throws if patch has a size of 0
  * @constructor
  */
 function UpdatePatch(patch) {
   this._properties = {};
-  for (var i = 0; i < patch.attributes.length; ++i) {
+  this.errorCode = 0;
+  this.state = STATE_NONE;
+
+  for (let i = 0; i < patch.attributes.length; ++i) {
     var attr = patch.attributes.item(i);
+    // If an undefined value is saved to the xml file it will be a string when
+    // it is read from the xml file.
+    if (attr.value == "undefined") {
+      continue;
+    }
     switch (attr.name) {
       case "xmlns":
         // Don't save the XML namespace.
         break;
       case "selected":
         this.selected = attr.value == "true";
         break;
       case "size":
         if (0 == parseInt(attr.value)) {
           LOG("UpdatePatch:init - 0-sized patch!");
           throw Cr.NS_ERROR_ILLEGAL_VALUE;
         }
-        // fall through
+        this[attr.name] = attr.value;
+        break;
+      case "errorCode":
+        if (attr.value) {
+          let val = parseInt(attr.value);
+          // This will evaluate to false if the value is 0 but that's ok since
+          // this.errorCode is set to the default of 0 above.
+          if (val) {
+            this.errorCode = val;
+          }
+        }
+        break;
+      case "finalURL":
+      case "state":
       case "type":
       case "URL":
-      case "finalURL":
-      case "state":
-      case "errorCode":
         this[attr.name] = attr.value;
         break;
       default:
-        this[attr.name] = attr.value;
-        // Save custom attributes when serializing to the local xml file but
-        // don't use this method for the expected attributes which are already
-        // handled in serialize.
+        // Set nsIPropertyBag properties that were read from the xml file.
         this.setProperty(attr.name, attr.value);
         break;
     }
   }
 }
 UpdatePatch.prototype = {
   /**
    * See nsIUpdateService.idl
    */
   serialize: function UpdatePatch_serialize(updates) {
     var patch = updates.createElementNS(URI_UPDATE_NS, "patch");
+    patch.setAttribute("size", this.size);
+    patch.setAttribute("type", this.type);
+    patch.setAttribute("URL", this.URL);
     // Don't write an errorCode if it evaluates to false since 0 is the same as
     // no error code.
     if (this.errorCode) {
       patch.setAttribute("errorCode", this.errorCode);
     }
     // finalURL is not available until after the download has started
     if (this.finalURL) {
       patch.setAttribute("finalURL", this.finalURL);
     }
+    // The selected patch is the only patch that should have this attribute.
     if (this.selected) {
       patch.setAttribute("selected", this.selected);
     }
-    patch.setAttribute("size", this.size);
-    patch.setAttribute("state", this.state);
-    patch.setAttribute("type", this.type);
-    patch.setAttribute("URL", this.URL);
-
-    for (let p in this._properties) {
-      if (this._properties[p].present) {
-        patch.setAttribute(p, this._properties[p].data);
+    if (this.state != STATE_NONE) {
+      patch.setAttribute("state", this.state);
+    }
+
+    for (let [name, value] of Object.entries(this._properties)) {
+      if (value.present) {
+        patch.setAttribute(name, value.data);
       }
     }
-
     return patch;
   },
 
   /**
-   * A hash of custom properties
-   */
-  _properties: null,
-
-  /**
    * See nsIWritablePropertyBag.idl
    */
   setProperty: function UpdatePatch_setProperty(name, value) {
     this._properties[name] = { data: value, present: true };
   },
 
   /**
    * See nsIWritablePropertyBag.idl
    */
   deleteProperty: function UpdatePatch_deleteProperty(name) {
-    if (name in this._properties)
+    if (name in this._properties) {
       this._properties[name].present = false;
-    else
+    } else {
       throw Cr.NS_ERROR_FAILURE;
+    }
   },
 
   /**
    * See nsIPropertyBag.idl
    *
    * Note: this only contains the nsIPropertyBag name / value pairs and not the
    *       nsIUpdatePatch name / value pairs.
    */
   get enumerator() {
     return this.enumerate();
   },
 
   * enumerate() {
-    for (let propName in this._properties) {
-      if (this._properties[propName].present) {
+    // An nsISupportsInterfacePointer is used so creating an array using
+    // Array.from will retain the QueryInterface for nsIProperty.
+    let ip = Cc["@mozilla.org/supports-interface-pointer;1"].
+             createInstance(Ci.nsISupportsInterfacePointer);
+    let qi = ChromeUtils.generateQI([Ci.nsIProperty]);
+    for (let [name, value] of Object.entries(this._properties)) {
+      if (value.present) {
         // The nsIPropertyBag enumerator returns a nsISimpleEnumerator whose
-        // elements are nsIProperty objects.
-        yield { name: propName,
-                value: this._properties[propName].data,
-                QueryInterface: ChromeUtils.generateQI([Ci.nsIProperty])};
+        // elements are nsIProperty objects. Calling QueryInterface for
+        // nsIProperty on the object doesn't return to the caller an object that
+        // is already queried to nsIProperty but do it just in case it is fixed
+        // at some point.
+        ip.data = {name, value: value.data, QueryInterface: qi};
+        yield ip.data.QueryInterface(Ci.nsIProperty);
       }
     }
   },
 
   /**
    * See nsIPropertyBag.idl
+   *
    * Note: returns null instead of throwing when the property doesn't exist to
    *       simplify code and to silence warnings in debug builds.
    */
   getProperty: function UpdatePatch_getProperty(name) {
-    if (name in this._properties &&
-        this._properties[name].present) {
+    if (name in this._properties && this._properties[name].present) {
       return this._properties[name].data;
     }
     return null;
   },
 
-  /**
-   * See nsIUpdateService.idl
-   */
-  get errorCode() {
-    return this._properties.errorCode || 0;
-  },
-  set errorCode(val) {
-    this._properties.errorCode = val;
-  },
-
-  /**
-   * See nsIUpdateService.idl
-   */
-  get state() {
-    return this._properties.state || STATE_NONE;
-  },
-  set state(val) {
-    this._properties.state = val;
-  },
-
   QueryInterface: ChromeUtils.generateQI([Ci.nsIUpdatePatch,
                                           Ci.nsIPropertyBag,
                                           Ci.nsIWritablePropertyBag]),
 };
 
 /**
  * Update
  * Implements nsIUpdate
  * @param   update
  *          An <update> element to initialize this object with
  * @throws if the update contains no patches
  * @constructor
  */
 function Update(update) {
+  this._patches = [];
   this._properties = {};
-  this._patches = [];
   this.isCompleteUpdate = false;
-  this.unsupported = false;
   this.channel = "default";
   this.promptWaitTime = Services.prefs.getIntPref(PREF_APP_UPDATE_PROMPTWAITTIME, 43200);
+  this.unsupported = false;
 
   // Null <update>, assume this is a message container and do no
   // further initialization
   if (!update) {
     return;
   }
 
-  let patch;
   for (let i = 0; i < update.childNodes.length; ++i) {
     let patchElement = update.childNodes.item(i);
     if (patchElement.nodeType != patchElement.ELEMENT_NODE ||
         patchElement.localName != "patch") {
       continue;
     }
 
+    let patch;
     try {
       patch = new UpdatePatch(patchElement);
     } catch (e) {
       continue;
     }
     this._patches.push(patch);
   }
 
   if (this._patches.length == 0 && !update.hasAttribute("unsupported")) {
     throw Cr.NS_ERROR_ILLEGAL_VALUE;
   }
 
   // Set the installDate value with the current time. If the update has an
   // installDate attribute this will be replaced with that value if it doesn't
   // equal 0.
   this.installDate = (new Date()).getTime();
+  this.patchCount = this._patches.length;
 
   for (let i = 0; i < update.attributes.length; ++i) {
-    var attr = update.attributes.item(i);
+    let attr = update.attributes.item(i);
     if (attr.name == "xmlns" || attr.value == "undefined") {
       // Don't save the XML namespace or undefined values.
+      // If an undefined value is saved to the xml file it will be a string when
+      // it is read from the xml file.
       continue;
     } else if (attr.name == "detailsURL") {
-      this._detailsURL = attr.value;
+      this.detailsURL = attr.value;
     } else if (attr.name == "installDate" && attr.value) {
       let val = parseInt(attr.value);
       if (val) {
         this.installDate = val;
       }
     } else if (attr.name == "errorCode" && attr.value) {
       let val = parseInt(attr.value);
       if (val) {
-        this.errorCode = val;
+        // Set the value of |_errorCode| instead of |errorCode| since
+        // selectedPatch won't be available at this point and normally the
+        // nsIUpdatePatch will provide the errorCode.
+        this._errorCode = val;
       }
     } else if (attr.name == "isCompleteUpdate") {
       this.isCompleteUpdate = attr.value == "true";
     } else if (attr.name == "promptWaitTime") {
       if (!isNaN(attr.value)) {
         this.promptWaitTime = parseInt(attr.value);
       }
     } else if (attr.name == "unsupported") {
       this.unsupported = attr.value == "true";
     } else {
-      this[attr.name] = attr.value;
-
       switch (attr.name) {
         case "appVersion":
         case "buildID":
         case "channel":
         case "displayVersion":
+        case "elevationFailure":
         case "name":
         case "previousAppVersion":
         case "serviceURL":
         case "statusText":
         case "type":
+          this[attr.name] = attr.value;
           break;
         default:
-          // Save custom attributes when serializing to the local xml file but
-          // don't use this method for the expected attributes which are already
-          // handled in serialize.
+          // Set nsIPropertyBag properties that were read from the xml file.
           this.setProperty(attr.name, attr.value);
           break;
       }
     }
   }
 
+  if (!this.previousAppVersion) {
+    this.previousAppVersion = Services.appinfo.version;
+  }
+
+  if (!this.elevationFailure) {
+    this.elevationFailure = false;
+  }
+
+  if (!this.detailsURL) {
+    try {
+      // Try using a default details URL supplied by the distribution
+      // if the update XML does not supply one.
+      this.detailsURL = Services.urlFormatter.formatURLPref(PREF_APP_UPDATE_URL_DETAILS);
+    } catch (e) {
+      this.detailsURL = "";
+    }
+  }
+
   if (!this.displayVersion) {
     this.displayVersion = this.appVersion;
   }
 
-  // The Update Name is either the string provided by the <update> element, or
-  // the string: "<App Name> <Update App Version>"
-  var name = "";
-  if (update.hasAttribute("name")) {
-    name = update.getAttribute("name");
-  } else {
-    var brandBundle = Services.strings.createBundle(URI_BRAND_PROPERTIES);
-    var appName = brandBundle.GetStringFromName("brandShortName");
-    name = gUpdateBundle.formatStringFromName("updateName",
-                                              [appName, this.displayVersion], 2);
+  if (!this.name) {
+    // When the update doesn't provide a name fallback to using
+    // "<App Name> <Update App Version>"
+    let brandBundle = Services.strings.createBundle(URI_BRAND_PROPERTIES);
+    let appName = brandBundle.GetStringFromName("brandShortName");
+    this.name = gUpdateBundle.formatStringFromName("updateName",
+                                                   [appName, this.displayVersion], 2);
   }
-  this.name = name;
 }
 Update.prototype = {
   /**
    * See nsIUpdateService.idl
    */
-  get patchCount() {
-    return this._patches.length;
-  },
-
-  /**
-   * See nsIUpdateService.idl
-   */
   getPatchAt: function Update_getPatchAt(index) {
     return this._patches[index];
   },
 
   /**
    * See nsIUpdateService.idl
    *
    * We use a copy of the state cached on this object in |_state| only when
@@ -1499,126 +1510,114 @@ Update.prototype = {
       this.selectedPatch.errorCode = errorCode;
     this._errorCode = errorCode;
   },
 
   /**
    * See nsIUpdateService.idl
    */
   get selectedPatch() {
-    for (var i = 0; i < this.patchCount; ++i) {
-      if (this._patches[i].selected)
+    for (let i = 0; i < this.patchCount; ++i) {
+      if (this._patches[i].selected) {
         return this._patches[i];
+      }
     }
     return null;
   },
 
   /**
    * See nsIUpdateService.idl
    */
-  get detailsURL() {
-    if (!this._detailsURL) {
-      try {
-        // Try using a default details URL supplied by the distribution
-        // if the update XML does not supply one.
-        return Services.urlFormatter.formatURLPref(PREF_APP_UPDATE_URL_DETAILS);
-      } catch (e) {
-      }
-    }
-    return this._detailsURL || "";
-  },
-
-  /**
-   * See nsIUpdateService.idl
-   */
   serialize: function Update_serialize(updates) {
     // If appVersion isn't defined just return null. This happens when cleaning
     // up invalid updates (e.g. incorrect channel).
     if (!this.appVersion) {
       return null;
     }
-    var update = updates.createElementNS(URI_UPDATE_NS, "update");
+    let update = updates.createElementNS(URI_UPDATE_NS, "update");
     update.setAttribute("appVersion", this.appVersion);
     update.setAttribute("buildID", this.buildID);
     update.setAttribute("channel", this.channel);
+    update.setAttribute("detailsURL", this.detailsURL);
     update.setAttribute("displayVersion", this.displayVersion);
     update.setAttribute("installDate", this.installDate);
     update.setAttribute("isCompleteUpdate", this.isCompleteUpdate);
     update.setAttribute("name", this.name);
+    update.setAttribute("previousAppVersion", this.previousAppVersion);
     update.setAttribute("promptWaitTime", this.promptWaitTime);
     update.setAttribute("serviceURL", this.serviceURL);
     update.setAttribute("type", this.type);
 
-    if (this.detailsURL) {
-      update.setAttribute("detailsURL", this.detailsURL);
-    }
-    if (this.previousAppVersion) {
-      update.setAttribute("previousAppVersion", this.previousAppVersion);
-    }
     if (this.statusText) {
       update.setAttribute("statusText", this.statusText);
     }
     if (this.unsupported) {
       update.setAttribute("unsupported", this.unsupported);
     }
-    updates.documentElement.appendChild(update);
-
-    for (let p in this._properties) {
-      if (this._properties[p].present) {
-        update.setAttribute(p, this._properties[p].data);
+    if (this.elevationFailure) {
+      update.setAttribute("elevationFailure", this.elevationFailure);
+    }
+
+    for (let [name, value] of Object.entries(this._properties)) {
+      if (value.present) {
+        update.setAttribute(name, value.data);
       }
     }
 
     for (let i = 0; i < this.patchCount; ++i) {
       update.appendChild(this.getPatchAt(i).serialize(updates));
     }
 
+    updates.documentElement.appendChild(update);
     return update;
   },
 
   /**
-   * A hash of custom properties
-   */
-  _properties: null,
-
-  /**
    * See nsIWritablePropertyBag.idl
    */
   setProperty: function Update_setProperty(name, value) {
     this._properties[name] = { data: value, present: true };
   },
 
   /**
    * See nsIWritablePropertyBag.idl
    */
   deleteProperty: function Update_deleteProperty(name) {
-    if (name in this._properties)
+    if (name in this._properties) {
       this._properties[name].present = false;
-    else
+    } else {
       throw Cr.NS_ERROR_FAILURE;
+    }
   },
 
   /**
    * See nsIPropertyBag.idl
    *
    * Note: this only contains the nsIPropertyBag name value / pairs and not the
    *       nsIUpdate name / value pairs.
    */
   get enumerator() {
     return this.enumerate();
   },
 
   * enumerate() {
-    for (let propName in this._properties) {
-      if (this._properties[propName].present) {
+    // An nsISupportsInterfacePointer is used so creating an array using
+    // Array.from will retain the QueryInterface for nsIProperty.
+    let ip = Cc["@mozilla.org/supports-interface-pointer;1"].
+             createInstance(Ci.nsISupportsInterfacePointer);
+    let qi = ChromeUtils.generateQI([Ci.nsIProperty]);
+    for (let [name, value] of Object.entries(this._properties)) {
+      if (value.present) {
         // The nsIPropertyBag enumerator returns a nsISimpleEnumerator whose
-        // elements are nsIProperty objects.
-        yield { name: propName,
-                value: this._properties[propName].data,
-                QueryInterface: ChromeUtils.generateQI([Ci.nsIProperty])};
+        // elements are nsIProperty objects. Calling QueryInterface for
+        // nsIProperty on the object doesn't return to the caller an object that
+        // is already queried to nsIProperty but do it just in case it is fixed
+        // at some point.
+        ip.data = {name, value: value.data, QueryInterface: qi};
+        yield ip.data.QueryInterface(Ci.nsIProperty);
       }
     }
   },
 
   /**
    * See nsIPropertyBag.idl
    * Note: returns null instead of throwing when the property doesn't exist to
    *       simplify code and to silence warnings in debug builds.
@@ -1633,18 +1632,19 @@ Update.prototype = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsIUpdate,
                                           Ci.nsIPropertyBag,
                                           Ci.nsIWritablePropertyBag]),
 };
 
 const UpdateServiceFactory = {
   _instance: null,
   createInstance(outer, iid) {
-    if (outer != null)
+    if (outer != null) {
       throw Cr.NS_ERROR_NO_AGGREGATION;
+    }
     return this._instance == null ? this._instance = new UpdateService() :
                                     this._instance;
   },
 };
 
 /**
  * UpdateService
  * A Service for managing the discovery and installation of software updates.
@@ -2577,18 +2577,16 @@ UpdateService.prototype = {
       if (update.isCompleteUpdate == this._downloader.isCompleteUpdate &&
           background == this._downloader.background) {
         LOG("UpdateService:downloadUpdate - no support for downloading more " +
             "than one update at a time");
         return readStatusFile(getUpdatesDir());
       }
       this._downloader.cancel();
     }
-    // Set the previous application version prior to downloading the update.
-    update.previousAppVersion = Services.appinfo.version;
     this._downloader = new Downloader(background, this);
     return this._downloader.downloadUpdate(update);
   },
 
   /**
    * See nsIUpdateService.idl
    */
   pauseDownload: function AUS_pauseDownload() {
--- a/toolkit/mozapps/update/tests/data/sharedUpdateXML.js
+++ b/toolkit/mozapps/update/tests/data/sharedUpdateXML.js
@@ -127,19 +127,19 @@ function getRemoteUpdateString(aUpdatePr
     promptWaitTime: null,
     type: "major",
   };
 
   for (let name in aUpdateProps) {
     updateProps[name] = aUpdateProps[name];
   }
 
-  return getUpdateString(updateProps) + ">" +
+  return getUpdateString(updateProps) + ">\n " +
          aPatches +
-         "</update>";
+         "\n</update>";
 }
 
 /**
  * Constructs a string representing a patch element for a remote update xml
  * file. See getPatchString for parameter information not provided below.
  *
  * @param  aPatchProps (optional)
  *         An object containing non default test values for an nsIUpdatePatch.
--- a/toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js
+++ b/toolkit/mozapps/update/tests/unit_aus_update/updateManagerXML.js
@@ -103,40 +103,37 @@ function run_test() {
                "the update previousAppVersion attribute" + MSG_SHOULD_EQUAL);
   // Custom attributes
   Assert.equal(update.getProperty("custom1_attr"), "custom1 value",
                "the update custom1_attr property" + MSG_SHOULD_EQUAL);
   Assert.equal(update.getProperty("custom2_attr"), "custom2 value",
                "the update custom2_attr property" + MSG_SHOULD_EQUAL);
   // nsIPropertyBag enumerator
   debugDump("checking the first update enumerator");
-  let e = update.enumerator;
-  Assert.ok(e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
-  let prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom1_attr",
+  Assert.ok(update.enumerator instanceof Ci.nsISimpleEnumerator,
+            "update enumerator should be an instance of nsISimpleEnumerator");
+  let results = Array.from(update.enumerator);
+  Assert.equal(results.length, 3,
+               "the length of the array created from the update enumerator" +
+               MSG_SHOULD_EQUAL);
+  Assert.ok(results.every(prop => prop instanceof Ci.nsIProperty),
+            "the objects in the array created from the update enumerator " +
+            "should all be an instance of nsIProperty");
+  Assert.equal(results[0].name, "custom1_attr",
                "the first property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom1 value",
+  Assert.equal(results[0].value, "custom1 value",
                "the first property value" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom2_attr",
+  Assert.equal(results[1].name, "custom2_attr",
                "the second property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom2 value",
+  Assert.equal(results[1].value, "custom2 value",
                "the second property value" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.equal(prop.name, "foregroundDownload",
-               "the third property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "true",
+  Assert.equal(results[2].name, "foregroundDownload",
+               "the second property name" + MSG_SHOULD_EQUAL);
+  Assert.equal(results[2].value, "true",
                "the third property value" + MSG_SHOULD_EQUAL);
-  Assert.ok(!e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
 
   debugDump("checking the first update patch properties");
   let patch = update.selectedPatch.QueryInterface(Ci.nsIWritablePropertyBag);
   Assert.equal(patch.type, "partial",
                "the update patch type attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.URL, "http://partial/",
                "the update patch URL attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.size, "86",
@@ -146,35 +143,33 @@ function run_test() {
   Assert.equal(patch.state, STATE_SUCCEEDED,
                "the update patch state attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.getProperty("custom1_attr"), "custom1 patch value",
                "the update patch custom1_attr property" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.getProperty("custom2_attr"), "custom2 patch value",
                "the update patch custom2_attr property" + MSG_SHOULD_EQUAL);
   // nsIPropertyBag enumerator
   debugDump("checking the first update patch enumerator");
-  e = patch.enumerator;
-  Assert.ok(e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom1_attr",
+  Assert.ok(patch.enumerator instanceof Ci.nsISimpleEnumerator,
+            "patch enumerator should be an instance of nsISimpleEnumerator");
+  results = Array.from(patch.enumerator);
+  Assert.equal(results.length, 2,
+               "the length of the array created from the patch enumerator" +
+               MSG_SHOULD_EQUAL);
+  Assert.ok(results.every(prop => prop instanceof Ci.nsIProperty),
+            "the objects in the array created from the patch enumerator " +
+            "should all be an instance of nsIProperty");
+  Assert.equal(results[0].name, "custom1_attr",
                "the first property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom1 patch value",
+  Assert.equal(results[0].value, "custom1 patch value",
                "the first property value" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom2_attr",
+  Assert.equal(results[1].name, "custom2_attr",
                "the second property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom2 patch value",
+  Assert.equal(results[1].value, "custom2 patch value",
                "the second property value" + MSG_SHOULD_EQUAL);
-  Assert.ok(!e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
 
   debugDump("checking the second update properties");
   update = gUpdateManager.getUpdateAt(1).QueryInterface(Ci.nsIWritablePropertyBag);
   Assert.equal(update.state, STATE_FAILED,
                "the update state attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(update.name, "Existing",
                "the update name attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(update.type, "minor",
@@ -194,49 +189,46 @@ function run_test() {
   Assert.equal(update.buildID, "20080811053724",
                "the update buildID attribute" + MSG_SHOULD_EQUAL);
   Assert.ok(!!update.isCompleteUpdate,
             "the update isCompleteUpdate attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(update.channel, "test_channel",
                "the update channel attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(update.promptWaitTime, "691200",
                "the update promptWaitTime attribute" + MSG_SHOULD_EQUAL);
-  Assert.equal(update.previousAppVersion, null,
+  Assert.equal(update.previousAppVersion, "1.0",
                "the update previousAppVersion attribute" + MSG_SHOULD_EQUAL);
   // Custom attributes
   Assert.equal(update.getProperty("custom3_attr"), "custom3 value",
                "the update custom3_attr property" + MSG_SHOULD_EQUAL);
   Assert.equal(update.getProperty("custom4_attr"), "custom4 value",
                "the update custom4_attr property" + MSG_SHOULD_EQUAL);
   // nsIPropertyBag enumerator
   debugDump("checking the second update enumerator");
-  e = update.enumerator;
-  Assert.ok(e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom3_attr",
+  Assert.ok(update.enumerator instanceof Ci.nsISimpleEnumerator,
+            "update enumerator should be an instance of nsISimpleEnumerator");
+  results = Array.from(update.enumerator);
+  Assert.equal(results.length, 3,
+               "the length of the array created from the update enumerator" +
+               MSG_SHOULD_EQUAL);
+  Assert.ok(results.every(prop => prop instanceof Ci.nsIProperty),
+            "the objects in the array created from the update enumerator " +
+            "should all be an instance of nsIProperty");
+  Assert.equal(results[0].name, "custom3_attr",
                "the first property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom3 value",
+  Assert.equal(results[0].value, "custom3 value",
                "the first property value" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom4_attr",
+  Assert.equal(results[1].name, "custom4_attr",
                "the second property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom4 value",
+  Assert.equal(results[1].value, "custom4 value",
                "the second property value" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.equal(prop.name, "foregroundDownload",
+  Assert.equal(results[2].name, "foregroundDownload",
                "the third property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "false",
+  Assert.equal(results[2].value, "false",
                "the third property value" + MSG_SHOULD_EQUAL);
-  Assert.ok(!e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
 
   debugDump("checking the second update patch properties");
   patch = update.selectedPatch.QueryInterface(Ci.nsIWritablePropertyBag);
   Assert.equal(patch.type, "complete",
                "the update patch type attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.URL, "http://complete/",
                "the update patch URL attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.size, "75",
@@ -246,35 +238,33 @@ function run_test() {
   Assert.equal(patch.state, STATE_FAILED,
                "the update patch state attribute" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.getProperty("custom3_attr"), "custom3 patch value",
                "the update patch custom3_attr property" + MSG_SHOULD_EQUAL);
   Assert.equal(patch.getProperty("custom4_attr"), "custom4 patch value",
                "the update patch custom4_attr property" + MSG_SHOULD_EQUAL);
   // nsIPropertyBag enumerator
   debugDump("checking the second update patch enumerator");
-  e = patch.enumerator;
-  Assert.ok(e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom3_attr",
+  Assert.ok(patch.enumerator instanceof Ci.nsISimpleEnumerator,
+            "patch enumerator should be an instance of nsISimpleEnumerator");
+  results = Array.from(patch.enumerator);
+  Assert.equal(results.length, 2,
+               "the length of the array created from the patch enumerator" +
+               MSG_SHOULD_EQUAL);
+  Assert.ok(results.every(prop => prop instanceof Ci.nsIProperty),
+            "the objects in the array created from the patch enumerator " +
+            "should all be an instance of nsIProperty");
+  Assert.equal(results[0].name, "custom3_attr",
                "the first property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom3 patch value",
+  Assert.equal(results[0].value, "custom3 patch value",
                "the first property value" + MSG_SHOULD_EQUAL);
-  prop = e.getNext().QueryInterface(Ci.nsIProperty);
-  Assert.ok(!!prop,
-            "the enumerator.getNext()" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.name, "custom4_attr",
+  Assert.equal(results[1].name, "custom4_attr",
                "the second property name" + MSG_SHOULD_EQUAL);
-  Assert.equal(prop.value, "custom4 patch value",
+  Assert.equal(results[1].value, "custom4 patch value",
                "the second property value" + MSG_SHOULD_EQUAL);
-  Assert.ok(!e.hasMoreElements(),
-            "the enumerator.hasMoreElements()" + MSG_SHOULD_EQUAL);
 
   // Cleaning up the active update along with reloading the update manager
   // in doTestFinish will prevent writing the update xml files during
   // shutdown.
   gUpdateManager.cleanupActiveUpdate();
   executeSoon(waitForUpdateXMLFiles);
 }