Merge inbound to mozilla-central. a=merge
authorCiure Andrei <aciure@mozilla.com>
Sun, 06 Jan 2019 11:49:41 +0200
changeset 509757 27ef389fd0b9
parent 509751 9aa94abacb92 (current diff)
parent 509756 ad135e7ddad2 (diff)
child 509759 10040ee53d5b
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)
reviewersmerge
milestone66.0a1
first release with
nightly linux32
27ef389fd0b9 / 66.0a1 / 20190106095006 / files
nightly linux64
27ef389fd0b9 / 66.0a1 / 20190106095006 / files
nightly mac
27ef389fd0b9 / 66.0a1 / 20190106095006 / files
nightly win32
27ef389fd0b9 / 66.0a1 / 20190106095006 / files
nightly win64
27ef389fd0b9 / 66.0a1 / 20190106095006 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Merge inbound to mozilla-central. a=merge
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -128,17 +128,16 @@ function Inspector(toolbox) {
   this.nodeMenuTriggerInfo = null;
 
   this._clearSearchResultsLabel = this._clearSearchResultsLabel.bind(this);
   this._handleRejectionIfNotDestroyed = this._handleRejectionIfNotDestroyed.bind(this);
   this._onBeforeNavigate = this._onBeforeNavigate.bind(this);
   this._onContextMenu = this._onContextMenu.bind(this);
   this._onMarkupFrameLoad = this._onMarkupFrameLoad.bind(this);
   this._updateSearchResultsLabel = this._updateSearchResultsLabel.bind(this);
-  this._updateDebuggerPausedWarning = this._updateDebuggerPausedWarning.bind(this);
 
   this.onDetached = this.onDetached.bind(this);
   this.onHostChanged = this.onHostChanged.bind(this);
   this.onMarkupLoaded = this.onMarkupLoaded.bind(this);
   this.onNewSelection = this.onNewSelection.bind(this);
   this.onNewRoot = this.onNewRoot.bind(this);
   this.onPanelWindowResize = this.onPanelWindowResize.bind(this);
   this.onShowBoxModelHighlighterForNode =
@@ -314,23 +313,16 @@ Inspector.prototype = {
 
     this.onNewSelection();
 
     this.walker.on("new-root", this.onNewRoot);
     this.toolbox.on("host-changed", this.onHostChanged);
     this.selection.on("new-node-front", this.onNewSelection);
     this.selection.on("detached-front", this.onDetached);
 
-    if (this.target.isLocalTab) {
-      this.target.on("thread-paused", this._updateDebuggerPausedWarning);
-      this.target.on("thread-resumed", this._updateDebuggerPausedWarning);
-      this.toolbox.on("select", this._updateDebuggerPausedWarning);
-      this._updateDebuggerPausedWarning();
-    }
-
     // Log the 3 pane inspector setting on inspector open. The question we want to answer
     // is:
     // "What proportion of users use the 3 pane vs 2 pane inspector on inspector open?"
     this.telemetry.keyedScalarAdd(THREE_PANE_ENABLED_SCALAR, this.is3PaneModeEnabled, 1);
 
     this.emit("ready");
     return this;
   },
@@ -478,45 +470,16 @@ Inspector.prototype = {
       } else {
         str = INSPECTOR_L10N.getStr("inspector.searchResultsNone");
       }
     }
 
     this.searchResultsLabel.textContent = str;
   },
 
-  /**
-   * Show a warning notification box when the debugger is paused. We show the warning only
-   * when the inspector is selected.
-   */
-  _updateDebuggerPausedWarning: function() {
-    if (!this.toolbox.threadClient.paused && !this._notificationBox) {
-      return;
-    }
-
-    const notificationBox = this.notificationBox;
-    const notification = this.notificationBox.getNotificationWithValue(
-      "inspector-script-paused");
-
-    if (!notification && this.toolbox.currentToolId == "inspector" &&
-        this.toolbox.threadClient.paused) {
-      const message = INSPECTOR_L10N.getStr("debuggerPausedWarning.message");
-      notificationBox.appendNotification(message,
-        "inspector-script-paused", "", notificationBox.PRIORITY_WARNING_HIGH);
-    }
-
-    if (notification && this.toolbox.currentToolId != "inspector") {
-      notificationBox.removeNotification(notification);
-    }
-
-    if (notification && !this.toolbox.threadClient.paused) {
-      notificationBox.removeNotification(notification);
-    }
-  },
-
   get React() {
     return this._toolbox.React;
   },
 
   get ReactDOM() {
     return this._toolbox.ReactDOM;
   },
 
@@ -1429,19 +1392,16 @@ Inspector.prototype = {
     this.panelWin.removeEventListener("resize", this.onPanelWindowResize, true);
     this.selection.off("new-node-front", this.onNewSelection);
     this.selection.off("detached-front", this.onDetached);
     this.sidebar.off("select", this.onSidebarSelect);
     this.sidebar.off("show", this.onSidebarShown);
     this.sidebar.off("hide", this.onSidebarHidden);
     this.sidebar.off("destroy", this.onSidebarHidden);
     this.target.off("will-navigate", this._onBeforeNavigate);
-    this.target.off("thread-paused", this._updateDebuggerPausedWarning);
-    this.target.off("thread-resumed", this._updateDebuggerPausedWarning);
-    this._toolbox.off("select", this._updateDebuggerPausedWarning);
 
     for (const [, panel] of this._panels) {
       panel.destroy();
     }
     this._panels.clear();
 
     if (this._highlighters) {
       this._highlighters.destroy();
--- a/devtools/client/inspector/test/browser.ini
+++ b/devtools/client/inspector/test/browser.ini
@@ -68,16 +68,17 @@ skip-if = os == "mac" # Full keyboard na
 [browser_inspector_destroy-before-ready.js]
 [browser_inspector_expand-collapse.js]
 [browser_inspector_highlighter-01.js]
 [browser_inspector_highlighter-02.js]
 [browser_inspector_highlighter-03.js]
 [browser_inspector_highlighter-04.js]
 [browser_inspector_highlighter-05.js]
 [browser_inspector_highlighter-06.js]
+[browser_inspector_highlighter-07.js]
 [browser_inspector_highlighter-by-type.js]
 [browser_inspector_highlighter-cancel.js]
 [browser_inspector_highlighter-comments.js]
 [browser_inspector_highlighter-cssgrid_01.js]
 [browser_inspector_highlighter-cssgrid_02.js]
 [browser_inspector_highlighter-cssshape_01.js]
 [browser_inspector_highlighter-cssshape_02.js]
 [browser_inspector_highlighter-cssshape_03.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-07.js
@@ -0,0 +1,106 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+// Test that the highlighter works when the debugger is paused.
+
+function debuggerIsPaused(dbg) {
+  const {
+    selectors: { isPaused },
+    getState,
+  } = dbg;
+  return !!isPaused(getState());
+}
+
+function waitForPaused(dbg) {
+  return new Promise(resolve => {
+    if (debuggerIsPaused(dbg)) {
+      resolve();
+      return;
+    }
+
+    const unsubscribe = dbg.store.subscribe(() => {
+      if (debuggerIsPaused(dbg)) {
+        unsubscribe();
+        resolve();
+      }
+    });
+  });
+}
+
+function createDebuggerContext(toolbox) {
+  const panel = toolbox.getPanel("jsdebugger");
+  const win = panel.panelWin;
+  const { store, client, selectors, actions } = panel.getVarsForTests();
+
+  return {
+    actions: actions,
+    selectors: selectors,
+    getState: store.getState,
+    store: store,
+    client: client,
+    toolbox: toolbox,
+    win: win,
+    panel: panel,
+  };
+}
+
+const IFRAME_SRC = "<style>" +
+    "body {" +
+      "margin:0;" +
+      "height:100%;" +
+      "background-color:red" +
+    "}" +
+  "</style><body>hello from iframe</body>";
+
+const DOCUMENT_SRC = "<style>" +
+    "iframe {" +
+      "height:200px;" +
+      "border: 11px solid black;" +
+      "padding: 13px;" +
+    "}" +
+    "body,iframe {" +
+      "margin:0" +
+    "}" +
+  "</style>" +
+  "<body>" +
+   "<script>setInterval('debugger', 100)</script>" +
+   "<iframe src='data:text/html;charset=utf-8," + IFRAME_SRC + "'></iframe>" +
+  "</body>";
+
+const TEST_URI = "data:text/html;charset=utf-8," + DOCUMENT_SRC;
+
+add_task(async function() {
+  const { inspector, toolbox, testActor } = await openInspectorForURL(TEST_URI);
+
+  const target = await TargetFactory.forTab(gBrowser.selectedTab);
+  await gDevTools.showToolbox(target, "jsdebugger");
+  const dbg = createDebuggerContext(toolbox);
+
+  await waitForPaused(dbg);
+
+  await gDevTools.showToolbox(target, "inspector");
+
+  // Needed to get this test to pass consistently :(
+  await waitForTime(1000);
+
+  info("Waiting for box mode to show.");
+  const body = await getNodeFront("body", inspector);
+  await inspector.highlighter.showBoxModel(body);
+
+  info("Waiting for element picker to become active.");
+  await startPicker(toolbox);
+
+  info("Moving mouse over iframe padding.");
+  await moveMouseOver("iframe", 1, 1);
+
+  info("Performing checks");
+  await testActor.isNodeCorrectlyHighlighted("iframe", is);
+
+  function moveMouseOver(selector, x, y) {
+    info("Waiting for element " + selector + " to be highlighted");
+    testActor.synthesizeMouse({selector, x, y, options: {type: "mousemove"}});
+    return inspector.inspector.nodePicker.once("picker-node-hovered");
+  }
+});
--- a/devtools/client/locales/en-US/inspector.properties
+++ b/devtools/client/locales/en-US/inspector.properties
@@ -8,20 +8,16 @@
 # The correct localization of this file might be to keep it in
 # English, or another language commonly spoken among web developers.
 # You want to make that choice consistent across the developer tools.
 # A good criteria is the language in which you'd find the best
 # documentation on web development on the web.
 
 breadcrumbs.siblings=Siblings
 
-# LOCALIZATION NOTE (debuggerPausedWarning): Used in the Inspector tool, when
-# the user switch to the inspector when the debugger is paused.
-debuggerPausedWarning.message=Debugger is paused. Some features like mouse selection will not work.
-
 # LOCALIZATION NOTE (nodeMenu.tooltiptext)
 # This menu appears in the Infobar (on top of the highlighted node) once
 # the node is selected.
 nodeMenu.tooltiptext=Node operations
 
 inspector.panelLabel.markupView=Markup View
 
 # LOCALIZATION NOTE (markupView.more.showing)
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -376,41 +376,72 @@ exports.HighlighterActor = protocol.Acto
   _findAndAttachElement: function(event) {
     // originalTarget allows access to the "real" element before any retargeting
     // is applied, such as in the case of XBL anonymous elements.  See also
     // https://developer.mozilla.org/docs/XBL/XBL_1.0_Reference/Anonymous_Content#Event_Flow_and_Targeting
     const node = event.originalTarget || event.target;
     return this._walker.attachElement(node);
   },
 
+  _onSuppressedEvent(event) {
+    if (event.type == "mousemove") {
+      this._onHovered(event);
+    } else if (event.type == "mouseup") {
+      // Suppressed mousedown/mouseup events will be sent to us before they have
+      // been converted into click events. Just treat any mouseup as a click.
+      this._onPick(event);
+    }
+  },
+
+  /**
+   * When the debugger pauses execution in a page, events will not be delivered
+   * to any handlers added to elements on that page. This method uses the
+   * document's setSuppressedEventListener interface to bypass this restriction:
+   * events will be delivered to the callback at times when they would
+   * otherwise be suppressed. The set of events delivered this way is currently
+   * limited to mouse events.
+   *
+   * @param callback The function to call with suppressed events, or null.
+   */
+  _setSuppressedEventListener(callback) {
+    const document = this._targetActor.window.document;
+
+    // Pass the callback to setSuppressedEventListener as an EventListener.
+    document.setSuppressedEventListener(callback ? { handleEvent: callback } : null);
+  },
+
   _startPickerListeners: function() {
     const target = this._highlighterEnv.pageListenerTarget;
     target.addEventListener("mousemove", this._onHovered, true);
     target.addEventListener("click", this._onPick, true);
     target.addEventListener("mousedown", this._preventContentEvent, true);
     target.addEventListener("mouseup", this._preventContentEvent, true);
     target.addEventListener("dblclick", this._preventContentEvent, true);
     target.addEventListener("keydown", this._onKey, true);
     target.addEventListener("keyup", this._preventContentEvent, true);
+
+    this._setSuppressedEventListener(this._onSuppressedEvent.bind(this));
   },
 
   _stopPickerListeners: function() {
     const target = this._highlighterEnv.pageListenerTarget;
 
     if (!target) {
       return;
     }
 
     target.removeEventListener("mousemove", this._onHovered, true);
     target.removeEventListener("click", this._onPick, true);
     target.removeEventListener("mousedown", this._preventContentEvent, true);
     target.removeEventListener("mouseup", this._preventContentEvent, true);
     target.removeEventListener("dblclick", this._preventContentEvent, true);
     target.removeEventListener("keydown", this._onKey, true);
     target.removeEventListener("keyup", this._preventContentEvent, true);
+
+    this._setSuppressedEventListener(null);
   },
 
   _highlighterReady: function() {
     this._inspector.walker.emit("highlighter-ready");
   },
 
   _highlighterHidden: function() {
     this._inspector.walker.emit("highlighter-hide");
--- a/devtools/server/actors/highlighters/eye-dropper.js
+++ b/devtools/server/actors/highlighters/eye-dropper.js
@@ -159,16 +159,20 @@ EyeDropper.prototype = {
     this.magnifiedArea = {width: MAGNIFIER_WIDTH, height: MAGNIFIER_HEIGHT,
                           x: DEFAULT_START_POS_X, y: DEFAULT_START_POS_Y};
 
     this.moveTo(DEFAULT_START_POS_X, DEFAULT_START_POS_Y);
 
     // Focus the content so the keyboard can be used.
     this.win.focus();
 
+    // Make sure we receive mouse events when the debugger has paused execution
+    // in the page.
+    this.win.document.setSuppressedEventListener(this);
+
     return true;
   },
 
   /**
    * Hide the eye-dropper highlighter.
    */
   hide() {
     if (this.highlighterEnv.isXUL) {
@@ -186,16 +190,18 @@ EyeDropper.prototype = {
       pageListenerTarget.removeEventListener("DOMMouseScroll", this);
       pageListenerTarget.removeEventListener("FullZoomChange", this);
     }
 
     this.getElement("root").setAttribute("hidden", "true");
     this.getElement("root").removeAttribute("drawn");
 
     this.emit("hidden");
+
+    this.win.document.setSuppressedEventListener(null);
   },
 
   prepareImageCapture() {
     // Get the image data from the content window.
     const imageData = getWindowAsImageData(this.win);
 
     // We need to transform imageData to something drawWindow will consume. An ImageBitmap
     // works well. We could have used an Image, but doing so results in errors if the page
@@ -325,17 +331,20 @@ EyeDropper.prototype = {
         // Update the zoom area.
         this.magnifiedArea.x = x * this.pageZoom;
         this.magnifiedArea.y = y * this.pageZoom;
         // Redraw the portion of the screenshot that is now under the mouse.
         this.draw();
         // And move the eye-dropper's UI so it follows the mouse.
         this.moveTo(x, y);
         break;
+      // Note: when events are suppressed we will only get mousedown/mouseup and
+      // not any click events.
       case "click":
+      case "mouseup":
         this.selectColor();
         break;
       case "keydown":
         this.handleKeyDown(e);
         break;
       case "DOMMouseScroll":
         // Prevent scrolling. That's because we only took a screenshot of the viewport, so
         // scrolling out of the viewport wouldn't draw the expected things. In the future
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -1741,16 +1741,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLinks);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mForms);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScripts);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mApplets);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnchors);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnonymousContents)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCommandDispatcher)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFeaturePolicy)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSuppressedEventListener)
 
   // Traverse all our nsCOMArrays.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStyleSheets)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPreloadingImages)
 
   for (uint32_t i = 0; i < tmp->mFrameRequestCallbacks.Length(); ++i) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mFrameRequestCallbacks[i]");
     cb.NoteXPCOMChild(tmp->mFrameRequestCallbacks[i].mCallback);
@@ -1832,16 +1833,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Do
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mApplets);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mAnchors);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mOrientationPendingPromise)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mFontFaceSet)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mReadyForIdle);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mCommandDispatcher)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocumentL10n);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mFeaturePolicy)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mSuppressedEventListener)
 
   tmp->mParentDocument = nullptr;
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPreloadingImages)
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntersectionObservers)
 
   tmp->ClearAllBoxObjects();
@@ -8487,16 +8489,27 @@ void Document::UnsuppressEventHandlingAn
 }
 
 void Document::AddSuspendedChannelEventQueue(net::ChannelEventQueue* aQueue) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(EventHandlingSuppressed());
   mSuspendedQueues.AppendElement(aQueue);
 }
 
+static bool SetSuppressedEventListenerInSubDocument(Document* aDocument,
+                                                    void* aData) {
+  aDocument->SetSuppressedEventListener(static_cast<EventListener*>(aData));
+  return true;
+}
+
+void Document::SetSuppressedEventListener(EventListener* aListener) {
+  mSuppressedEventListener = aListener;
+  EnumerateSubDocuments(SetSuppressedEventListenerInSubDocument, aListener);
+}
+
 nsISupports* Document::GetCurrentContentSink() {
   return mParser ? mParser->GetContentSink() : nullptr;
 }
 
 Document* Document::GetTemplateContentsOwner() {
   if (!mTemplateContentsOwner) {
     bool hasHadScriptObject = true;
     nsIScriptGlobalObject* scriptObject =
--- a/dom/base/Document.h
+++ b/dom/base/Document.h
@@ -2441,16 +2441,22 @@ class Document : public nsINode,
   bool IsVisible() const { return mVisible; }
 
   /**
    * Return whether the document and all its ancestors are visible in the sense
    * of pageshow / hide.
    */
   bool IsVisibleConsideringAncestors() const;
 
+  void SetSuppressedEventListener(EventListener* aListener);
+
+  EventListener* GetSuppressedEventListener() {
+    return mSuppressedEventListener;
+  }
+
   /**
    * Return true when this document is active, i.e., an active document
    * in a content viewer.  Note that this will return true for bfcached
    * documents, so this does NOT match the "active document" concept in
    * the WHATWG spec - see IsCurrentActiveDocument.
    */
   bool IsActive() const { return mDocumentContainer && !mRemovedFromDocShell; }
 
@@ -4132,16 +4138,18 @@ class Document : public nsINode,
   RefPtr<Document> mDisplayDocument;
 
   uint32_t mEventsSuppressed;
 
   // Any XHR ChannelEventQueues that were suspended on this document while
   // events were suppressed.
   nsTArray<RefPtr<mozilla::net::ChannelEventQueue>> mSuspendedQueues;
 
+  RefPtr<EventListener> mSuppressedEventListener;
+
   /**
    * https://html.spec.whatwg.org/#ignore-destructive-writes-counter
    */
   uint32_t mIgnoreDestructiveWritesCounter;
 
   /**
    * The current frame request callback handle
    */
--- a/dom/webidl/Document.webidl
+++ b/dom/webidl/Document.webidl
@@ -500,16 +500,24 @@ partial interface Document {
 
 // Extension to give chrome JS the ability to simulate activate the docuement
 // by user gesture.
 partial interface Document {
   [ChromeOnly]
   void notifyUserGestureActivation();
 };
 
+// Extension to give chrome JS the ability to set an event handler which is
+// called with certain events that happened while events were suppressed in the
+// document or one of its subdocuments.
+partial interface Document {
+  [ChromeOnly]
+  void setSuppressedEventListener(EventListener? aListener);
+};
+
 // Extension to give chrome and XBL JS the ability to determine whether
 // the document is sandboxed without permission to run scripts
 // and whether inline scripts are blocked by the document's CSP.
 partial interface Document {
   [Func="IsChromeOrXBL"] readonly attribute boolean hasScriptsBlockedBySandbox;
   [Func="IsChromeOrXBL"] readonly attribute boolean inlineScriptAllowedByCSP;
 };
 
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -6780,16 +6780,38 @@ nsresult PresShell::HandleEvent(nsIFrame
       } else if (!mNoDelayedMouseEvents &&
                  (aEvent->mMessage == eMouseUp ||
                   // contextmenu is triggered after right mouseup on Windows and
                   // right mousedown on other platforms.
                   aEvent->mMessage == eContextMenu)) {
         auto event = MakeUnique<DelayedMouseEvent>(aEvent->AsMouseEvent());
         mDelayedEvents.AppendElement(std::move(event));
       }
+
+      // If there is a suppressed event listener associated with the document,
+      // notify it about the suppressed mouse event. This allows devtools
+      // features to continue receiving mouse events even when the devtools
+      // debugger has paused execution in a page.
+      RefPtr<EventListener> suppressedListener =
+        frame->PresContext()->Document()->GetSuppressedEventListener();
+      if (suppressedListener &&
+          aEvent->AsMouseEvent()->mReason != WidgetMouseEvent::eSynthesized) {
+        nsCOMPtr<nsIContent> targetContent;
+        frame->GetContentForEvent(aEvent, getter_AddRefs(targetContent));
+        if (targetContent) {
+          aEvent->mTarget = targetContent;
+        }
+
+        nsCOMPtr<EventTarget> et = aEvent->mTarget;
+        RefPtr<Event> event = EventDispatcher::CreateEvent(
+                et, frame->PresContext(), aEvent, EmptyString());
+
+        suppressedListener->HandleEvent(*event);
+      }
+
       return NS_OK;
     }
 
     if (!frame) {
       NS_WARNING("Nothing to handle this event!");
       return NS_OK;
     }
 
--- 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);
 }