Merge mozilla-central to inbound. a=merge CLOSED TREE

Cargo.lock

@@ -910,7 +910,7 @@ dependencies = [
  "rkv 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rsdparsa_capi 0.1.0",
  "rustc_version 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "u2fhid 0.2.0",
+ "u2fhid 0.2.1",
  "webrender_bindings 0.1.0",
  "xpcom 0.1.0",
 ]
@@ -2480,7 +2480,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "u2fhid"
-version = "0.2.0"
+version = "0.2.1"
 dependencies = [
  "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "boxfnonce 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2492,7 +2492,7 @@ dependencies = [
  "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)",
  "runloop 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "winapi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]

browser/app/profile/firefox.js

@@ -59,7 +59,7 @@ pref("extensions.autoDisableScopes", 15);
 // Scopes to scan for changes at startup.
 pref("extensions.startupScanScopes", 0);
 
-pref("extensions.geckoProfiler.acceptedExtensionIds", "geckoprofiler@mozilla.com,quantum-foxfooding@mozilla.com");
+pref("extensions.geckoProfiler.acceptedExtensionIds", "geckoprofiler@mozilla.com,quantum-foxfooding@mozilla.com,raptor@mozilla.org");
 #if defined(XP_LINUX) || defined (XP_MACOSX)
 pref("extensions.geckoProfiler.getSymbolRules", "localBreakpad,nm");
 #else // defined(XP_WIN)

browser/app/winlauncher/LauncherProcessWin.cpp

@@ -137,9 +137,7 @@ IsSameBinaryAsParentProcess()
     return mozilla::Nothing();
   }
 
-  bool isSame = parentExeLen == ourExeOk &&
-                !_wcsnicmp(ourExe, parentExe, ourExeOk);
-  return mozilla::Some(isSame);
+  return mozilla::DoPathsPointToIdenticalFile(parentExe, ourExe);
 }
 
 #endif // defined(MOZ_LAUNCHER_PROCESS)

browser/base/content/test/siteIdentity/browser.ini

@@ -108,3 +108,4 @@ support-files =
 [browser_iframe_navigation.js]
 support-files =
   iframe_navigation.html
+[browser_tls_handshake_failure.js]

browser/base/content/test/siteIdentity/browser_iframe_navigation.js

@@ -7,10 +7,10 @@
 // security state after navigating an iframe in various contexts.
 // See bug 1490982.
 
-const SECURE_TEST_URI =
-  "https://example.com/browser/browser/base/content/test/siteIdentity/iframe_navigation.html";
-const INSECURE_TEST_URI =
-  "http://example.com/browser/browser/base/content/test/siteIdentity/iframe_navigation.html";
+const ROOT_URI = getRootDirectory(gTestPath).replace("chrome://mochitests/content",
+                                                     "https://example.com");
+const SECURE_TEST_URI = ROOT_URI + "iframe_navigation.html";
+const INSECURE_TEST_URI = SECURE_TEST_URI.replace("https://", "http://");
 
 // From a secure URI, navigate the iframe to about:blank (should still be
 // secure).

browser/base/content/test/siteIdentity/browser_tls_handshake_failure.js

@@ -0,0 +1,25 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests that the site identity indicator is properly updated for connections
+// where the TLS handshake fails.
+// See bug 1492424.
+
+add_task(async function() {
+  let rootURI = getRootDirectory(gTestPath).replace("chrome://mochitests/content",
+                                                    "https://example.com");
+  await BrowserTestUtils.withNewTab(rootURI + "dummy_page.html", async (browser) => {
+    let identityMode = window.document.getElementById("identity-box").className;
+    is(identityMode, "verifiedDomain", "identity should be secure before");
+
+    const TLS_HANDSHAKE_FAILURE_URI = "https://ssl3.example.com/";
+    // Try to connect to a server where the TLS handshake will fail.
+    BrowserTestUtils.loadURI(browser, TLS_HANDSHAKE_FAILURE_URI);
+    await BrowserTestUtils.browserLoaded(browser, false, TLS_HANDSHAKE_FAILURE_URI, true);
+
+    let newIdentityMode = window.document.getElementById("identity-box").className;
+    is(newIdentityMode, "unknownIdentity", "identity should be unknown (not secure) after");
+  });
+});

browser/base/content/urlbarBindings.xml

@@ -1774,7 +1774,8 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
             // Only wait for a result when we are sure to get one.  In some
             // cases, like when pasting the same exact text, we may not fire
             // a new search and we won't get a result.
-            if (this.mController.handleText()) {
+            this._onInputHandledText = this.mController.handleText();
+            if (this._onInputHandledText) {
               this.gotResultForCurrentQuery = false;
               this._searchStartDate = Cu.now();
               this._deferredKeyEventQueue = [];
@@ -1931,11 +1932,6 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
         <parameter name="value"/>
         <parameter name="options"/>
         <body><![CDATA[
-          this.focus();
-          this.textValue = value;
-          // Avoid selecting the url bar text if `search` is called twice in a row
-          this.selectionStart = -1;
-
           options = options || {};
 
           if (options.disableOneOffButtons) {
@@ -1958,8 +1954,31 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
             }, {once: true});
           }
 
-          this.gotResultForCurrentQuery = false;
-          this.controller.startSearch(value);
+          // We want the value to be treated as text that the user typed.  It
+          // should go through the controller.handleText() path in onInput() so
+          // that gBrowser.userTypedValue, this.valueIsTyped, etc. are set and
+          // nsAutoCompleteController::HandleText() is called.  Set this.value
+          // and fire an input event to do that.  (If we set this.textValue we'd
+          // get an input event for free, but it would also set mIgnoreInput,
+          // skipping all of the above requirements.)
+          this.focus();
+          this.value = value;
+
+          // Avoid selecting the text if this method is called twice in a row.
+          this.selectionStart = -1;
+
+          let event = document.createEvent("Events");
+          event.initEvent("input", true, true);
+          this.dispatchEvent(event);
+
+          // handleText() ignores the value if it's the same as the previous
+          // value, but we want consecutive searches with the same value to be
+          // possible.  If handleText() returned false, then manually start a
+          // new search here.
+          if (!this._onInputHandledText) {
+            this.gotResultForCurrentQuery = false;
+            this.controller.startSearch(value);
+          }
         ]]></body>
       </method>
     </implementation>

browser/components/extensions/test/mochitest/test_ext_all_apis.html

@@ -13,7 +13,6 @@
 "use strict";
 /* exported expectedContentApisTargetSpecific, expectedBackgroundApisTargetSpecific */
 let expectedContentApisTargetSpecific = [
-  "userScripts.setScriptAPIs",
 ];
 
 let expectedBackgroundApisTargetSpecific = [

browser/components/newtab/lib/ActivityStream.jsm

@@ -41,6 +41,7 @@ const DEFAULT_SITES = new Map([
 ]);
 const GEO_PREF = "browser.search.region";
 const SPOCS_GEOS = ["US"];
+const IS_NIGHTLY_OR_UNBRANDED_BUILD = ["nightly", "default"].includes(AppConstants.MOZ_UPDATE_CHANNEL);
 
 const ONE_HOUR_IN_MS = 60 * 60 * 1000;
 
@@ -222,8 +223,8 @@ const PREFS_CONFIG = new Map([
       id: "cfr",
       type: "local",
       localProvider: "CFRMessageProvider",
-      enabled: AppConstants.MOZ_UPDATE_CHANNEL !== "release",
-      cohort: 0,
+      enabled: IS_NIGHTLY_OR_UNBRANDED_BUILD,
+      cohort: IS_NIGHTLY_OR_UNBRANDED_BUILD ? "nightly" : "",
     }]),
   }],
 ]);

browser/components/newtab/lib/CFRMessageProvider.jsm

@@ -82,6 +82,7 @@ const CFR_MESSAGES = [
     },
     frequency: {lifetime: 1},
     targeting: `
+      (providerCohorts.cfr in ["one_per_day_amazon", "three_per_day_amazon"]) &&
       (${JSON.stringify(AMAZON_ASSISTANT_PARAMS.existing_addons)} intersect addonsInfo.addons|keys)|length == 0 &&
       (${JSON.stringify(AMAZON_ASSISTANT_PARAMS.open_urls)} intersect topFrecentSites[.frecency >= ${AMAZON_ASSISTANT_PARAMS.min_frecency}]|mapToProperty('host'))|length > 0`,
     trigger: {id: "openURL", params: AMAZON_ASSISTANT_PARAMS.open_urls},
@@ -122,6 +123,7 @@ const CFR_MESSAGES = [
     },
     frequency: {lifetime: 1},
     targeting: `
+      (providerCohorts.cfr in ["one_per_day", "three_per_day", "nightly"]) &&
       (${JSON.stringify(FACEBOOK_CONTAINER_PARAMS.existing_addons)} intersect addonsInfo.addons|keys)|length == 0 &&
       (${JSON.stringify(FACEBOOK_CONTAINER_PARAMS.open_urls)} intersect topFrecentSites[.frecency >= ${FACEBOOK_CONTAINER_PARAMS.min_frecency}]|mapToProperty('host'))|length > 0`,
     trigger: {id: "openURL", params: FACEBOOK_CONTAINER_PARAMS.open_urls},
@@ -162,6 +164,7 @@ const CFR_MESSAGES = [
     },
     frequency: {lifetime: 1},
     targeting: `
+      (providerCohorts.cfr in ["one_per_day", "three_per_day", "nightly"]) &&
       (${JSON.stringify(GOOGLE_TRANSLATE_PARAMS.existing_addons)} intersect addonsInfo.addons|keys)|length == 0 &&
       (${JSON.stringify(GOOGLE_TRANSLATE_PARAMS.open_urls)} intersect topFrecentSites[.frecency >= ${GOOGLE_TRANSLATE_PARAMS.min_frecency}]|mapToProperty('host'))|length > 0`,
     trigger: {id: "openURL", params: GOOGLE_TRANSLATE_PARAMS.open_urls},
@@ -202,6 +205,7 @@ const CFR_MESSAGES = [
     },
     frequency: {lifetime: 1},
     targeting: `
+      (providerCohorts.cfr in ["one_per_day", "three_per_day", "nightly"]) &&
       (${JSON.stringify(YOUTUBE_ENHANCE_PARAMS.existing_addons)} intersect addonsInfo.addons|keys)|length == 0 &&
       (${JSON.stringify(YOUTUBE_ENHANCE_PARAMS.open_urls)} intersect topFrecentSites[.frecency >= ${YOUTUBE_ENHANCE_PARAMS.min_frecency}]|mapToProperty('host'))|length > 0`,
     trigger: {id: "openURL", params: YOUTUBE_ENHANCE_PARAMS.open_urls},
@@ -242,6 +246,7 @@ const CFR_MESSAGES = [
     },
     frequency: {lifetime: 1},
     targeting: `
+      (providerCohorts.cfr in ["one_per_day", "three_per_day", "nightly"]) &&
       (${JSON.stringify(WIKIPEDIA_CONTEXT_MENU_SEARCH_PARAMS.existing_addons)} intersect addonsInfo.addons|keys)|length == 0 &&
       (${JSON.stringify(WIKIPEDIA_CONTEXT_MENU_SEARCH_PARAMS.open_urls)} intersect topFrecentSites[.frecency >= ${WIKIPEDIA_CONTEXT_MENU_SEARCH_PARAMS.min_frecency}]|mapToProperty('host'))|length > 0`,
     trigger: {id: "openURL", params: WIKIPEDIA_CONTEXT_MENU_SEARCH_PARAMS.open_urls},
@@ -282,6 +287,7 @@ const CFR_MESSAGES = [
     },
     frequency: {lifetime: 1},
     targeting: `
+      (providerCohorts.cfr in ["one_per_day", "three_per_day", "nightly"]) &&
       (${JSON.stringify(REDDIT_ENHANCEMENT_PARAMS.existing_addons)} intersect addonsInfo.addons|keys)|length == 0 &&
       (${JSON.stringify(REDDIT_ENHANCEMENT_PARAMS.open_urls)} intersect topFrecentSites[.frecency >= ${REDDIT_ENHANCEMENT_PARAMS.min_frecency}]|mapToProperty('host'))|length > 0`,
     trigger: {id: "openURL", params: REDDIT_ENHANCEMENT_PARAMS.open_urls},

browser/components/payments/res/components/address-option.css

@@ -2,67 +2,28 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-address-option {
+address-option.rich-option {
   grid-row-gap: 5px;
-  grid-column-gap: 10px;
-  grid-template-areas:
-    "name          "
-    "street-address";
 }
 
-rich-select[open] > .rich-select-popup-box > address-option {
-  grid-template-areas:
-    "name           name          "
-    "street-address street-address"
-    "email          tel           ";
-}
-
-address-picker.payer-related > rich-select address-option {
-  grid-template-areas:
-    "name name"
-    "tel email";
-}
-
-address-option > .name {
-  grid-area: name;
-}
-
-address-option > .street-address {
-  grid-area: street-address;
-}
-
-address-option > .email {
-  grid-area: email;
-}
-
-address-option > .tel {
-  grid-area: tel;
-}
-
-address-option > .name,
-address-option > .street-address,
-address-option > .email,
-address-option > .tel {
+address-option > .line {
+  overflow: hidden;
+  text-overflow: ellipsis;
   white-space: nowrap;
 }
 
-address-picker.shipping-related address-option > .email,
-address-picker.shipping-related address-option.rich-select-selected-option > .tel {
+address-option > .line:empty {
+  /* Hide the 2nd line in cases where it's empty
+     (e.g. payer field with one or two fields requested) */
   display: none;
 }
 
-/* for payer contact details:
- * display fields selectively based on the contents of the address-fields attribute
- */
-address-picker.payer-related address-option > .name,
-address-picker.payer-related address-option > .street-address,
-address-picker.payer-related address-option > .email,
-address-picker.payer-related address-option > .tel {
-  display: none;
+address-option > .line > span {
+  white-space: nowrap;
 }
 
-address-picker[address-fields~='name'].payer-related address-option  > .name,
-address-picker[address-fields~='email'].payer-related address-option  > .email,
-address-picker[address-fields~='tel'].payer-related address-option  > .tel {
-  display: inline-block;
+address-option > .line > span:empty::before {
+  /* Show the string for missing fields in grey when the field is empty */
+  color: GrayText;
+  content: attr(data-missing-string);
 }

browser/components/payments/res/components/address-option.js

@@ -2,11 +2,15 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/* import-globals-from ../../../../../browser/extensions/formautofill/content/autofillEditForms.js*/
 import ObservedPropertiesMixin from "../mixins/ObservedPropertiesMixin.js";
 import RichOption from "./rich-option.js";
 /* import-globals-from ../unprivileged-fallbacks.js */
 
 /**
+ * Up to two-line address display. After bug 1475684 this will also be used for
+ * the single-line <option> substitute too.
+ *
  * <rich-select>
  *  <address-option guid="98hgvnbmytfc"
  *                  address-level1="MI"
@@ -31,6 +35,7 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
       "email",
       "guid",
       "name",
+      "organization",
       "postal-code",
       "street-address",
       "tel",
@@ -38,22 +43,34 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
   }
 
   static get observedAttributes() {
-    return RichOption.observedAttributes.concat(AddressOption.recordAttributes);
+    return RichOption.observedAttributes.concat(AddressOption.recordAttributes,
+                                                "address-fields",
+                                                "break-after-nth-field",
+                                                "data-field-separator");
   }
 
   constructor() {
     super();
 
-    for (let name of ["name", "street-address", "email", "tel"]) {
+    this._line1 = document.createElement("div");
+    this._line1.classList.add("line");
+    this._line2 = document.createElement("div");
+    this._line2.classList.add("line");
+
+    for (let name of AddressOption.recordAttributes) {
       this[`_${name}`] = document.createElement("span");
       this[`_${name}`].classList.add(name);
+      // XXX Bug 1490816: Use appropriate strings
+      let missingValueString = name.replace(/(-|^)([a-z])/g, ($0, $1, $2) => {
+        return $1.replace("-", " ") + $2.toUpperCase();
+      }) + " Missing";
+      this[`_${name}`].dataset.missingString = missingValueString;
     }
   }
 
   connectedCallback() {
-    for (let name of ["name", "street-address", "email", "tel"]) {
-      this.appendChild(this[`_${name}`]);
-    }
+    this.appendChild(this._line1);
+    this.appendChild(this._line2);
     super.connectedCallback();
   }
 
@@ -61,14 +78,72 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
     return PaymentDialogUtils.getAddressLabel(address, addressFields);
   }
 
+  get requiredFields() {
+    if (this.hasAttribute("address-fields")) {
+      let names = this.getAttribute("address-fields").trim().split(/\s+/);
+      if (names.length) {
+        return names;
+      }
+    }
+
+    return [
+      // "address-level1", // TODO: bug 1481481 - not required for some countries e.g. DE
+      "address-level2",
+      "country",
+      "name",
+      "postal-code",
+      "street-address",
+    ];
+  }
+
   render() {
+    // Clear the lines of the fields so we can append only the ones still
+    // visible in the correct order below.
+    this._line1.textContent = "";
+    this._line2.textContent = "";
+
+    // Fill the fields with their text/strings.
     // Fall back to empty strings to prevent 'null' from appearing.
-    this._name.textContent = this.name || "";
-    this["_street-address"].textContent =
-      `${this.streetAddress || ""} ${this.addressLevel2 || ""} ` +
-      `${this.addressLevel1 || ""} ${this.postalCode || ""} ${this.country || ""}`;
-    this._email.textContent = this.email || "";
-    this._tel.textContent = this.tel || "";
+    for (let name of AddressOption.recordAttributes) {
+      let camelCaseName = super.constructor.kebabToCamelCase(name);
+      let fieldEl = this[`_${name}`];
+      fieldEl.textContent = this[camelCaseName] || "";
+    }
+
+    let {fieldsOrder} = PaymentDialogUtils.getFormFormat(this.country);
+    // A subset of the requested fields may be returned if the fields don't apply to the country.
+    let requestedVisibleFields = this.addressFields || "mailing-address";
+    let visibleFields = EditAddress.computeVisibleFields(fieldsOrder, requestedVisibleFields);
+    let visibleFieldCount = 0;
+    let requiredFields = this.requiredFields;
+    // Start by populating line 1
+    let lineEl = this._line1;
+    // Which field number to start line 2 after.
+    let breakAfterNthField = this.breakAfterNthField || 2;
+
+    // Now actually place the fields in the proper place on the lines.
+    for (let field of visibleFields) {
+      let fieldEl = this[`_${field.fieldId}`];
+      if (!fieldEl) {
+        log.warn(`address-option render: '${field.fieldId}' doesn't exist`);
+        continue;
+      }
+
+      if (!fieldEl.textContent && !requiredFields.includes(field.fieldId)) {
+        // The field is empty and we don't need to show "Missing …" so don't append.
+        continue;
+      }
+
+      if (lineEl.children.length > 0) {
+        lineEl.append(this.dataset.fieldSeparator);
+      }
+      lineEl.appendChild(fieldEl);
+
+      // Add a break after this field, if requested.
+      if (++visibleFieldCount == breakAfterNthField) {
+        lineEl = this._line2;
+      }
+    }
   }
 }
 

browser/components/payments/res/components/basic-card-option.css

@@ -3,13 +3,26 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 basic-card-option {
-  grid-column-gap: 1ch;
-  grid-template-areas: "type cc-number cc-exp cc-name";
+  grid-column-gap: 1em;
+  grid-template-areas: "cc-type cc-number cc-exp cc-name";
+  /* Need to set a minimum width for the cc-type svg in the <img> to fill */
+  grid-template-columns: minmax(1em, auto);
   justify-content: start;
 }
 
+basic-card-option > .cc-number,
+basic-card-option > .cc-name,
+basic-card-option > .cc-exp,
+basic-card-option > .cc-type {
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
 basic-card-option > .cc-number {
   grid-area: cc-number;
+  /* Don't truncate the card number */
+  overflow: visible;
 }
 
 basic-card-option > .cc-name {
@@ -20,14 +33,8 @@ basic-card-option > .cc-exp {
   grid-area: cc-exp;
 }
 
-basic-card-option > .type {
-  grid-area: type;
-  display: none;
-}
-
-basic-card-option > .cc-number,
-basic-card-option > .cc-name,
-basic-card-option > .cc-exp,
-basic-card-option > .type {
-  white-space: nowrap;
+basic-card-option > .cc-type {
+  grid-area: cc-type;
+  height: 100%;
+  text-transform: capitalize;
 }

browser/components/payments/res/components/basic-card-option.js

@@ -30,7 +30,7 @@ export default class BasicCardOption extends ObservedPropertiesMixin(RichOption)
     super();
 
     for (let name of ["cc-name", "cc-number", "cc-exp", "cc-type"]) {
-      this[`_${name}`] = document.createElement("span");
+      this[`_${name}`] = document.createElement(name == "cc-type" ? "img" : "span");
       this[`_${name}`].classList.add(name);
     }
   }
@@ -42,21 +42,42 @@ export default class BasicCardOption extends ObservedPropertiesMixin(RichOption)
     super.connectedCallback();
   }
 
+  static formatCCNumber(ccNumber) {
+    // XXX: Bug 1470175 - This should probably be unified with CreditCard.jsm logic.
+    return ccNumber ? ccNumber.replace(/[*]{4,}/, "****") : "";
+  }
+
   static formatSingleLineLabel(basicCard) {
-    // Fall back to empty strings to prevent 'undefined' from appearing.
-    let ccNumber = basicCard["cc-number"] || "";
-    let ccExp = basicCard["cc-exp"] || "";
-    let ccName = basicCard["cc-name"] || "";
+    let ccNumber = BasicCardOption.formatCCNumber(basicCard["cc-number"]);
+
+    // XXX Bug 1473772 - Hard-coded string
+    let ccExp = basicCard["cc-exp"] ? "Exp. " + basicCard["cc-exp"] : "";
+    let ccName = basicCard["cc-name"];
     // XXX: Bug 1491040, displaying cc-type in this context may need its own localized string
     let ccType = basicCard["cc-type"] || "";
-    return `${ccType} ${ccNumber} ${ccExp} ${ccName}`;
+    // Filter out empty/undefined tokens before joining by three spaces
+    // (  in the middle of two normal spaces to avoid them visually collapsing in HTML)
+    return [
+      ccType.replace(/^[a-z]/, $0 => $0.toUpperCase()),
+      ccNumber,
+      ccExp,
+      ccName,
+      // XXX Bug 1473772 - Hard-coded string:
+    ].filter(str => !!str).join(" \xa0 ");
+  }
+
+  get requiredFields() {
+    return BasicCardOption.recordAttributes;
   }
 
   render() {
-    this["_cc-name"].textContent = this.ccName;
-    this["_cc-number"].textContent = this.ccNumber;
-    this["_cc-exp"].textContent = this.ccExp;
-    this["_cc-type"].textContent = this.ccType;
+    this["_cc-name"].textContent = this.ccName || "";
+    this["_cc-number"].textContent = BasicCardOption.formatCCNumber(this.ccNumber);
+    // XXX Bug 1473772 - Hard-coded string:
+    this["_cc-exp"].textContent = this.ccExp ? "Exp. " + this.ccExp : "";
+    // XXX: Bug 1491040, displaying cc-type in this context may need its own localized string
+    this["_cc-type"].alt = this.ccType || "";
+    this["_cc-type"].src = "chrome://formautofill/content/icon-credit-card-generic.svg";
   }
 }
 

browser/components/payments/res/components/rich-select.css

@@ -3,16 +3,23 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 rich-select {
-  border: 1px solid #0C0C0D33;
+  /* Include the padding in the max-width calculation so that we truncate rather
+     than grow wider than 100% of the parent. */
+  box-sizing: border-box;
   display: block;
   margin: 14px 0;
+  /* Padding for the dropmarker (copied from common.css) */
+  padding-inline-end: 24px;
   position: relative;
+  /* Don't allow the <rich-select> to grow wider than the container so that we
+     truncate with text-overflow for long options instead. */
+  max-width: 100%;
 }
 
 /* Focusing on the underlying select element outlines the outer
-rich-select wrapper making it appear like rich-select is focused. */
-rich-select:focus-within {
-  outline: 1px dotted;
+   rich-select wrapper making it appear like rich-select is focused. */
+rich-select:focus-within > select {
+  outline: 1px dotted var(--in-content-text-color);
 }
 
 /*
@@ -20,7 +27,9 @@ rich-select:focus-within {
  * element to make it look like clicking on the rich-option element
  * in the closed state opens the HTML select dropdown. */
 rich-select > select {
-  opacity: 0;
+  /* Hide the text from the closed state so that the text/layout from
+     <rich-option> won't overlap it. */
+  color: transparent;
   position: absolute;
   top: 0;
   left: 0;
@@ -29,8 +38,20 @@ rich-select > select {
   margin: 0;
 }
 
+rich-select > select > option {
+  /* Reset the text color in the popup/open state */
+  color: var(--in-content-text-color);
+}
+
 .rich-option {
   display: grid;
-  background: #fff; /* TODO: system colors */
   padding: 8px;
 }
+
+.rich-select-selected-option {
+  /* Clicks on the selected rich option should go to the <select> below to open the popup */
+  pointer-events: none;
+  /* Use position:relative so this is positioned on top of the <select> which
+     also has position:relative. */
+  position: relative;
+}

browser/components/payments/res/components/rich-select.js

@@ -36,6 +36,12 @@ export default class RichSelect extends ObservedPropertiesMixin(HTMLElement) {
     return this.getOptionByValue(this.value);
   }
 
+  get selectedRichOption() {
+    // XXX: Bug 1475684 - This can be removed once `selectedOption` returns a
+    // RichOption which extends HTMLOptionElement.
+    return this.querySelector(":scope > .rich-select-selected-option");
+  }
+
   get value() {
     return this.popupBox.value;
   }
@@ -73,7 +79,7 @@ export default class RichSelect extends ObservedPropertiesMixin(HTMLElement) {
       }
 
       let option = this.getOptionByValue(this.value);
-      let attributeNames = selectedRichOption.constructor.recordAttributes;
+      let attributeNames = selectedRichOption.constructor.observedAttributes;
       for (let attributeName of attributeNames) {
         let attributeValue = option.getAttribute(attributeName);
         if (attributeValue) {
@@ -84,9 +90,12 @@ export default class RichSelect extends ObservedPropertiesMixin(HTMLElement) {
       }
     } else {
       selectedRichOption = new RichOption();
-      selectedRichOption.textContent = "(None selected)";
+      selectedRichOption.textContent = "(None selected)"; // XXX: bug 1473772
     }
     selectedRichOption.classList.add("rich-select-selected-option");
+    // Hide the rich-option from a11y tools since the native <select> will
+    // already provide the selected option label.
+    selectedRichOption.setAttribute("aria-hidden", "true");
     selectedRichOption = this.appendChild(selectedRichOption);
   }
 }

browser/components/payments/res/components/shipping-option.css

@@ -4,6 +4,10 @@
 
 shipping-option.rich-option {
   display: block;
+  /* Below properties are to support truncating with an ellipsis for long options */
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
 }
 
 shipping-option > .label,

browser/components/payments/res/containers/address-picker.js

@@ -13,8 +13,16 @@ import paymentRequest from "../paymentRequest.js";
  */
 
 export default class AddressPicker extends RichPicker {
+  static get pickerAttributes() {
+    return [
+      "address-fields",
+      "break-after-nth-field",
+      "data-field-separator",
+    ];
+  }
+
   static get observedAttributes() {
-    return RichPicker.observedAttributes.concat(["address-fields"]);
+    return RichPicker.observedAttributes.concat(AddressPicker.pickerAttributes);
   }
 
   constructor() {
@@ -24,7 +32,7 @@ export default class AddressPicker extends RichPicker {
 
   attributeChangedCallback(name, oldValue, newValue) {
     super.attributeChangedCallback(name, oldValue, newValue);
-    if (name == "address-fields" && oldValue !== newValue) {
+    if (AddressPicker.pickerAttributes.includes(name) && oldValue !== newValue) {
       this.render(this.requestStore.getState());
     }
   }
@@ -100,6 +108,20 @@ export default class AddressPicker extends RichPicker {
         }
       }
 
+      optionEl.dataset.fieldSeparator = this.dataset.fieldSeparator;
+
+      if (this.hasAttribute("address-fields")) {
+        optionEl.setAttribute("address-fields", this.getAttribute("address-fields"));
+      } else {
+        optionEl.removeAttribute("address-fields");
+      }
+
+      if (this.hasAttribute("break-after-nth-field")) {
+        optionEl.setAttribute("break-after-nth-field", this.getAttribute("break-after-nth-field"));
+      } else {
+        optionEl.removeAttribute("break-after-nth-field");
+      }
+
       // fieldNames getter is not used here because it returns a default array with
       // attributes even when "address-fields" observed attribute is null.
       let addressFields = this.getAttribute("address-fields");

browser/components/payments/res/containers/payment-dialog.js

@@ -265,6 +265,39 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
     }
   }
 
+  _renderPayerFields(state) {
+    let paymentOptions = state.request.paymentOptions;
+    let payerRequested = this._isPayerRequested(paymentOptions);
+    for (let element of this._payerRelatedEls) {
+      element.hidden = !payerRequested;
+    }
+
+    if (payerRequested) {
+      let fieldNames = new Set(); // default: ["name", "tel", "email"]
+      if (paymentOptions.requestPayerName) {
+        fieldNames.add("name");
+      }
+      if (paymentOptions.requestPayerEmail) {
+        fieldNames.add("email");
+      }
+      if (paymentOptions.requestPayerPhone) {
+        fieldNames.add("tel");
+      }
+      this._payerAddressPicker.setAttribute("address-fields", [...fieldNames].join(" "));
+      // For the payer picker we want to have a line break after the name field (#1)
+      // if all three fields are requested.
+      if (fieldNames.size == 3) {
+        this._payerAddressPicker.setAttribute("break-after-nth-field", 1);
+      } else {
+        this._payerAddressPicker.removeAttribute("break-after-nth-field");
+      }
+    } else {
+      this._payerAddressPicker.removeAttribute("address-fields");
+    }
+    this._payerAddressPicker.dataset.addAddressTitle = this.dataset.payerTitleAdd;
+    this._payerAddressPicker.dataset.editAddressTitle = this.dataset.payerTitleEdit;
+  }
+
   stateChangeCallback(state) {
     super.stateChangeCallback(state);
 
@@ -325,28 +358,8 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
     for (let element of this._shippingRelatedEls) {
       element.hidden = !paymentOptions.requestShipping;
     }
-    let payerRequested = this._isPayerRequested(paymentOptions);
-    for (let element of this._payerRelatedEls) {
-      element.hidden = !payerRequested;
-    }
 
-    if (payerRequested) {
-      let fieldNames = new Set(); // default: ["name", "tel", "email"]
-      if (paymentOptions.requestPayerName) {
-        fieldNames.add("name");
-      }
-      if (paymentOptions.requestPayerEmail) {
-        fieldNames.add("email");
-      }
-      if (paymentOptions.requestPayerPhone) {
-        fieldNames.add("tel");
-      }
-      this._payerAddressPicker.setAttribute("address-fields", [...fieldNames].join(" "));
-    } else {
-      this._payerAddressPicker.removeAttribute("address-fields");
-    }
-    this._payerAddressPicker.dataset.addAddressTitle = this.dataset.payerTitleAdd;
-    this._payerAddressPicker.dataset.editAddressTitle = this.dataset.payerTitleEdit;
+    this._renderPayerFields(state);
 
     // hide the accepted cards list if the merchant didn't specify a preference
     let basicCardMethod = request.paymentMethods

browser/components/payments/res/containers/rich-picker.css

@@ -35,8 +35,8 @@
   grid-area: dropdown;
 }
 
-.invalid-selected-option > rich-select {
-  outline: 1px solid #c70011;
+.invalid-selected-option > rich-select > select {
+  border: 1px solid #c70011;
 }
 
 .rich-picker > .invalid-label {

browser/components/payments/res/containers/rich-picker.js

@@ -63,8 +63,15 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
   }
 
   get selectedOption() {
-    return this.dropdown &&
-           this.dropdown.selectedOption;
+    return this.dropdown.selectedOption;
+  }
+
+  get selectedRichOption() {
+    return this.dropdown.selectedRichOption;
+  }
+
+  get requiredFields() {
+    return this.selectedOption ? this.selectedOption.requiredFields || [] : [];
   }
 
   get fieldNames() {
@@ -81,7 +88,7 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
       return [];
     }
 
-    let fieldNames = this.fieldNames;
+    let fieldNames = this.selectedRichOption.requiredFields;
     // Return all field names that are empty or missing from the option.
     return fieldNames.filter(name => !selectedOption.getAttribute(name));
   }

browser/components/payments/res/debugging.js

@@ -56,8 +56,8 @@ let REQUEST_1 = {
     shippingAddressErrors: {},
     shippingOptions: [
       {
-        id: "123",
-        label: "Fast",
+        id: "std",
+        label: "Standard (3-5 business days)",
         amount: {
           currency: "USD",
           value: 10,
@@ -65,11 +65,12 @@ let REQUEST_1 = {
         selected: false,
       },
       {
-        id: "456",
-        label: "Faster (default)",
+        id: "super-slow",
+        // Long to test truncation
+        label: "Ssssssssuuuuuuuuupppppeeeeeeerrrrr sssssllllllloooooowwwwww",
         amount: {
           currency: "USD",
-          value: 20,
+          value: 1.50,
         },
         selected: true,
       },
@@ -92,7 +93,19 @@ let REQUEST_2 = {
   topLevelPrincipal: {URI: {displayHost: "example.com"}},
   requestId: "3797081f-a96b-c34b-a58b-1083c6e66e25",
   completeStatus: "",
-  paymentMethods: [],
+  paymentMethods: [
+    {
+      "supportedMethods": "basic-card",
+      "data": {
+        "supportedNetworks": [
+          "amex",
+          "discover",
+          "mastercard",
+          "visa",
+        ],
+      },
+    },
+  ],
   paymentDetails: {
     id: "",
     totalItem: {label: "", amount: {currency: "CAD", value: "25.75"}, pending: false},
@@ -208,9 +221,26 @@ let ADDRESSES_1 = {
   "abcde12345": {
     "address-level2": "Mountain View",
     "country": "US",
+    "family-name": "Fields",
+    "given-name": "Mrs.",
     "guid": "abcde12345",
     "name": "Mrs. Fields",
   },
+  "german1": {
+    "additional-name": "Y.",
+    "address-level1": "",
+    "address-level2": "Berlin",
+    "country": "DE",
+    "email": "de@example.com",
+    "family-name": "Mouse",
+    "given-name": "Anon",
+    "guid": "german1",
+    "name": "Anon Y. Mouse",
+    "organization": "Mozilla",
+    "postal-code": "10997",
+    "street-address": "Schlesische Str. 27",
+    "tel": "+49 30 983333002",
+  },
   "missing-country": {
     "address-level1": "ON",
     "address-level2": "Toronto",
@@ -343,7 +373,6 @@ let BASIC_CARDS_1 = {
     "cc-exp-month": 8,
     "cc-exp-year": 2024,
     "cc-exp": "2024-08",
-    "cc-type": "amex",
   },
 };
 

browser/components/payments/res/mixins/ObservedPropertiesMixin.js

@@ -10,6 +10,10 @@
 
 export default function ObservedPropertiesMixin(superClass) {
   return class ObservedProperties extends superClass {
+    static kebabToCamelCase(name) {
+      return name.replace(/-([a-z])/g, ($0, $1) => $1.toUpperCase());
+    }
+
     constructor() {
       super();
 
@@ -24,7 +28,7 @@ export default function ObservedPropertiesMixin(superClass) {
           continue;
         }
         // Convert attribute names from kebab-case to camelCase properties
-        Object.defineProperty(this, name.replace(/-([a-z])/g, ($0, $1) => $1.toUpperCase()), {
+        Object.defineProperty(this, ObservedProperties.kebabToCamelCase(name), {
           configurable: true,
           get() {
             return this.getAttribute(name);

browser/components/payments/res/paymentRequest.xhtml

@@ -23,6 +23,7 @@
   <!ENTITY deliveryGenericError       "Can’t deliver to this address. Select a different address.">
   <!ENTITY pickupGenericError         "Can’t pick up from this address. Select a different address.">
   <!ENTITY paymentMethodsLabel        "Payment Method">
+  <!ENTITY address.fieldSeparator     ", ">
   <!ENTITY address.addLink.label      "Add">
   <!ENTITY address.editLink.label     "Edit">
   <!ENTITY basicCard.addLink.label    "Add">
@@ -132,6 +133,7 @@
           <address-picker class="shipping-related"
                           data-add-link-label="&address.addLink.label;"
                           data-edit-link-label="&address.editLink.label;"
+                          data-field-separator="&address.fieldSeparator;"
                           data-shipping-address-label="&shippingAddressLabel;"
                           data-delivery-address-label="&deliveryAddressLabel;"
                           data-pickup-address-label="&pickupAddressLabel;"
@@ -155,6 +157,7 @@
                           label="&payerLabel;"
                           data-add-link-label="&payer.addLink.label;"
                           data-edit-link-label="&payer.editLink.label;"
+                          data-field-separator="&address.fieldSeparator;"
                           data-invalid-label="&invalidOption.label;"
                           selected-state-key="selectedPayerAddress"></address-picker>
         </div>

browser/components/payments/res/unprivileged-fallbacks.js

@@ -81,11 +81,11 @@ var PaymentDialogUtils = {
       "postalCodeLabel": country == "US" ? "zip" : "postalCode",
       "fieldsOrder": [
         {fieldId: "name", newLine: true},
-        {fieldId: "organization", newLine: true},
         {fieldId: "street-address", newLine: true},
         {fieldId: "address-level2"},
         {fieldId: "address-level1"},
         {fieldId: "postal-code"},
+        {fieldId: "organization"},
       ],
       // The following values come from addressReferences.js and should not be changed.
       /* eslint-disable-next-line max-len */

browser/components/payments/test/browser/head.js

@@ -344,6 +344,10 @@ add_task(async function setup_head() {
       // Bug 1478142 - Console spam from the Find Toolbar.
       return;
     }
+    if (msg.message && msg.message.match(/PrioEncoder is not defined/)) {
+      // Bug 1492638 - Console spam from TelemetrySession.
+      return;
+    }
     if (msg.errorMessage == "AbortError: The operation was aborted. " &&
         msg.sourceName == "" && msg.lineNumber == 0) {
       return;

browser/components/payments/test/mochitest/mochitest.ini

@@ -11,9 +11,12 @@ support-files =
    payments_common.js
 skip-if = !e10s
 
+[test_accepted_cards.html]
 [test_address_form.html]
+[test_address_option.html]
 [test_address_picker.html]
 [test_basic_card_form.html]
+[test_basic_card_option.html]
 [test_completion_error_page.html]
 [test_currency_amount.html]
 [test_labelled_checkbox.html]
@@ -28,4 +31,3 @@ skip-if = !e10s
 [test_ObservedPropertiesMixin.html]
 [test_PaymentsStore.html]
 [test_PaymentStateSubscriberMixin.html]
-[test_accepted_cards.html]

browser/components/payments/test/mochitest/payments_common.js

@@ -1,7 +1,7 @@
 "use strict";
 
 /* exported asyncElementRendered, promiseStateChange, promiseContentToChromeMessage, deepClone,
-   PTU, registerConsoleFilter, fillField */
+   PTU, registerConsoleFilter, fillField, importDialogDependencies */
 
 const PTU = SpecialPowers.Cu.import("resource://testing-common/PaymentTestUtils.jsm", {})
                             .PaymentTestUtils;
@@ -43,6 +43,27 @@ function promiseContentToChromeMessage(messageType) {
   });
 }
 
+/**
+ * Import the templates and stylesheets from the real shipping dialog to avoid
+ * duplication in the tests.
+ * @param {HTMLIFrameElement} templateFrame - Frame to copy the resources from
+ * @param {HTMLElement} destinationEl - Where to append the copied resources
+ */
+function importDialogDependencies(templateFrame, destinationEl) {
+  for (let template of templateFrame.contentDocument.querySelectorAll("template")) {
+    let imported = document.importNode(template, true);
+    destinationEl.appendChild(imported);
+  }
+
+  let baseURL = new URL("../../res/", window.location.href);
+  let stylesheetLinks = templateFrame.contentDocument.querySelectorAll("link[rel~='stylesheet']");
+  for (let stylesheet of stylesheetLinks) {
+    let imported = document.importNode(stylesheet, true);
+    imported.href = new URL(imported.getAttribute("href"), baseURL);
+    destinationEl.appendChild(imported);
+  }
+}
+
 function deepClone(obj) {
   return JSON.parse(JSON.stringify(obj));
 }

browser/components/payments/test/mochitest/test_address_option.html

@@ -0,0 +1,179 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Test the address-option component
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test the address-option component</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <script src="payments_common.js"></script>
+  <script src="../../res/vendor/custom-elements.min.js"></script>
+  <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
+
+  <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
+  <link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+  <p id="display">
+    <option id="option1"
+            data-field-separator=", "
+            address-level1="MI"
+            address-level2="Some City"
+            country="US"
+            email="foo@bar.com"
+            name="John Smith"
+            postal-code="90210"
+            street-address="123 Sesame Street,&#xA;Apt 40"
+            tel="+1 519 555-5555"
+            value="option1"
+            guid="option1"></option>
+    <option id="option2"
+            data-field-separator=", "
+            value="option2"
+            guid="option2"></option>
+
+    <rich-select id="richSelect1"
+                 option-type="address-option"></rich-select>
+  </p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+<script type="module">
+/** Test the address-option component **/
+
+import "../../res/components/address-option.js";
+import "../../res/components/rich-select.js";
+
+let option1 = document.getElementById("option1");
+let option2 = document.getElementById("option2");
+let richSelect1 = document.getElementById("richSelect1");
+
+add_task(async function test_populated_option_rendering() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+
+  is(richOption.name, "John Smith", "Check name getter");
+  is(richOption.streetAddress, "123 Sesame Street,\nApt 40", "Check streetAddress getter");
+  is(richOption.addressLevel2, "Some City", "Check addressLevel2 getter");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith, 123 Sesame Street, Apt 40", "Line 1 text");
+  is(richOption._line2.innerText, "Some City, MI, 90210, US", "Line 2 text");
+
+  // Note that innerText takes visibility into account so that's why it's used over textContent here
+  is(richOption._name.innerText, "John Smith", "name text");
+  is(richOption["_street-address"].innerText, "123 Sesame Street, Apt 40", "street-address text");
+  is(richOption["_address-level2"].innerText, "Some City", "address-level2 text");
+
+  is(richOption._email.parentElement, null,
+     "Check email field isn't in the document for a mailing-address option");
+});
+
+// Same option as the last test but with @break-after-nth-field=1
+add_task(async function test_breakAfterNthField() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  richOption.breakAfterNthField = 1;
+  await asyncElementRendered();
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith", "Line 1 text with breakAfterNthField = 1");
+  is(richOption._line2.innerText, "123 Sesame Street, Apt 40, Some City, MI, 90210, US",
+     "Line 2 text with breakAfterNthField = 1");
+});
+
+add_task(async function test_addressField_mailingAddress() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  richOption.addressFields = "mailing-address";
+  await asyncElementRendered();
+  is(richOption.getAttribute("address-fields"), "mailing-address", "Check @address-fields");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith, 123 Sesame Street, Apt 40", "Line 1 text");
+  is(richOption._line2.innerText, "Some City, MI, 90210, US", "Line 2 text");
+
+  ok(!isHidden(richOption._line2), "Line 2 should be visible when it's used");
+
+  is(richOption._email.parentElement, null,
+     "Check email field isn't in the document for a mailing-address option");
+});
+
+add_task(async function test_addressField_nameEmail() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  richOption.addressFields = "name email";
+  await asyncElementRendered();
+  is(richOption.getAttribute("address-fields"), "name email", "Check @address-fields");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith, foo@bar.com", "Line 1 text");
+  is(richOption._line2.innerText, "", "Line 2 text");
+
+  ok(isHidden(richOption._line2), "Line 2 should be hidden when it's not used");
+
+  isnot(richOption._email.parentElement, null,
+        "Check email field is in the document for a 'name email' option");
+});
+
+add_task(async function test_missing_fields_option_rendering() {
+  richSelect1.popupBox.appendChild(option2);
+  richSelect1.value = option2.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  is(richOption.name, null, "Check name getter");
+  is(richOption.streetAddress, null, "Check streetAddress getter");
+  is(richOption.addressLevel2, null, "Check addressLevel2 getter");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  is(richOption._name.innerText, "", "name text");
+  is(window.getComputedStyle(richOption._name, "::before").content, "attr(data-missing-string)",
+     "Check missing field pseudo content");
+  is(richOption._name.getAttribute("data-missing-string"), "Name Missing",
+     "Check @data-missing-string");
+  is(richOption._email.parentElement, null,
+     "Check email field isn't in the document for a mailing-address option");
+});
+
+</script>
+
+</body>
+</html>

browser/components/payments/test/mochitest/test_address_picker.html

@@ -12,6 +12,7 @@ Test the address-picker component
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
@@ -21,6 +22,7 @@ Test the address-picker component
 <body>
   <p id="display">
     <address-picker id="picker1"
+                    data-field-separator=", "
                     selected-state-key="selectedShippingAddress"></address-picker>
   </p>
 <div id="content" style="display: none">
@@ -162,13 +164,13 @@ add_task(async function test_change_selected_address() {
   ok(isHidden(picker1.invalidLabel), "The invalid label should be hidden");
 });
 
-add_task(async function test_streetAddress_combines_street_level2_level1_postalCode_country() {
+add_task(async function test_address_combines_name_street_level2_level1_postalCode_country() {
   let options = picker1.dropdown.popupBox.children;
   let richoption1 = picker1.dropdown.querySelector(".rich-select-selected-option");
-  let streetAddress = richoption1.querySelector(".street-address");
   /* eslint-disable max-len */
-  is(streetAddress.textContent,
-     `${options[1].getAttribute("street-address")} ${options[1].getAttribute("address-level2")} ${options[1].getAttribute("address-level1")} ${options[1].getAttribute("postal-code")} ${options[1].getAttribute("country")}`,
+  is(richoption1.innerText,
+     `${options[1].getAttribute("name")}, ${options[1].getAttribute("street-address")}
+${options[1].getAttribute("address-level2")}, ${options[1].getAttribute("address-level1")}, ${options[1].getAttribute("postal-code")}, ${options[1].getAttribute("country")}`,
      "The address shown should be human readable and include all fields");
   /* eslint-enable max-len */
 
@@ -177,8 +179,8 @@ add_task(async function test_streetAddress_combines_street_level2_level1_postalC
   await asyncElementRendered();
 
   richoption1 = picker1.dropdown.querySelector(".rich-select-selected-option");
-  streetAddress = richoption1.querySelector(".street-address");
-  is(streetAddress.textContent, " Mountain View   US",
+  // "Missing …" text is rendered via a pseudo element content and isn't included in innerText
+  is(richoption1.innerText, "Mrs. Fields, \nMountain View, , US",
      "The address shown should be human readable and include all fields");
 
   picker1.dropdown.popupBox.focus();

browser/components/payments/test/mochitest/test_basic_card_option.html

@@ -0,0 +1,98 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Test the basic-card-option component
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test the basic-card-option component</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <script src="payments_common.js"></script>
+  <script src="../../res/vendor/custom-elements.min.js"></script>
+  <script src="../../res/unprivileged-fallbacks.js"></script>
+
+  <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
+  <link rel="stylesheet" type="text/css" href="../../res/components/basic-card-option.css"/>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+  <p id="display">
+    <option id="option1"
+            value="option1"
+            cc-exp="2024-06"
+            cc-name="John Smith"
+            cc-number="************5461"
+            cc-type="visa"
+            guid="option1"></option>
+    <option id="option2"
+            value="option2"
+            cc-number="************1111"
+            guid="option2"></option>
+
+    <rich-select id="richSelect1"
+                 option-type="basic-card-option"></rich-select>
+  </p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+<script type="module">
+/** Test the basic-card-option component **/
+
+import "../../res/components/basic-card-option.js";
+import "../../res/components/rich-select.js";
+
+let option1 = document.getElementById("option1");
+let option2 = document.getElementById("option2");
+let richSelect1 = document.getElementById("richSelect1");
+
+add_task(async function test_populated_option_rendering() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  is(richOption.ccExp, "2024-06", "Check ccExp getter");
+  is(richOption.ccName, "John Smith", "Check ccName getter");
+  is(richOption.ccNumber, "************5461", "Check ccNumber getter");
+  is(richOption.ccType, "visa", "Check ccType getter");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  // Note that innerText takes visibility into account so that's why it's used over textContent here
+  is(richOption["_cc-exp"].innerText, "Exp. 2024-06", "cc-exp text");
+  is(richOption["_cc-name"].innerText, "John Smith", "cc-name text");
+  is(richOption["_cc-number"].innerText, "****5461", "cc-number text");
+  is(richOption["_cc-type"].localName, "img", "cc-type localName");
+  is(richOption["_cc-type"].alt, "visa", "cc-type img alt");
+});
+
+add_task(async function test_minimal_option_rendering() {
+  richSelect1.popupBox.appendChild(option2);
+  richSelect1.value = option2.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  is(richOption.ccExp, null, "Check ccExp getter");
+  is(richOption.ccName, null, "Check ccName getter");
+  is(richOption.ccNumber, "************1111", "Check ccNumber getter");
+  is(richOption.ccType, null, "Check ccType getter");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  is(richOption["_cc-exp"].innerText, "", "cc-exp text");
+  is(richOption["_cc-name"].innerText, "", "cc-name text");
+  is(richOption["_cc-number"].innerText, "****1111", "cc-number text");
+  is(richOption["_cc-type"].localName, "img", "cc-type localName");
+  is(richOption["_cc-type"].alt, "", "cc-type img alt");
+});
+
+</script>
+
+</body>
+</html>

browser/components/payments/test/mochitest/test_payer_address_picker.html

@@ -12,6 +12,7 @@ Test the paymentOptions address-picker
 
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
@@ -19,7 +20,7 @@ Test the paymentOptions address-picker
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
-  <p id="display">
+  <p id="display" style="height: 100vh; margin: 0;">
     <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"></iframe>
   </p>
 <div id="content" style="display: none">
@@ -126,11 +127,7 @@ add_task(async function setup_once() {
   await SimpleTest.promiseFocus(templateFrame.contentWindow);
 
   let displayEl = document.getElementById("display");
-  // Import the templates from the real shipping dialog to avoid duplication.
-  for (let template of templateFrame.contentDocument.querySelectorAll("template")) {
-    let imported = document.importNode(template, true);
-    displayEl.appendChild(imported);
-  }
+  importDialogDependencies(templateFrame, displayEl);
 
   elDialog = new PaymentDialog();
   displayEl.appendChild(elDialog);
@@ -193,8 +190,7 @@ add_task(async function test_visible_fields() {
     ok(elem, `field ${fieldName} exists`);
     ok(isVisible(elem), `field ${fieldName} is visible`);
   }
-  ok(!isVisible(closedRichOption.querySelector(".street-address")),
-     "street-address is not visible");
+  ok(!closedRichOption.querySelector(".street-address"), "street-address element is not present");
 });
 
 add_task(async function test_selective_fields() {
@@ -207,6 +203,7 @@ add_task(async function test_selective_fields() {
   });
 
   let payerFieldVariations = [
+    {requestPayerName: true, requestPayerEmail: true, requestPayerPhone: true },
     {requestPayerName: true, requestPayerEmail: false, requestPayerPhone: false },
     {requestPayerName: false, requestPayerEmail: true, requestPayerPhone: false },
     {requestPayerName: false, requestPayerEmail: false, requestPayerPhone: true },
@@ -224,12 +221,22 @@ add_task(async function test_selective_fields() {
     let elEmail = closedRichOption.querySelector(".email");
     let elPhone = closedRichOption.querySelector(".tel");
 
-    is(isVisible(elName), payerFields.requestPayerName,
+    is(!!elName && isVisible(elName), payerFields.requestPayerName,
        "name field is correctly toggled");
-    is(isVisible(elEmail), payerFields.requestPayerEmail,
+    is(!!elEmail && isVisible(elEmail), payerFields.requestPayerEmail,
        "email field is correctly toggled");
-    is(isVisible(elPhone), payerFields.requestPayerPhone,
+    is(!!elPhone && isVisible(elPhone), payerFields.requestPayerPhone,
        "tel field is correctly toggled");
+
+    let numPayerFieldsRequested = [...Object.values(payerFields)].filter(val => val).length;
+    is(elPicker.getAttribute("break-after-nth-field"), numPayerFieldsRequested == 3 ? "1" : null,
+       "Check @break-after-nth-field");
+    if (numPayerFieldsRequested == 3) {
+      is(closedRichOption.breakAfterNthField, "1",
+         "Make sure @break-after-nth-field was propagated to <address-option>");
+    } else {
+      is(closedRichOption.breakAfterNthField, null, "Make sure @break-after-nth-field was cleared");
+    }
   }
 });
 

browser/components/payments/test/mochitest/test_payment_dialog.html

@@ -13,14 +13,16 @@ Test the payment-dialog custom element
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/paymentRequest.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
 </head>
 <body>
-  <p id="display">
-    <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"></iframe>
+  <p id="display" style="height: 100vh; margin: 0;">
+    <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"
+            style="float: left;"></iframe>
   </p>
 <div id="content" style="display: none">
 
@@ -39,13 +41,8 @@ let el1;
 add_task(async function setup_once() {
   let templateFrame = document.getElementById("templateFrame");
   await SimpleTest.promiseFocus(templateFrame.contentWindow);
-
   let displayEl = document.getElementById("display");
-  // Import the templates from the real shipping dialog to avoid duplication.
-  for (let template of templateFrame.contentDocument.querySelectorAll("template")) {
-    let imported = document.importNode(template, true);
-    displayEl.appendChild(imported);
-  }
+  importDialogDependencies(templateFrame, displayEl);
 
   el1 = new PaymentDialog();
   displayEl.appendChild(el1);

browser/components/payments/test/mochitest/test_payment_dialog_required_top_level_items.html

@@ -12,14 +12,16 @@ Test the payment-dialog custom element
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/paymentRequest.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
 </head>
 <body>
-  <p id="display">
-    <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"></iframe>
+  <p id="display" style="height: 100vh; margin: 0;">
+    <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"
+            style="float: left;"></iframe>
   </p>
 <div id="content" style="display: none">
 
@@ -40,11 +42,7 @@ add_task(async function setupOnce() {
   await SimpleTest.promiseFocus(templateFrame.contentWindow);
 
   let displayEl = document.getElementById("display");
-  // Import the templates from the real shipping dialog to avoid duplication.
-  for (let template of templateFrame.contentDocument.querySelectorAll("template")) {
-    let imported = document.importNode(template, true);
-    displayEl.appendChild(imported);
-  }
+  importDialogDependencies(templateFrame, displayEl);
 
   el1 = new PaymentDialog();
   displayEl.appendChild(el1);

browser/components/payments/test/mochitest/test_rich_select.html

@@ -11,6 +11,8 @@ Test the rich-select component
   <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
+  <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
@@ -61,6 +63,7 @@ for (let address of Object.values(addresses)) {
   let option = document.createElement("option");
   option.textContent = address.name + " " + address["street-address"];
   option.setAttribute("value", address.guid);
+  option.dataset.fieldSeparator = ", ";
   for (let field of Object.keys(address)) {
     option.setAttribute(field, address[field]);
   }
@@ -82,6 +85,14 @@ function is_visible(element, message) {
   ok(!isHidden(element), message);
 }
 
+add_task(async function test_clickable_area() {
+  ok(select1, "select1 exists");
+  isnot(document.activeElement, select1.popupBox, "<select> shouldn't have focus");
+  synthesizeMouseAtCenter(select1, {});
+  is(document.activeElement, select1.popupBox, "<select> should have focus when clicked");
+  document.activeElement.blur();
+});
+
 add_task(async function test_closed_state_on_selection() {
   ok(select1, "select1 exists");
   select1.popupBox.focus();

browser/extensions/formautofill/content/autofillEditForms.js

@@ -160,10 +160,10 @@ class EditAddress extends EditAutofillForm {
    * `mailing-address` is a special attribute token to indicate mailing fields + country.
    *
    * @param {object[]} mailingFieldsOrder - `fieldsOrder` from `getFormFormat`
+   * @param {string} addressFields - white-space-separated string of requested address fields to show
    * @returns {object[]} in the same structure as `mailingFieldsOrder` but including non-mail fields
    */
-  computeVisibleFields(mailingFieldsOrder) {
-    let addressFields = this._elements.form.dataset.addressFields;
+  static computeVisibleFields(mailingFieldsOrder, addressFields) {
     if (addressFields) {
       let requestedFieldClasses = addressFields.trim().split(/\s+/);
       let fieldClasses = [];
@@ -212,7 +212,8 @@ class EditAddress extends EditAutofillForm {
     } = this.getFormFormat(country);
     this._elements.addressLevel1Label.dataset.localization = addressLevel1Label;
     this._elements.postalCodeLabel.dataset.localization = postalCodeLabel;
-    let fieldClasses = this.computeVisibleFields(mailingFieldsOrder);
+    let addressFields = this._elements.form.dataset.addressFields;
+    let fieldClasses = EditAddress.computeVisibleFields(mailingFieldsOrder, addressFields);
     this.arrangeFields(fieldClasses);
     this.updatePostalCodeValidation(postalCodePattern);
   }

browser/extensions/screenshots/bootstrap.js

@@ -9,6 +9,8 @@ const HISTORY_ENABLED_PREF = "places.history.enabled";
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.defineModuleGetter(this, "AddonManager",
                                "resource://gre/modules/AddonManager.jsm");
+ChromeUtils.defineModuleGetter(this, "AddonManagerPrivate",
+                               "resource://gre/modules/AddonManager.jsm");
 ChromeUtils.defineModuleGetter(this, "AppConstants",
                                "resource://gre/modules/AppConstants.jsm");
 ChromeUtils.defineModuleGetter(this, "CustomizableUI",
@@ -95,8 +97,6 @@ const LibraryButton = {
   },
 };
 
-const APP_STARTUP = 1;
-const APP_SHUTDOWN = 2;
 let addonData, startupReason;
 
 function startup(data, reason) { // eslint-disable-line no-unused-vars
@@ -114,7 +114,7 @@ function startup(data, reason) { // eslint-disable-line no-unused-vars
 
   addonData = data;
   startupReason = reason;
-  if (reason === APP_STARTUP) {
+  if (reason === AddonManagerPrivate.BOOTSTRAP_REASONS.APP_STARTUP) {
     appStartupObserver.register();
   } else {
     appStartupDone();
@@ -129,7 +129,7 @@ function shutdown(data, reason) { // eslint-disable-line no-unused-vars
     resourceURI: addonResourceURI
   });
   // Immediately exit if Firefox is exiting, #3323
-  if (reason === APP_SHUTDOWN) {
+  if (reason === AddonManagerPrivate.BOOTSTRAP_REASONS.APP_SHUTDOWN) {
     stop(webExtension, reason);
     return;
   }
@@ -157,7 +157,8 @@ function handleStartup() {
 }
 
 function start(webExtension) {
-  return webExtension.startup(startupReason, addonData).then((api) => {
+  let reasonStr = stringReasonFromNumericReason(startupReason);
+  return webExtension.startup(reasonStr, addonData).then((api) => {
     api.browser.runtime.onMessage.addListener(handleMessage);
     LibraryButton.init(webExtension);
   }).catch((err) => {
@@ -172,10 +173,18 @@ function start(webExtension) {
 }
 
 function stop(webExtension, reason) {
-  if (reason !== APP_SHUTDOWN) {
+  if (reason !== AddonManagerPrivate.BOOTSTRAP_REASONS.APP_SHUTDOWN) {
     LibraryButton.uninit();
   }
-  return Promise.resolve(webExtension.shutdown(reason));
+  let reasonStr = stringReasonFromNumericReason(reason);
+  return Promise.resolve(webExtension.shutdown(reasonStr));
+}
+
+function stringReasonFromNumericReason(numericReason) {
+  let { BOOTSTRAP_REASONS } = AddonManagerPrivate;
+  return Object.keys(BOOTSTRAP_REASONS).find(
+    key => BOOTSTRAP_REASONS[key] == numericReason
+  );
 }
 
 function handleMessage(msg, sender, sendReply) {

devtools/client/aboutdebugging/test/browser_addons_debug_webextension_popup.js

@@ -97,7 +97,7 @@ add_task(async function testWebExtensionsToolboxSwitchToPopup() {
             toolbox.doc.getElementById("toolbox-meatball-menu-button").click();
             toolbox.doc.addEventListener("popupshown", () => {
               const menuItem =
-                toolbox.doc.getElementById("toolbox-meatball-menu-noautohide");
+                    toolbox.doc.getElementById("toolbox-meatball-menu-noautohide");
               menuItem.click();
               resolve();
             }, { once: true });
@@ -132,19 +132,31 @@ add_task(async function testWebExtensionsToolboxSwitchToPopup() {
 
         dump(`Clicking the frame list button\n`);
         const btn = toolbox.doc.getElementById("command-button-frames");
-        const frameMenu = await toolbox.showFramesMenu({target: btn});
+        btn.click();
+
+        // This is webextension toolbox process. So we can't access mochitest framework.
+        const waitUntil = function(predicate, interval = 10) {
+          if (predicate()) {
+            return Promise.resolve(true);
+          }
+          return new Promise(resolve => {
+            toolbox.win.setTimeout(function() {
+              waitUntil(predicate, interval).then(() => resolve(true));
+            }, interval);
+          });
+        };
+        await waitUntil(() => btn.style.pointerEvents === "none");
         dump(`Clicked the frame list button\n`);
 
-        await frameMenu.once("open");
-
-        const frames = frameMenu.items;
+        const menuList = toolbox.doc.getElementById("toolbox-frame-menu");
+        const frames = Array.from(menuList.querySelectorAll(".command"));
 
         if (frames.length != 2) {
           throw Error(`Number of frames found is wrong: ${frames.length} != 2`);
         }
 
         const popupFrameBtn = frames.filter((frame) => {
-          return frame.label.endsWith("popup.html");
+          return frame.querySelector(".label").textContent.endsWith("popup.html");
         }).pop();
 
         if (!popupFrameBtn) {

devtools/client/framework/components/ToolboxToolbar.js

@@ -7,9 +7,12 @@ const { Component, createFactory } = require("devtools/client/shared/vendor/reac
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const {div, button} = dom;
+const { getUnicodeUrl } = require("devtools/client/shared/unicode-url");
 
 const MeatballMenu = createFactory(require("devtools/client/framework/components/MeatballMenu"));
 const MenuButton = createFactory(require("devtools/client/shared/components/menu/MenuButton"));
+const MenuItem = createFactory(require("devtools/client/shared/components/menu/MenuItem"));
+const MenuList = createFactory(require("devtools/client/shared/components/menu/MenuList"));
 const ToolboxTabs = createFactory(require("devtools/client/framework/components/ToolboxTabs"));
 
 /**
@@ -87,6 +90,9 @@ class ToolboxToolbar extends Component {
     super(props);
 
     this.hideMenu = this.hideMenu.bind(this);
+    this.createFrameList = this.createFrameList.bind(this);
+    this.highlightFrame = this.highlightFrame.bind(this);
+    this.clickFrameButton = this.clickFrameButton.bind(this);
   }
 
   componentDidMount() {
@@ -101,6 +107,294 @@ class ToolboxToolbar extends Component {
     if (this.refs.meatballMenuButton) {
       this.refs.meatballMenuButton.hideMenu();
     }
+
+    if (this.refs.frameMenuButton) {
+      this.refs.frameMenuButton.hideMenu();
+    }
+  }
+
+  /**
+   * A little helper function to call renderToolboxButtons for buttons at the start
+   * of the toolbox.
+   */
+  renderToolboxButtonsStart() {
+    return this.renderToolboxButtons(true);
+  }
+
+  /**
+   * A little helper function to call renderToolboxButtons for buttons at the end
+   * of the toolbox.
+   */
+  renderToolboxButtonsEnd() {
+    return this.renderToolboxButtons(false);
+  }
+
+  /**
+   * Render all of the tabs, this takes in a list of toolbox button states. These are plain
+   * objects that have all of the relevant information needed to render the button.
+   * See Toolbox.prototype._createButtonState in devtools/client/framework/toolbox.js for
+   * documentation on this object.
+   *
+   * @param {String} focusedButton - The id of the focused button.
+   * @param {Array} toolboxButtons - Array of objects that define the command buttons.
+   * @param {Function} focusButton - Keep a record of the currently focused button.
+   * @param {boolean} isStart - Render either the starting buttons, or ending buttons.
+   */
+  renderToolboxButtons(isStart) {
+    const {
+      focusedButton,
+      toolboxButtons,
+      focusButton
+    } = this.props;
+    const visibleButtons = toolboxButtons.filter(command => {
+      const {isVisible, isInStartContainer} = command;
+      return isVisible && (isStart ? isInStartContainer : !isInStartContainer);
+    });
+
+    if (visibleButtons.length === 0) {
+      return null;
+    }
+
+    // The RDM button, if present, should always go last
+    const rdmIndex = visibleButtons.findIndex(
+      button => button.id === "command-button-responsive"
+    );
+    if (rdmIndex !== -1 && rdmIndex !== visibleButtons.length - 1) {
+      const rdm = visibleButtons.splice(rdmIndex, 1)[0];
+      visibleButtons.push(rdm);
+    }
+
+    const renderedButtons =
+      visibleButtons.map(command => {
+        const {
+          id,
+          description,
+          disabled,
+          onClick,
+          isChecked,
+          className: buttonClass,
+          onKeyDown
+        } = command;
+
+        // If button is frame button, create menu button in order to
+        // use the doorhanger menu.
+        if (id === "command-button-frames") {
+          return this.renderFrameButton(command);
+        }
+
+        return button({
+          id,
+          title: description,
+          disabled,
+          className: (
+            "command-button devtools-button "
+            + buttonClass + (isChecked ? " checked" : "")
+          ),
+          onClick: (event) => {
+            onClick(event);
+            focusButton(id);
+          },
+          onFocus: () => focusButton(id),
+          tabIndex: id === focusedButton ? "0" : "-1",
+          onKeyDown: (event) => {
+            onKeyDown(event);
+          }
+        });
+      });
+
+    // Add the appropriate separator, if needed.
+    const children = renderedButtons;
+    if (renderedButtons.length) {
+      if (isStart) {
+        children.push(this.renderSeparator());
+        // For the end group we add a separator *before* the RDM button if it
+        // exists, but only if it is not the only button.
+      } else if (rdmIndex !== -1 && visibleButtons.length > 1) {
+        children.splice(
+          children.length - 1,
+          0,
+          this.renderSeparator()
+        );
+      }
+    }
+
+    return div({id: `toolbox-buttons-${isStart ? "start" : "end"}`}, ...children);
+  }
+
+  renderFrameButton(command) {
+    const {
+      id,
+      disabled,
+      description
+    } = command;
+
+    const { toolbox } = this.props;
+
+    return MenuButton(
+      {
+        id,
+        disabled,
+        menuId: id + "-panel",
+        doc: toolbox.doc,
+        className: "command-button devtools-button ",
+        ref: "frameMenuButton",
+        title: description,
+        onCloseButton: toolbox.highlighterUtils.unhighlight,
+      },
+      this.createFrameList
+    );
+  }
+
+  clickFrameButton(event) {
+    const { toolbox } = this.props;
+    toolbox.onSelectFrame(event.target.id);
+  }
+
+  highlightFrame(id) {
+    if (!id) {
+      return;
+    }
+
+    const { toolbox } = this.props;
+    toolbox.onHighlightFrame(id);
+  }
+
+  createFrameList() {
+    const { toolbox } = this.props;
+    if (toolbox.frameMap.size < 1) {
+      return null;
+    }
+
+    const items = [];
+    toolbox.frameMap.forEach((frame, index) => {
+      const label = toolbox.target.isWebExtension
+                    ? toolbox.target.getExtensionPathName(frame.url)
+                    : getUnicodeUrl(frame.url);
+      items.push(MenuItem({
+        id: frame.id.toString(),
+        key: "toolbox-frame-key-" + frame.id,
+        label,
+        checked: frame.id === toolbox.selectedFrameId,
+        onClick: this.clickFrameButton
+      }));
+    });
+
+    return MenuList(
+      {
+        id: "toolbox-frame-menu",
+        onHighlightedChildChange: this.highlightFrame
+      },
+      items);
+  }
+
+  /**
+   * Render a separator.
+   */
+  renderSeparator() {
+    return div({className: "devtools-separator"});
+  }
+
+  /**
+   * Render the toolbox control buttons. The following props are expected:
+   *
+   * @param {string} props.focusedButton
+   *        The id of the focused button.
+   * @param {string} props.currentToolId
+   *        The id of the currently selected tool, e.g. "inspector".
+   * @param {Object[]} props.hostTypes
+   *        Array of host type objects.
+   * @param {string} props.hostTypes[].position
+   *        Position name.
+   * @param {Function} props.hostTypes[].switchHost
+   *        Function to switch the host.
+   * @param {string} props.currentHostType
+   *        The current docking configuration.
+   * @param {boolean} props.areDockOptionsEnabled
+   *        They are not enabled in certain situations like when they are in the
+   *        WebIDE.
+   * @param {boolean} props.canCloseToolbox
+   *        Do we need to add UI for closing the toolbox? We don't when the
+   *        toolbox is undocked, for example.
+   * @param {boolean} props.isSplitConsoleActive
+   *         Is the split console currently visible?
+   *        toolbox is undocked, for example.
+   * @param {boolean|undefined} props.disableAutohide
+   *        Are we disabling the behavior where pop-ups are automatically
+   *        closed when clicking outside them?
+   *        (Only defined for the browser toolbox.)
+   * @param {Function} props.selectTool
+   *        Function to select a tool based on its id.
+   * @param {Function} props.toggleOptions
+   *        Function to turn the options panel on / off.
+   * @param {Function} props.toggleSplitConsole
+   *        Function to turn the split console on / off.
+   * @param {Function} props.toggleNoAutohide
+   *        Function to turn the disable pop-up autohide behavior on / off.
+   * @param {Function} props.closeToolbox
+   *        Completely close the toolbox.
+   * @param {Function} props.focusButton
+   *        Keep a record of the currently focused button.
+   * @param {Object} props.L10N
+   *        Localization interface.
+   * @param {Object} props.toolbox
+   *        The devtools toolbox. Used by the MenuButton component to display
+   *        the menu popup.
+   * @param {Object} refs
+   *        The components refs object. Used to keep a reference to the MenuButton
+   *        for the meatball menu so that we can tell it to resize its contents
+   *        when they change.
+   */
+  renderToolboxControls() {
+    const {
+      focusedButton,
+      canCloseToolbox,
+      closeToolbox,
+      focusButton,
+      L10N,
+      toolbox,
+    } = this.props;
+
+    const meatballMenuButtonId = "toolbox-meatball-menu-button";
+
+    const meatballMenuButton = MenuButton(
+      {
+        id: meatballMenuButtonId,
+        menuId: meatballMenuButtonId + "-panel",
+        doc: toolbox.doc,
+        onFocus: () => focusButton(meatballMenuButtonId),
+        className: "devtools-button",
+        title: L10N.getStr("toolbox.meatballMenu.button.tooltip"),
+        tabIndex: focusedButton === meatballMenuButtonId ? "0" : "-1",
+        ref: "meatballMenuButton",
+      },
+      MeatballMenu({
+        ...this.props,
+        hostTypes: this.props.areDockOptionsEnabled ? this.props.hostTypes : [],
+        onResize: () => {
+          this.refs.meatballMenuButton.resizeContent();
+        },
+      })
+    );
+
+    const closeButtonId = "toolbox-close";
+
+    const closeButton = canCloseToolbox
+      ? button({
+        id: closeButtonId,
+        onFocus: () => focusButton(closeButtonId),
+        className: "devtools-button",
+        title: L10N.getStr("toolbox.closebutton.tooltip"),
+        onClick: () => {
+          closeToolbox();
+        },
+        tabIndex: focusedButton === "toolbox-close" ? "0" : "-1",
+      })
+      : null;
+
+    return div({id: "toolbox-controls"},
+      meatballMenuButton,
+      closeButton
+    );
   }
 
   /**
@@ -109,8 +403,8 @@ class ToolboxToolbar extends Component {
    */
   render() {
     const classnames = ["devtools-tabbar"];
-    const startButtons = renderToolboxButtonsStart(this.props);
-    const endButtons = renderToolboxButtonsEnd(this.props);
+    const startButtons = this.renderToolboxButtonsStart();
+    const endButtons = this.renderToolboxButtonsEnd();
 
     if (!startButtons) {
       classnames.push("devtools-tabbar-has-start");
@@ -128,7 +422,7 @@ class ToolboxToolbar extends Component {
           startButtons,
           ToolboxTabs(this.props),
           endButtons,
-          renderToolboxControls(this.props, this.refs)
+          this.renderToolboxControls()
         )
       )
       : div({ className: classnames.join(" ") });
@@ -136,209 +430,3 @@ class ToolboxToolbar extends Component {
 }
 
 module.exports = ToolboxToolbar;
-
-/**
- * A little helper function to call renderToolboxButtons for buttons at the start
- * of the toolbox.
- */
-function renderToolboxButtonsStart(props) {
-  return renderToolboxButtons(props, true);
-}
-
-/**
-* A little helper function to call renderToolboxButtons for buttons at the end
-* of the toolbox.
- */
-function renderToolboxButtonsEnd(props) {
-  return renderToolboxButtons(props, false);
-}
-
-/**
- * Render all of the tabs, this takes in a list of toolbox button states. These are plain
- * objects that have all of the relevant information needed to render the button.
- * See Toolbox.prototype._createButtonState in devtools/client/framework/toolbox.js for
- * documentation on this object.
- *
- * @param {String} focusedButton - The id of the focused button.
- * @param {Array} toolboxButtons - Array of objects that define the command buttons.
- * @param {Function} focusButton - Keep a record of the currently focused button.
- * @param {boolean} isStart - Render either the starting buttons, or ending buttons.
- */
-function renderToolboxButtons({focusedButton, toolboxButtons, focusButton}, isStart) {
-  const visibleButtons = toolboxButtons.filter(command => {
-    const {isVisible, isInStartContainer} = command;
-    return isVisible && (isStart ? isInStartContainer : !isInStartContainer);
-  });
-
-  if (visibleButtons.length === 0) {
-    return null;
-  }
-
-  // The RDM button, if present, should always go last
-  const rdmIndex = visibleButtons.findIndex(
-    button => button.id === "command-button-responsive"
-  );
-  if (rdmIndex !== -1 && rdmIndex !== visibleButtons.length - 1) {
-    const rdm = visibleButtons.splice(rdmIndex, 1)[0];
-    visibleButtons.push(rdm);
-  }
-
-  const renderedButtons =
-    visibleButtons.map(command => {
-      const {
-        id,
-        description,
-        disabled,
-        isChecked,
-        onClick,
-        className: buttonClass,
-        onKeyDown
-      } = command;
-      return button({
-        id,
-        title: description,
-        disabled,
-        className: (
-          "command-button devtools-button "
-          + buttonClass + (isChecked ? " checked" : "")
-        ),
-        onClick: (event) => {
-          onClick(event);
-          focusButton(id);
-        },
-        onFocus: () => focusButton(id),
-        tabIndex: id === focusedButton ? "0" : "-1",
-        onKeyDown: (event) => {
-          onKeyDown(event);
-        }
-      });
-    });
-
-  // Add the appropriate separator, if needed.
-  const children = renderedButtons;
-  if (renderedButtons.length) {
-    if (isStart) {
-      children.push(renderSeparator());
-    // For the end group we add a separator *before* the RDM button if it
-    // exists, but only if it is not the only button.
-    } else if (rdmIndex !== -1 && visibleButtons.length > 1) {
-      children.splice(
-        children.length - 1,
-        0,
-        renderSeparator()
-      );
-    }
-  }
-
-  return div({id: `toolbox-buttons-${isStart ? "start" : "end"}`}, ...children);
-}
-
-/**
- * Render a separator.
- */
-function renderSeparator() {
-  return div({className: "devtools-separator"});
-}
-
-/**
- * Render the toolbox control buttons. The following props are expected:
- *
- * @param {string} props.focusedButton
- *        The id of the focused button.
- * @param {string} props.currentToolId
- *        The id of the currently selected tool, e.g. "inspector".
- * @param {Object[]} props.hostTypes
- *        Array of host type objects.
- * @param {string} props.hostTypes[].position
- *        Position name.
- * @param {Function} props.hostTypes[].switchHost
- *        Function to switch the host.
- * @param {string} props.currentHostType
- *        The current docking configuration.
- * @param {boolean} props.areDockOptionsEnabled
- *        They are not enabled in certain situations like when they are in the
- *        WebIDE.
- * @param {boolean} props.canCloseToolbox
- *        Do we need to add UI for closing the toolbox? We don't when the
- *        toolbox is undocked, for example.
- * @param {boolean} props.isSplitConsoleActive
- *         Is the split console currently visible?
- *        toolbox is undocked, for example.
- * @param {boolean|undefined} props.disableAutohide
- *        Are we disabling the behavior where pop-ups are automatically
- *        closed when clicking outside them?
- *        (Only defined for the browser toolbox.)
- * @param {Function} props.selectTool
- *        Function to select a tool based on its id.
- * @param {Function} props.toggleOptions
- *        Function to turn the options panel on / off.
- * @param {Function} props.toggleSplitConsole
- *        Function to turn the split console on / off.
- * @param {Function} props.toggleNoAutohide
- *        Function to turn the disable pop-up autohide behavior on / off.
- * @param {Function} props.closeToolbox
- *        Completely close the toolbox.
- * @param {Function} props.focusButton
- *        Keep a record of the currently focused button.
- * @param {Object} props.L10N
- *        Localization interface.
- * @param {Object} props.toolbox
- *        The devtools toolbox. Used by the MenuButton component to display
- *        the menu popup.
- * @param {Object} refs
- *        The components refs object. Used to keep a reference to the MenuButton
- *        for the meatball menu so that we can tell it to resize its contents
- *        when they change.
- */
-function renderToolboxControls(props, refs) {
-  const {
-    focusedButton,
-    canCloseToolbox,
-    closeToolbox,
-    focusButton,
-    L10N,
-    toolbox,
-  } = props;
-
-  const meatballMenuButtonId = "toolbox-meatball-menu-button";
-
-  const meatballMenuButton = MenuButton(
-    {
-      id: meatballMenuButtonId,
-      menuId: meatballMenuButtonId + "-panel",
-      doc: toolbox.doc,
-      onFocus: () => focusButton(meatballMenuButtonId),
-      className: "devtools-button",
-      title: L10N.getStr("toolbox.meatballMenu.button.tooltip"),
-      tabIndex: focusedButton === meatballMenuButtonId ? "0" : "-1",
-      ref: "meatballMenuButton",
-    },
-    MeatballMenu({
-      ...props,
-      hostTypes: props.areDockOptionsEnabled ? props.hostTypes : [],
-      onResize: () => {
-        refs.meatballMenuButton.resizeContent();
-      },
-    })
-  );
-
-  const closeButtonId = "toolbox-close";
-
-  const closeButton = canCloseToolbox
-    ? button({
-      id: closeButtonId,
-      onFocus: () => focusButton(closeButtonId),
-      className: "devtools-button",
-      title: L10N.getStr("toolbox.closebutton.tooltip"),
-      onClick: () => {
-        closeToolbox();
-      },
-      tabIndex: focusedButton === "toolbox-close" ? "0" : "-1",
-    })
-    : null;
-
-  return div({id: "toolbox-controls"},
-    meatballMenuButton,
-    closeButton
-  );
-}

devtools/client/framework/test/browser_toolbox_meatball.js

@@ -60,7 +60,7 @@ async function openMeatballMenuWithClick(toolbox) {
 
 async function closeMeatballMenuWithClick(toolbox) {
   const meatballButton = toolbox.doc.getElementById("toolbox-meatball-menu-button");
-  await waitUntil(() => meatballButton.style.pointerEvents === "none");
+  await waitUntil(() => toolbox.win.getComputedStyle(meatballButton).pointerEvents === "none");
   meatballButton.click();
 
   const menuPanel = toolbox.doc.getElementById("toolbox-meatball-menu-button-panel");

devtools/client/framework/test/browser_toolbox_options_frames_button.js

@@ -51,8 +51,10 @@ add_task(async function() {
   framesButton = doc.getElementById("command-button-frames");
   ok(framesButton, "Frames button is still rendered.");
 
-  await waitUntil(() => !framesButton.disabled);
-  ok(!framesButton.disabled, "Frames button is not disabled.");
+  await waitUntil(() => {
+    framesButton = doc.getElementById("command-button-frames");
+    return framesButton && !framesButton.disabled;
+  });
 
   Services.prefs.clearUserPref(FRAME_BUTTON_PREF);
 });

devtools/client/framework/test/browser_toolbox_window_title_frame_select.js

@@ -42,19 +42,27 @@ add_task(async function() {
   await testShortcutToOpenFrames(btn, toolbox);
 
   // Open frame menu and wait till it's available on the screen.
-  // Also check 'open' attribute on the command button.
-  ok(!btn.classList.contains("checked"), "The checked class must not be present");
-  const menu = await toolbox.showFramesMenu({target: btn});
-  await once(menu, "open");
+  // Also check 'aria-expanded' attribute on the command button.
+  is(btn.getAttribute("aria-expanded"), "false",
+     "The aria-expanded attribute must be set to false");
+  btn.click();
 
-  ok(btn.classList.contains("checked"), "The checked class must be set");
+  const panel = toolbox.doc.getElementById("command-button-frames-panel");
+  ok(panel, "popup panel has created.");
+  await waitUntil(() => panel.classList.contains("tooltip-visible"));
+
+  is(btn.getAttribute("aria-expanded"), "true",
+     "The aria-expanded attribute must be set to true");
 
   // Verify that the frame list menu is populated
-  const frames = menu.items;
+  const menuList = toolbox.doc.getElementById("toolbox-frame-menu");
+  const frames = Array.from(menuList.querySelectorAll(".command"));
   is(frames.length, 2, "We have both frames in the list");
 
-  const topFrameBtn = frames.filter(b => b.label == URL)[0];
-  const iframeBtn = frames.filter(b => b.label == IFRAME_URL)[0];
+  const topFrameBtn =
+        frames.filter(b => b.querySelector(".label").textContent == URL)[0];
+  const iframeBtn =
+        frames.filter(b => b.querySelector(".label").textContent == IFRAME_URL)[0];
   ok(topFrameBtn, "Got top level document in the list");
   ok(iframeBtn, "Got iframe document in the list");
 
@@ -100,16 +108,17 @@ async function testShortcutToOpenFrames(btn, toolbox) {
   const shortcut = L10N.getStr("toolbox.showFrames.key");
   synthesizeKeyShortcut(shortcut, toolbox.win);
 
-  // wait for 200 ms for UI to render
-  await wait(200);
+  const panel = toolbox.doc.getElementById("command-button-frames-panel");
+  ok(panel, "popup panel has created.");
+  await waitUntil(() => panel.classList.contains("tooltip-visible"));
 
-  // btn should now have the checked class set
-  ok(btn.classList.contains("checked"), "The checked class must be set");
+  is(btn.getAttribute("aria-expanded"), "true",
+     "The aria-expanded attribute must be set to true");
 
   // pressing Esc should hide the menu again
-  synthesizeKeyShortcut("Esc", toolbox.win);
-  await wait(200);
+  EventUtils.sendKey("ESCAPE", toolbox.win);
+  await waitUntil(() => !panel.classList.contains("tooltip-visible"));
 
-  // btn shouldn't have the checked class set
-  ok(!btn.classList.contains("checked"), "The checked class must not be set");
+  is(btn.getAttribute("aria-expanded"), "false",
+     "The aria-expanded attribute must be set to false");
 }

devtools/client/framework/test/browser_toolbox_zoom_popup.js

@@ -118,7 +118,13 @@ add_task(async function() {
  */
 async function getButtonAndMenuInfo(doc, menuButton) {
   info("Show popup menu with click event.");
-  menuButton.click();
+  EventUtils.sendMouseEvent(
+    {
+      type: "click",
+      screenX: 1,
+    },
+    menuButton,
+    doc.defaultView);
 
   let menuPopup;
   let menuType;

devtools/client/framework/toolbox.js

@@ -24,8 +24,6 @@ var EventEmitter = require("devtools/shared/event-emitter");
 var Telemetry = require("devtools/client/shared/telemetry");
 const { getUnicodeUrl } = require("devtools/client/shared/unicode-url");
 var { attachThread, detachThread } = require("./attach-thread");
-var Menu = require("devtools/client/framework/menu");
-var MenuItem = require("devtools/client/framework/menu-item");
 var { DOMHelpers } = require("resource://devtools/client/shared/DOMHelpers.jsm");
 const { KeyCodes } = require("devtools/client/shared/keycodes");
 var Startup = Cc["@mozilla.org/devtools/startup-clh;1"].getService(Ci.nsISupports)
@@ -128,9 +126,6 @@ function Toolbox(target, selectedTool, hostType, contentWindow, frameId,
   this._onWillNavigate = this._onWillNavigate.bind(this);
   this._refreshHostTitle = this._refreshHostTitle.bind(this);
   this.toggleNoAutohide = this.toggleNoAutohide.bind(this);
-  this.showFramesMenu = this.showFramesMenu.bind(this);
-  this.handleKeyDownOnFramesButton = this.handleKeyDownOnFramesButton.bind(this);
-  this.showFramesMenuOnKeyDown = this.showFramesMenuOnKeyDown.bind(this);
   this._updateFrames = this._updateFrames.bind(this);
   this._splitConsoleOnKeypress = this._splitConsoleOnKeypress.bind(this);
   this.destroy = this.destroy.bind(this);
@@ -1235,7 +1230,6 @@ Toolbox.prototype = {
     this.frameButton = this._createButtonState({
       id: "command-button-frames",
       description: L10N.getStr("toolbox.frames.tooltip"),
-      onClick: this.showFramesMenu,
       isTargetSupported: target => {
         return target.activeTab && target.activeTab.traits.frames;
       },
@@ -1244,7 +1238,13 @@ Toolbox.prototype = {
         const isOnOptionsPanel = this.currentToolId === "options";
         return hasFrames || isOnOptionsPanel;
       },
-      onKeyDown: this.handleKeyDownOnFramesButton
+    });
+
+    // Listen for the shortcut key to show the frame list
+    this.shortcuts.on(L10N.getStr("toolbox.showFrames.key"), event => {
+      if (event.target.id === "command-button-frames") {
+        event.target.click();
+      }
     });
 
     return this.frameButton;
@@ -2285,88 +2285,6 @@ Toolbox.prototype = {
     this._updateFrames({ frames });
   },
 
-  /**
-   * Show a drop down menu that allows the user to switch frames.
-   */
-  showFramesMenu: async function(event) {
-    const menu = new Menu();
-    const target = event.target;
-
-    // Need to initInspector to check presence of getNodeActorFromWindowID
-    // and use the highlighter later
-    await this.initInspector();
-    if (!("_supportsFrameHighlight" in this)) {
-    // Only works with FF58+ targets
-      this._supportsFrameHighlight =
-        await this.target.actorHasMethod("domwalker", "getNodeActorFromWindowID");
-    }
-
-    // Generate list of menu items from the list of frames.
-    this.frameMap.forEach(frame => {
-      // A frame is checked if it's the selected one.
-      const checked = frame.id == this.selectedFrameId;
-
-      let label;
-      if (this.target.isWebExtension) {
-        // Show a shorter url for extensions page.
-        label = this.target.getExtensionPathName(frame.url);
-      } else {
-        label = getUnicodeUrl(frame.url);
-      }
-
-      // Create menu item.
-      menu.append(new MenuItem({
-        label,
-        type: "radio",
-        checked,
-        click: () => {
-          this.onSelectFrame(frame.id);
-        },
-        hover: () => {
-          this.onHightlightFrame(frame.id);
-        }
-      }));
-    });
-
-    menu.once("open").then(() => {
-      this.frameButton.isChecked = true;
-    });
-
-    menu.once("close").then(() => {
-      this.frameButton.isChecked = false;
-      this.highlighterUtils.unhighlight();
-    });
-
-    // Show a drop down menu with frames.
-    // XXX Missing menu API for specifying target (anchor)
-    // and relative position to it. See also:
-    // https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Method/openPopup
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1274551
-    const rect = target.getBoundingClientRect();
-    const screenX = target.ownerDocument.defaultView.mozInnerScreenX;
-    const screenY = target.ownerDocument.defaultView.mozInnerScreenY;
-    menu.popupWithZoom(rect.left + screenX, rect.bottom + screenY, this);
-
-    return menu;
-  },
-
-  /**
-   * Handle keyDown event on 'frames' button to show available frames
-   */
-  handleKeyDownOnFramesButton: function(event) {
-    this.shortcuts.on(L10N.getStr("toolbox.showFrames.key"),
-      this.showFramesMenuOnKeyDown);
-  },
-
-  /**
-   * Show 'frames' menu on key down
-   */
-  showFramesMenuOnKeyDown: function(event) {
-    if (event.target.id == "command-button-frames") {
-      this.showFramesMenu(event);
-    }
-  },
-
   /**
    * Select a frame by sending 'switchToFrame' packet to the backend.
    */
@@ -2379,7 +2297,11 @@ Toolbox.prototype = {
   /**
    * Highlight a frame in the page
    */
-  onHightlightFrame: async function(frameId) {
+  onHighlightFrame: async function(frameId) {
+    // Need to initInspector to check presence of getNodeActorFromWindowID
+    // and use the highlighter later
+    await this.initInspector();
+
     // Only enable frame highlighting when the top level document is targeted
     if (this._supportsFrameHighlight && this.rootFrameSelected) {
       const frameActor = await this.walker.getNodeActorFromWindowID(frameId);
@@ -2732,6 +2654,11 @@ Toolbox.prototype = {
           const autohide = !flags.testing;
           this._highlighter = await this._inspector.getHighlighter(autohide);
         }
+        if (!("_supportsFrameHighlight" in this)) {
+          // Only works with FF58+ targets
+          this._supportsFrameHighlight =
+            await this.target.actorHasMethod("domwalker", "getNodeActorFromWindowID");
+        }
       }.bind(this))();
     }
     return this._initInspector;

devtools/client/inspector/test/browser_inspector_highlighter-iframes_02.js

@@ -44,13 +44,17 @@ add_task(async function() {
 async function switchToFrameContext(frameIndex, toolbox, inspector) {
   // Open frame menu and wait till it's available on the screen.
   const btn = toolbox.doc.getElementById("command-button-frames");
-  const menu = await toolbox.showFramesMenu({target: btn});
-  await once(menu, "open");
+  const panel = toolbox.doc.getElementById("command-button-frames-panel");
+  btn.click();
+  ok(panel, "popup panel has created.");
+  await waitUntil(() => panel.classList.contains("tooltip-visible"));
 
   info("Select the iframe in the frame list.");
+  const menuList = toolbox.doc.getElementById("toolbox-frame-menu");
+  const firstButton = menuList.querySelectorAll(".command")[frameIndex];
   const newRoot = inspector.once("new-root");
 
-  menu.items[frameIndex].click();
+  firstButton.click();
 
   await newRoot;
   await inspector.once("inspector-updated");

devtools/client/inspector/test/browser_inspector_inspect_node_contextmenu.js

@@ -54,16 +54,19 @@ async function testContextMenuWithinIframe(testActor, nodeFrontGetter) {
 async function changeToolboxToInnerFrame() {
   const { toolbox } = getActiveInspector();
 
-  const frameButton = toolbox.doc.getElementById("command-button-frames");
-  const menu = await toolbox.showFramesMenu({
-    target: frameButton
-  });
-  await once(menu, "open");
+  const btn = toolbox.doc.getElementById("command-button-frames");
+  const panel = toolbox.doc.getElementById("command-button-frames-panel");
+  btn.click();
+  ok(panel, "popup panel has created.");
+  await waitUntil(() => panel.classList.contains("tooltip-visible"));
 
-  const frames = menu.items;
+  info("Select the iframe in the frame list.");
+  const menuList = toolbox.doc.getElementById("toolbox-frame-menu");
+  const frames = Array.from(menuList.querySelectorAll(".command"));
   is(frames.length, 2, "Two frames shown in the switcher");
 
-  const innerFrameButton = frames.filter(f => f.label == FRAME_URI)[0];
+  const innerFrameButton =
+        frames.filter(b => b.querySelector(".label").textContent === FRAME_URI)[0];
   ok(innerFrameButton, "Found frame button for inner frame");
 
   const newRoot = toolbox.getPanel("inspector").once("new-root");

devtools/client/inspector/test/browser_inspector_select-docshell.js

@@ -29,19 +29,24 @@ add_task(async function() {
   ok(!btn.firstChild, "The frame list button doesn't have any children");
 
   // Open frame menu and wait till it's available on the screen.
-  const menu = await toolbox.showFramesMenu({target: btn});
-  await once(menu, "open");
+  const panel = toolbox.doc.getElementById("command-button-frames-panel");
+  btn.click();
+  ok(panel, "popup panel has created.");
+  await waitUntil(() => panel.classList.contains("tooltip-visible"));
 
   // Verify that the menu is popuplated.
-  const frames = menu.items.slice();
+  const menuList = toolbox.doc.getElementById("toolbox-frame-menu");
+  const frames = Array.prototype.slice.call(menuList.querySelectorAll(".command"));
   is(frames.length, 2, "We have both frames in the menu");
 
   frames.sort(function(a, b) {
-    return a.label.localeCompare(b.label);
+    return a.children[0].innerHTML.localeCompare(b.children[0].innerHTML);
   });
 
-  is(frames[0].label, FrameURL, "Got top level document in the list");
-  is(frames[1].label, URL, "Got iframe document in the list");
+  is(frames[0].querySelector(".label").textContent, FrameURL,
+     "Got top level document in the list");
+  is(frames[1].querySelector(".label").textContent, URL,
+     "Got iframe document in the list");
 
   // Listen to will-navigate to check if the view is empty
   const willNavigate = toolbox.target.once("will-navigate").then(() => {

devtools/client/shared/components/menu/MenuButton.js

@@ -48,6 +48,9 @@ class MenuButton extends PureComponent {
 
       // Callback function to be invoked when the button is clicked.
       onClick: PropTypes.func,
+
+      // Callback function to be invoked when the child panel is closed.
+      onCloseButton: PropTypes.func,
     };
   }
 
@@ -100,6 +103,15 @@ class MenuButton extends PureComponent {
     }
   }
 
+  componentDidUpdate() {
+    // The MenuButton creates the child panel when initializing the MenuButton.
+    // If the children function is called during the rendering process,
+    // this child list size might change. So we need to adjust content size here.
+    if (typeof this.props.children === "function") {
+      this.resizeContent();
+    }
+  }
+
   componentWillUnmount() {
     this.resetTooltip();
   }
@@ -193,6 +205,10 @@ class MenuButton extends PureComponent {
         this.buttonRef.style.pointerEvents = "auto";
       }
     }, 0);
+
+    if (this.props.onCloseButton) {
+      this.props.onCloseButton();
+    }
   }
 
   async onClick(e) {
@@ -224,6 +240,12 @@ class MenuButton extends PureComponent {
         if (wasKeyboardEvent && this.tooltip) {
           this.tooltip.focus();
         }
+
+        // MenuButton creates the children dynamically when clicking the button,
+        // so execute the goggle menu after updating the children panel.
+        if (typeof this.props.children === "function") {
+          this.forceUpdate();
+        }
       }
     // If we clicked one of the menu items, then, by default, we should
     // auto-collapse the menu.
@@ -274,7 +296,9 @@ class MenuButton extends PureComponent {
     //
     // Bug 1472942: Do this for all users of HTMLTooltip.
     const menu = ReactDOM.createPortal(
-      this.props.children,
+      typeof this.props.children === "function"
+        ? this.props.children()
+        : this.props.children,
       this.tooltip.panel
     );
 

devtools/client/shared/components/menu/MenuList.js

@@ -24,6 +24,11 @@ class MenuList extends PureComponent {
 
       // Children of the list.
       children: PropTypes.any,
+
+      // Called whenever there is a change to the hovered or selected child.
+      // The callback is passed the ID of the highlighted child or null if no
+      // child is highlighted.
+      onHighlightedChildChange: PropTypes.func,
     };
   }
 
@@ -31,12 +36,32 @@ class MenuList extends PureComponent {
     super(props);
 
     this.onKeyDown = this.onKeyDown.bind(this);
+    this.onMouseOverOrFocus = this.onMouseOverOrFocus.bind(this);
+    this.onMouseOutOrBlur = this.onMouseOutOrBlur.bind(this);
+    this.notifyHighlightedChildChange = this.notifyHighlightedChildChange.bind(this);
 
     this.setWrapperRef = element => {
       this.wrapperRef = element;
     };
   }
 
+  onMouseOverOrFocus(e) {
+    this.notifyHighlightedChildChange(e.target.id);
+  }
+
+  onMouseOutOrBlur(e) {
+    const hoveredElem = this.wrapperRef.querySelector(":hover");
+    if (!hoveredElem) {
+      this.notifyHighlightedChildChange(null);
+    }
+  }
+
+  notifyHighlightedChildChange(id) {
+    if (this.props.onHighlightedChildChange) {
+      this.props.onHighlightedChildChange(id);
+    }
+  }
+
   onKeyDown(e) {
     // Check if the focus is in the list.
     if (
@@ -103,6 +128,10 @@ class MenuList extends PureComponent {
       role: "menu",
       ref: this.setWrapperRef,
       onKeyDown: this.onKeyDown,
+      onMouseOver: this.onMouseOverOrFocus,
+      onMouseOut: this.onMouseOutOrBlur,
+      onFocus: this.onMouseOverOrFocus,
+      onBlur: this.onMouseOutOrBlur
     };
 
     if (this.props.id) {

devtools/client/themes/toolbox.css

@@ -347,6 +347,18 @@
   background-position: left 2px top 14px;
 }
 
+/* Tooltip of frames menu  */
+
+#command-button-frames-panel > .tooltip-panel {
+  max-width: 500px;
+  overflow-y: auto;
+}
+
+#toolbox-frame-menu span {
+  overflow: hidden;
+  text-overflow: ellipsis;
+}
+
 /* Toolbox panels */
 
 .toolbox-panel {

devtools/client/webaudioeditor/test/browser_wa_reset-04.js

@@ -22,14 +22,17 @@ add_task(async function() {
   ok(!btn.firstChild, "The frame list button has no children");
 
   // Open frame menu and wait till it's available on the screen.
-  const menu = await toolbox.showFramesMenu({target: btn});
-  await once(menu, "open");
+  const framePanel = toolbox.doc.getElementById("command-button-frames-panel");
+  btn.click();
+  ok(framePanel, "popup panel has created.");
+  await waitUntil(() => framePanel.classList.contains("tooltip-visible"));
 
-  const frames = menu.items;
-  is(frames.length, 2, "We have both frames in the list");
+  const menuList = toolbox.doc.getElementById("toolbox-frame-menu");
+  const buttonNodeList = menuList.querySelectorAll(".command");
+  is(buttonNodeList.length, 2, "We have both frames in the list");
 
   // Select the iframe
-  frames[1].click();
+  buttonNodeList[1].click();
 
   let navigating = once(target, "will-navigate");
 

dom/media/test/mochitest.ini

@@ -407,6 +407,7 @@ support-files =
   bug1301226-odd.wav^headers^
   bug1377278.webm
   bug1377278.webm^headers^
+  bunny.webm
   can_play_type_dash.js
   can_play_type_ogg.js
   can_play_type_wave.js
@@ -1136,6 +1137,7 @@ skip-if = android_version == '15' || android_version == '17' # android(bug 12323
 [test_seekToNextFrame.html]
 skip-if = toolkit == 'android' # bug 1329391, android(bug 1232305)
 tags=seektonextframe
+[test_seek_duration.html]
 [test_source.html]
 skip-if = android_version == '17' # android(bug 1232305)
 [test_source_null.html]

dom/media/test/test_seek_duration.html

@@ -0,0 +1,55 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Media test: seek tests</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <script type="text/javascript" src="manifest.js"></script>
+  <script type="text/javascript" src="seek_support.js"></script>
+</head>
+<body>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/**
+ * This test is used to make sure video's duration won't be changed when it
+ * reachs to the end after seeking to position where the time is very close to
+ * video's end time.
+ */
+
+SimpleTest.waitForExplicitFinish();
+
+(async function startTest()
+{
+  const video = document.createElement('video');
+  video.src = "bunny.webm";
+  document.body.appendChild(video);
+
+  const loadedMetadata = once(video, "loadedmetadata");
+  const canplay = once(video, "canplay");
+  const end = once(video, "ended");
+
+  info(`- wait for video loading metadata -`);
+  await loadedMetadata;
+  const originalDuration = video.duration;
+
+  info(`- seek video to the position which is close to end time -`);
+  // video's duration is 2.1 and the last key frame is in 2.0, we want to seek
+  // to that keyframe.
+  video.currentTime = originalDuration - 0.1;
+
+  info(`- play video until it ends -`);
+  await canplay;
+  await video.play();
+  await end;
+
+  ok(video.duration === originalDuration, `Duration shouldn't change`);
+  removeNodeAndSource(video);
+
+  SimpleTest.finish();
+})();
+
+</script>
+</pre>
+</body>
+</html>

dom/media/tests/mochitest/mochitest.ini

@@ -231,8 +231,12 @@ skip-if = toolkit == 'android'
 skip-if = toolkit == 'android'
 [test_peerConnection_restartIceLocalRollback.html]
 skip-if = toolkit == 'android'
+[test_peerConnection_restartIceLocalRollbackNoSubsequentRestart.html]
+skip-if = toolkit == 'android'
 [test_peerConnection_restartIceLocalAndRemoteRollback.html]
 skip-if = toolkit == 'android'
+[test_peerConnection_restartIceLocalAndRemoteRollbackNoSubsequentRestart.html]
+skip-if = toolkit == 'android'
 [test_peerConnection_restartIceBadAnswer.html]
 skip-if = toolkit == 'android'
 [test_peerConnection_scaleResolution.html]

dom/media/tests/mochitest/test_peerConnection_restartIceLocalAndRemoteRollback.html

@@ -64,13 +64,13 @@
               STABLE);
         },
 
-        // Rolling back should shut down gathering
+        // Rolling back should shut down gathering for the offerer,
+        // but because the answerer never set a local description, no ICE
+        // gathering has happened yet, so there's no changes to ICE gathering
+        // state
         function PC_LOCAL_WAIT_FOR_END_OF_TRICKLE(test) {
           return test.pcLocal.endOfTrickleIce;
         },
-        function PC_REMOTE_WAIT_FOR_END_OF_TRICKLE(test) {
-          return test.pcRemote.endOfTrickleIce;
-        },
 
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
           test.pcLocal.expectIceChecking();

dom/media/tests/mochitest/test_peerConnection_restartIceLocalAndRemoteRollbackNoSubsequentRestart.html

@@ -0,0 +1,86 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <script type="application/javascript" src="pc.js"></script>
+</head>
+<body>
+<pre id="test">
+<script type="application/javascript">
+  createHTML({
+    bug: "906986",
+    title: "Renegotiation: restart ice, local and remote rollback, without a subsequent ICE restart"
+  });
+
+  var test;
+  runNetworkTest(function (options) {
+    test = new PeerConnectionTest(options);
+
+    addRenegotiation(test.chain,
+      [
+        // causes a full, normal ice restart
+        function PC_LOCAL_SET_OFFER_OPTION(test) {
+          test.setOfferOptions({ iceRestart: true });
+        }
+      ]
+    );
+
+    test.chain.replaceAfter('PC_REMOTE_CREATE_ANSWER',
+      [
+        function PC_LOCAL_SETUP_ICE_HANDLER(test) {
+          test.pcLocal.setupIceCandidateHandler(test);
+          if (test.testOptions.steeplechase) {
+            test.pcLocal.endOfTrickleIce.then(() => {
+              send_message({"type": "end_of_trickle_ice"});
+            });
+          }
+        },
+        function PC_REMOTE_SETUP_ICE_HANDLER(test) {
+          test.pcRemote.setupIceCandidateHandler(test);
+          if (test.testOptions.steeplechase) {
+            test.pcRemote.endOfTrickleIce.then(() => {
+              send_message({"type": "end_of_trickle_ice"});
+            });
+          }
+        },
+
+        function PC_REMOTE_ROLLBACK(test) {
+          return test.setRemoteDescription(test.pcRemote, { type: "rollback" },
+                                           STABLE);
+        },
+
+        function PC_LOCAL_ROLLBACK(test) {
+          // We haven't negotiated the new stream yet.
+          test.pcLocal.expectNegotiationNeeded();
+          return test.setLocalDescription(
+              test.pcLocal,
+              new RTCSessionDescription({ type: "rollback", sdp: ""}),
+              STABLE);
+        },
+
+        // Rolling back should shut down gathering for the offerer,
+        // but because the answerer never set a local description, no ICE
+        // gathering has happened yet, so there's no changes to ICE gathering
+        // state
+        function PC_LOCAL_WAIT_FOR_END_OF_TRICKLE(test) {
+          return test.pcLocal.endOfTrickleIce;
+        },
+
+        function PC_LOCAL_SET_OFFER_OPTION(test) {
+          test.setOfferOptions({ iceRestart: false });
+        }
+      ],
+      1 // Replaces after second PC_REMOTE_CREATE_ANSWER
+    );
+    test.chain.append(commandsPeerConnectionOfferAnswer);
+
+    // for now, only use one stream, because rollback doesn't seem to
+    // like multiple streams.  See bug 1259465.
+    test.setMediaConstraints([{audio: true}],
+                             [{audio: true}]);
+    test.run();
+  });
+
+</script>
+</pre>
+</body>
+</html>

dom/media/tests/mochitest/test_peerConnection_restartIceLocalRollbackNoSubsequentRestart.html

@@ -0,0 +1,66 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <script type="application/javascript" src="pc.js"></script>
+</head>
+<body>
+<pre id="test">
+<script type="application/javascript">
+  createHTML({
+    bug: "906986",
+    title: "Renegotiation: restart ice, local rollback, then renegotiation without ICE restart"
+  });
+
+  var test;
+  runNetworkTest(function (options) {
+    test = new PeerConnectionTest(options);
+
+    addRenegotiation(test.chain,
+      [
+        // causes a full, normal ice restart
+        function PC_LOCAL_SET_OFFER_OPTION(test) {
+          test.setOfferOptions({ iceRestart: true });
+        },
+        // causes an ice restart and then rolls it back
+        // (does not result in sending an offer)
+        function PC_LOCAL_SETUP_ICE_HANDLER(test) {
+          test.pcLocal.setupIceCandidateHandler(test);
+          if (test.testOptions.steeplechase) {
+            test.pcLocal.endOfTrickleIce.then(() => {
+              send_message({"type": "end_of_trickle_ice"});
+            });
+          }
+        },
+        function PC_LOCAL_CREATE_AND_SET_OFFER(test) {
+          return test.createOffer(test.pcLocal).then(offer => {
+            return test.setLocalDescription(test.pcLocal,
+                                            offer,
+                                            HAVE_LOCAL_OFFER);
+          });
+        },
+        function PC_LOCAL_ROLLBACK(test) {
+          return test.setLocalDescription(test.pcLocal,
+                                          { type: "rollback", sdp: ""},
+                                          STABLE);
+        },
+        // Rolling back should shut down gathering
+        function PC_LOCAL_WAIT_FOR_END_OF_TRICKLE(test) {
+          return test.pcLocal.endOfTrickleIce;
+        },
+        function PC_LOCAL_SET_OFFER_OPTION(test) {
+          test.setOfferOptions({ iceRestart: false });
+        }
+      ]
+    );
+
+    // for now, only use one stream, because rollback doesn't seem to
+    // like multiple streams.  See bug 1259465.
+    test.setMediaConstraints([{audio: true}],
+                             [{audio: true}]);
+    test.run();
+  });
+
+</script>
+</pre>
+</body>
+</html>

dom/media/webm/WebMDemuxer.cpp

@@ -597,45 +597,41 @@ WebMDemuxer::GetNextPacket(TrackInfo::TrackType aType,
   // the timestamp of the next packet for this track.  If we've reached the
   // end of the resource, use the file's duration as the end time of this
   // video frame.
+  RefPtr<NesteggPacketHolder> next_holder;
+  rv = NextPacket(aType, next_holder);
+  if (NS_FAILED(rv) && rv != NS_ERROR_DOM_MEDIA_END_OF_STREAM) {
+    return rv;
+  }
+
   int64_t next_tstamp = INT64_MIN;
-  if (aType == TrackInfo::kAudioTrack) {
-    RefPtr<NesteggPacketHolder> next_holder;
-    rv = NextPacket(aType, next_holder);
-    if (NS_FAILED(rv) && rv != NS_ERROR_DOM_MEDIA_END_OF_STREAM) {
-      return rv;
-    }
-    if (next_holder) {
-      next_tstamp = next_holder->Timestamp();
-      PushAudioPacket(next_holder);
-    } else if (duration >= 0) {
-      next_tstamp = tstamp + duration;
-    } else if (!mIsMediaSource ||
-               (mIsMediaSource && mLastAudioFrameTime.isSome())) {
-      next_tstamp = tstamp;
-      next_tstamp += tstamp - mLastAudioFrameTime.refOr(0);
-    } else {
-      PushAudioPacket(holder);
-    }
-    mLastAudioFrameTime = Some(tstamp);
-  } else if (aType == TrackInfo::kVideoTrack) {
-    RefPtr<NesteggPacketHolder> next_holder;
-    rv = NextPacket(aType, next_holder);
-    if (NS_FAILED(rv) && rv != NS_ERROR_DOM_MEDIA_END_OF_STREAM) {
-      return rv;
-    }
-    if (next_holder) {
-      next_tstamp = next_holder->Timestamp();
-      PushVideoPacket(next_holder);
-    } else if (duration >= 0) {
-      next_tstamp = tstamp + duration;
-    } else if (!mIsMediaSource ||
-               (mIsMediaSource && mLastVideoFrameTime.isSome())) {
-      next_tstamp = tstamp;
-      next_tstamp += tstamp - mLastVideoFrameTime.refOr(0);
-    } else {
-      PushVideoPacket(holder);
-    }
-    mLastVideoFrameTime = Some(tstamp);
+  auto calculateNextTimestamp =
+    [&] (auto&& pushPacket, auto&& lastFrameTime, auto&& trackEndTime) {
+      if (next_holder) {
+        next_tstamp = next_holder->Timestamp();
+        (this->*pushPacket)(next_holder);
+      } else if (duration >= 0) {
+        next_tstamp = tstamp + duration;
+      } else if (lastFrameTime.isSome()) {
+        next_tstamp = tstamp + (tstamp - lastFrameTime.ref());
+      } else if (mIsMediaSource) {
+        (this->*pushPacket)(holder);
+      } else {
+        // If we can't get frame's duration, it means either we need to wait for
+        // more data for MSE case or this is the last frame for file resource case.
+        MOZ_ASSERT(trackEndTime >= tstamp);
+        next_tstamp = trackEndTime;
+      }
+      lastFrameTime = Some(tstamp);
+  };
+
+  if (aType == TrackInfo::kAudioTrack)  {
+    calculateNextTimestamp(&WebMDemuxer::PushAudioPacket,
+                           mLastAudioFrameTime,
+                           mInfo.mAudio.mDuration.ToMicroseconds());
+  } else {
+    calculateNextTimestamp(&WebMDemuxer::PushVideoPacket,
+                           mLastVideoFrameTime,
+                           mInfo.mVideo.mDuration.ToMicroseconds());
   }
 
   if (mIsMediaSource && next_tstamp == INT64_MIN) {
@@ -1154,7 +1150,7 @@ WebMTrackDemuxer::Seek(const TimeUnit& aTime)
 nsresult
 WebMTrackDemuxer::NextSample(RefPtr<MediaRawData>& aData)
 {
-  nsresult rv = NS_ERROR_DOM_MEDIA_END_OF_STREAM;;
+  nsresult rv = NS_ERROR_DOM_MEDIA_END_OF_STREAM;
   while (mSamples.GetSize() < 1 &&
          NS_SUCCEEDED((rv = mParent->GetNextPacket(mType, &mSamples)))) {
   }

dom/webauthn/u2f-hid-rs/.travis.yml

@@ -20,7 +20,10 @@ before_install:
   - pkg-config --list-all
   - pkg-config --libs libudev
   - pkg-config --modversion libudev
-  - cargo install rustfmt || true
+
+install:
+  - rustup install nightly
+  - rustup component add --toolchain nightly rustfmt-preview
 
 script:
 - |
@@ -28,7 +31,6 @@ script:
     export ASAN_OPTIONS="detect_odr_violation=1:leak_check_at_exit=0:detect_leaks=0"
     export RUSTFLAGS="-Z sanitizer=address"
   fi
-- |
-  cargo fmt -- --write-mode=diff &&
-  cargo build &&
-  cargo test
+- cargo +nightly fmt --all -- --check
+- cargo build
+- cargo test

dom/webauthn/u2f-hid-rs/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "u2fhid"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Kyle Machulis <kyle@nonpolynomial.com>", "J.C. Jones <jc@mozilla.com>", "Tim Taubert <ttaubert@mozilla.com>"]
 
 [target.'cfg(target_os = "linux")'.dependencies]
@@ -13,8 +13,15 @@ devd-rs = "0.2.1"
 core-foundation-sys = "0.6.0"
 core-foundation = "0.6.0"
 
-[target.'cfg(target_os = "windows")'.dependencies]
-winapi = "0.2.8"
+[target.'cfg(target_os = "windows")'.dependencies.winapi]
+version = "0.3"
+features = [
+    "handleapi",
+    "hidclass",
+    "hidpi",
+    "hidusage",
+    "setupapi",
+]
 
 [dependencies]
 rand = "0.3"

dom/webauthn/u2f-hid-rs/examples/main.rs

@@ -69,8 +69,7 @@ fn main() {
             move |rv| {
                 tx.send(rv.unwrap()).unwrap();
             },
-        )
-        .unwrap();
+        ).unwrap();
 
     let register_data = try_or!(rx.recv(), |_| {
         panic!("Problem receiving, unable to continue");
@@ -95,8 +94,7 @@ fn main() {
             move |rv| {
                 tx.send(rv.unwrap()).unwrap();
             },
-        )
-        .unwrap();
+        ).unwrap();
 
     let (_, handle_used, sign_data) = try_or!(rx.recv(), |_| {
         println!("Problem receiving");

dom/webauthn/u2f-hid-rs/src/freebsd/monitor.rs

@@ -24,7 +24,8 @@ impl Event {
                 ref dev,
                 parent: _,
                 location: _,
-            } if dev.starts_with("uhid") =>
+            }
+                if dev.starts_with("uhid") =>
             {
                 Some(Event::Add(("/dev/".to_owned() + dev).into()))
             }
@@ -32,7 +33,8 @@ impl Event {
                 ref dev,
                 parent: _,
                 location: _,
-            } if dev.starts_with("uhid") =>
+            }
+                if dev.starts_with("uhid") =>
             {
                 Some(Event::Remove(("/dev/".to_owned() + dev).into()))
             }

dom/webauthn/u2f-hid-rs/src/hidproto.rs

@@ -6,7 +6,7 @@
 
 use std::mem;
 
-use consts::{FIDO_USAGE_U2FHID, FIDO_USAGE_PAGE};
+use consts::{FIDO_USAGE_PAGE, FIDO_USAGE_U2FHID};
 
 // The 4 MSBs (the tag) are set when it's a long item.
 const HID_MASK_LONG_ITEM_TAG: u8 = 0b11110000;

dom/webauthn/u2f-hid-rs/src/lib.rs

@@ -36,11 +36,12 @@ pub mod platform;
 #[path = "windows/mod.rs"]
 pub mod platform;
 
-#[cfg(
-    not(
-        any(target_os = "linux", target_os = "freebsd", target_os = "macos", target_os = "windows")
-    )
-)]
+#[cfg(not(any(
+    target_os = "linux",
+    target_os = "freebsd",
+    target_os = "macos",
+    target_os = "windows"
+)))]
 #[path = "stub/mod.rs"]
 pub mod platform;
 

dom/webauthn/u2f-hid-rs/src/macos/iokit.rs

@@ -7,7 +7,7 @@
 extern crate core_foundation_sys;
 extern crate libc;
 
-use consts::{FIDO_USAGE_U2FHID, FIDO_USAGE_PAGE};
+use consts::{FIDO_USAGE_PAGE, FIDO_USAGE_U2FHID};
 use core_foundation::dictionary::*;
 use core_foundation::number::*;
 use core_foundation::string::*;

dom/webauthn/u2f-hid-rs/src/macos/transaction.rs

@@ -57,8 +57,7 @@ impl Transaction {
 
                 // Send an error, if the callback wasn't called already.
                 callback.call(Err(::Error::NotAllowed));
-            })
-            .map_err(|_| ::Error::Unknown)?;
+            }).map_err(|_| ::Error::Unknown)?;
 
         // Block until we enter the CFRunLoop.
         let runloop = rx.recv().map_err(|_| ::Error::Unknown)?;

dom/webauthn/u2f-hid-rs/src/statemachine.rs

@@ -90,9 +90,12 @@ impl StateMachine {
             // If so, we'll keep polling the device anyway to test for user
             // consent, to be consistent with CTAP2 device behavior.
             let excluded = key_handles.iter().any(|key_handle| {
-                is_valid_transport(key_handle.transports)
-                    && u2f_is_keyhandle_valid(dev, &challenge, &application, &key_handle.credential)
-                        .unwrap_or(false) /* no match on failure */
+                is_valid_transport(key_handle.transports) && u2f_is_keyhandle_valid(
+                    dev,
+                    &challenge,
+                    &application,
+                    &key_handle.credential,
+                ).unwrap_or(false) /* no match on failure */
             });
 
             while alive() {

dom/webauthn/u2f-hid-rs/src/u2fprotocol.rs

@@ -215,7 +215,7 @@ mod tests {
     use rand::{thread_rng, Rng};
 
     use super::{init_device, send_apdu, sendrecv, U2FDevice};
-    use consts::{U2FHID_INIT, U2FHID_MSG, U2FHID_PING, CID_BROADCAST, SW_NO_ERROR};
+    use consts::{CID_BROADCAST, SW_NO_ERROR, U2FHID_INIT, U2FHID_MSG, U2FHID_PING};
 
     mod platform {
         use std::io;

dom/webauthn/u2f-hid-rs/src/windows/device.rs

@@ -8,7 +8,7 @@ use std::io::{Read, Write};
 use std::os::windows::io::AsRawHandle;
 
 use super::winapi::DeviceCapabilities;
-use consts::{FIDO_USAGE_U2FHID, CID_BROADCAST, FIDO_USAGE_PAGE, HID_RPT_SIZE};
+use consts::{CID_BROADCAST, FIDO_USAGE_PAGE, FIDO_USAGE_U2FHID, HID_RPT_SIZE};
 
 use u2ftypes::U2FDevice;
 

dom/webauthn/u2f-hid-rs/src/windows/winapi.rs

@@ -14,47 +14,53 @@ use util::io_err;
 
 extern crate libc;
 extern crate winapi;
-use self::winapi::*;
+
+use platform::winapi::winapi::shared::{guiddef, minwindef, ntdef, windef};
+use platform::winapi::winapi::shared::{hidclass, hidpi, hidusage};
+use platform::winapi::winapi::um::{handleapi, setupapi};
 
 #[link(name = "setupapi")]
 extern "stdcall" {
     fn SetupDiGetClassDevsW(
-        ClassGuid: *const GUID,
-        Enumerator: PCSTR,
-        hwndParent: HWND,
-        flags: DWORD,
-    ) -> HDEVINFO;
+        ClassGuid: *const guiddef::GUID,
+        Enumerator: ntdef::PCSTR,
+        hwndParent: windef::HWND,
+        flags: minwindef::DWORD,
+    ) -> setupapi::HDEVINFO;
 
-    fn SetupDiDestroyDeviceInfoList(DeviceInfoSet: HDEVINFO) -> BOOL;
+    fn SetupDiDestroyDeviceInfoList(DeviceInfoSet: setupapi::HDEVINFO) -> minwindef::BOOL;
 
     fn SetupDiEnumDeviceInterfaces(
-        DeviceInfoSet: HDEVINFO,
-        DeviceInfoData: PSP_DEVINFO_DATA,
-        InterfaceClassGuid: *const GUID,
-        MemberIndex: DWORD,
-        DeviceInterfaceData: PSP_DEVICE_INTERFACE_DATA,
-    ) -> BOOL;
+        DeviceInfoSet: setupapi::HDEVINFO,
+        DeviceInfoData: setupapi::PSP_DEVINFO_DATA,
+        InterfaceClassGuid: *const guiddef::GUID,
+        MemberIndex: minwindef::DWORD,
+        DeviceInterfaceData: setupapi::PSP_DEVICE_INTERFACE_DATA,
+    ) -> minwindef::BOOL;
 
     fn SetupDiGetDeviceInterfaceDetailW(
-        DeviceInfoSet: HDEVINFO,
-        DeviceInterfaceData: PSP_DEVICE_INTERFACE_DATA,
-        DeviceInterfaceDetailData: PSP_DEVICE_INTERFACE_DETAIL_DATA_W,
-        DeviceInterfaceDetailDataSize: DWORD,
-        RequiredSize: PDWORD,
-        DeviceInfoData: PSP_DEVINFO_DATA,
-    ) -> BOOL;
+        DeviceInfoSet: setupapi::HDEVINFO,
+        DeviceInterfaceData: setupapi::PSP_DEVICE_INTERFACE_DATA,
+        DeviceInterfaceDetailData: setupapi::PSP_DEVICE_INTERFACE_DETAIL_DATA_W,
+        DeviceInterfaceDetailDataSize: minwindef::DWORD,
+        RequiredSize: minwindef::PDWORD,
+        DeviceInfoData: setupapi::PSP_DEVINFO_DATA,
+    ) -> minwindef::BOOL;
 }
 
 #[link(name = "hid")]
 extern "stdcall" {
     fn HidD_GetPreparsedData(
-        HidDeviceObject: HANDLE,
-        PreparsedData: *mut PHIDP_PREPARSED_DATA,
-    ) -> BOOLEAN;
+        HidDeviceObject: ntdef::HANDLE,
+        PreparsedData: *mut hidpi::PHIDP_PREPARSED_DATA,
+    ) -> ntdef::BOOLEAN;
 
-    fn HidD_FreePreparsedData(PreparsedData: PHIDP_PREPARSED_DATA) -> BOOLEAN;
+    fn HidD_FreePreparsedData(PreparsedData: hidpi::PHIDP_PREPARSED_DATA) -> ntdef::BOOLEAN;
 
-    fn HidP_GetCaps(PreparsedData: PHIDP_PREPARSED_DATA, Capabilities: PHIDP_CAPS) -> NTSTATUS;
+    fn HidP_GetCaps(
+        PreparsedData: hidpi::PHIDP_PREPARSED_DATA,
+        Capabilities: hidpi::PHIDP_CAPS,
+    ) -> ntdef::NTSTATUS;
 }
 
 macro_rules! offset_of {
@@ -70,28 +76,28 @@ fn from_wide_ptr(ptr: *const u16, len: usize) -> String {
 }
 
 pub struct DeviceInfoSet {
-    set: HDEVINFO,
+    set: setupapi::HDEVINFO,
 }
 
 impl DeviceInfoSet {
     pub fn new() -> io::Result<Self> {
-        let flags = DIGCF_PRESENT | DIGCF_DEVICEINTERFACE;
+        let flags = setupapi::DIGCF_PRESENT | setupapi::DIGCF_DEVICEINTERFACE;
         let set = unsafe {
             SetupDiGetClassDevsW(
-                &GUID_DEVINTERFACE_HID,
+                &hidclass::GUID_DEVINTERFACE_HID,
                 ptr::null_mut(),
                 ptr::null_mut(),
                 flags,
             )
         };
-        if set == INVALID_HANDLE_VALUE {
+        if set == handleapi::INVALID_HANDLE_VALUE {
             return Err(io_err("SetupDiGetClassDevsW failed!"));
         }
 
         Ok(Self { set })
     }
 
-    pub fn get(&self) -> HDEVINFO {
+    pub fn get(&self) -> setupapi::HDEVINFO {
         self.set
     }
 
@@ -108,7 +114,7 @@ impl Drop for DeviceInfoSet {
 
 pub struct DeviceInfoSetIter<'a> {
     set: &'a DeviceInfoSet,
-    index: DWORD,
+    index: minwindef::DWORD,
 }
 
 impl<'a> DeviceInfoSetIter<'a> {
@@ -121,14 +127,16 @@ impl<'a> Iterator for DeviceInfoSetIter<'a> {
     type Item = String;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let mut device_interface_data = unsafe { mem::uninitialized::<SP_DEVICE_INTERFACE_DATA>() };
-        device_interface_data.cbSize = mem::size_of::<SP_DEVICE_INTERFACE_DATA>() as UINT;
+        let mut device_interface_data =
+            unsafe { mem::uninitialized::<setupapi::SP_DEVICE_INTERFACE_DATA>() };
+        device_interface_data.cbSize =
+            mem::size_of::<setupapi::SP_DEVICE_INTERFACE_DATA>() as minwindef::UINT;
 
         let rv = unsafe {
             SetupDiEnumDeviceInterfaces(
                 self.set.get(),
                 ptr::null_mut(),
-                &GUID_DEVINTERFACE_HID,
+                &hidclass::GUID_DEVINTERFACE_HID,
                 self.index,
                 &mut device_interface_data,
             )
@@ -179,13 +187,13 @@ impl<'a> Iterator for DeviceInfoSetIter<'a> {
 }
 
 struct DeviceInterfaceDetailData {
-    data: PSP_DEVICE_INTERFACE_DETAIL_DATA_W,
+    data: setupapi::PSP_DEVICE_INTERFACE_DETAIL_DATA_W,
     path_len: usize,
 }
 
 impl DeviceInterfaceDetailData {
     fn new(size: usize) -> Option<Self> {
-        let mut cb_size = mem::size_of::<SP_DEVICE_INTERFACE_DETAIL_DATA_W>();
+        let mut cb_size = mem::size_of::<setupapi::SP_DEVICE_INTERFACE_DETAIL_DATA_W>();
         if cfg!(target_pointer_width = "32") {
             cb_size = 4 + 2; // 4-byte uint + default TCHAR size. size_of is inaccurate.
         }
@@ -195,16 +203,16 @@ impl DeviceInterfaceDetailData {
             return None;
         }
 
-        let data = unsafe { libc::malloc(size) as PSP_DEVICE_INTERFACE_DETAIL_DATA_W };
+        let data = unsafe { libc::malloc(size) as setupapi::PSP_DEVICE_INTERFACE_DETAIL_DATA_W };
         if data.is_null() {
             return None;
         }
 
         // Set total size of the structure.
-        unsafe { (*data).cbSize = cb_size as UINT };
+        unsafe { (*data).cbSize = cb_size as minwindef::UINT };
 
         // Compute offset of `SP_DEVICE_INTERFACE_DETAIL_DATA_W.DevicePath`.
-        let offset = offset_of!(SP_DEVICE_INTERFACE_DETAIL_DATA_W, DevicePath);
+        let offset = offset_of!(setupapi::SP_DEVICE_INTERFACE_DETAIL_DATA_W, DevicePath);
 
         Some(Self {
             data,
@@ -212,7 +220,7 @@ impl DeviceInterfaceDetailData {
         })
     }
 
-    fn get(&self) -> PSP_DEVICE_INTERFACE_DETAIL_DATA_W {
+    fn get(&self) -> setupapi::PSP_DEVICE_INTERFACE_DETAIL_DATA_W {
         self.data
     }
 
@@ -228,24 +236,24 @@ impl Drop for DeviceInterfaceDetailData {
 }
 
 pub struct DeviceCapabilities {
-    caps: HIDP_CAPS,
+    caps: hidpi::HIDP_CAPS,
 }
 
 impl DeviceCapabilities {
-    pub fn new(handle: HANDLE) -> io::Result<Self> {
+    pub fn new(handle: ntdef::HANDLE) -> io::Result<Self> {
         let mut preparsed_data = ptr::null_mut();
         let rv = unsafe { HidD_GetPreparsedData(handle, &mut preparsed_data) };
         if rv == 0 || preparsed_data.is_null() {
             return Err(io_err("HidD_GetPreparsedData failed!"));
         }
 
-        let mut caps: HIDP_CAPS = unsafe { mem::uninitialized() };
+        let mut caps: hidpi::HIDP_CAPS = unsafe { mem::uninitialized() };
 
         unsafe {
             let rv = HidP_GetCaps(preparsed_data, &mut caps);
             HidD_FreePreparsedData(preparsed_data);
 
-            if rv != HIDP_STATUS_SUCCESS {
+            if rv != hidpi::HIDP_STATUS_SUCCESS {
                 return Err(io_err("HidP_GetCaps failed!"));
             }
         }
@@ -253,11 +261,11 @@ impl DeviceCapabilities {
         Ok(Self { caps })
     }
 
-    pub fn usage(&self) -> USAGE {
+    pub fn usage(&self) -> hidusage::USAGE {
         self.caps.Usage
     }
 
-    pub fn usage_page(&self) -> USAGE {
+    pub fn usage_page(&self) -> hidusage::USAGE {
         self.caps.UsagePage
     }
 }

gfx/layers/wr/WebRenderCommandBuilder.cpp

@@ -1185,14 +1185,26 @@ WebRenderCommandBuilder::DoGroupingForDisplayList(nsDisplayList* aList,
       group.mAppUnitsPerDevPixel != appUnitsPerDevPixel ||
       group.mScale != scale ||
       group.mResidualOffset != residualOffset) {
+    GP("Property change. Deleting blob\n");
+
     if (group.mAppUnitsPerDevPixel != appUnitsPerDevPixel) {
-      GP("app unit %d %d\n", group.mAppUnitsPerDevPixel, appUnitsPerDevPixel);
+      GP(" app unit change%d %d\n", group.mAppUnitsPerDevPixel, appUnitsPerDevPixel);
     }
     // The bounds have changed so we need to discard the old image and add all
     // the commands again.
     auto p = group.mGroupBounds;
     auto q = groupBounds;
-    GP("Bounds change: %d %d %d %d vs %d %d %d %d\n", p.x, p.y, p.width, p.height, q.x, q.y, q.width, q.height);
+    if (!group.mGroupBounds.IsEqualEdges(groupBounds)) {
+      GP(" Bounds change: %d %d %d %d vs %d %d %d %d\n", p.x, p.y, p.width, p.height, q.x, q.y, q.width, q.height);
+    }
+
+    if (group.mScale != scale) {
+      GP(" Scale %f %f vs %f %f\n", group.mScale.width, group.mScale.height, scale.width, scale.height);
+    }
+
+    if (group.mResidualOffset != residualOffset) {
+      GP(" Residual Offset %f %f vs %f %f\n", group.mResidualOffset.x, group.mResidualOffset.y, residualOffset.x, residualOffset.y);
+    }
 
     group.ClearItems();
     group.ClearImageKey(mManager);

media/mtransport/common.build

@@ -10,7 +10,6 @@ mtransport_lcppsrcs = [
     'nr_socket_prsock.cpp',
     'nr_timer.cpp',
     'nricectx.cpp',
-    'nricectxhandler.cpp',
     'nricemediastream.cpp',
     'nriceresolver.cpp',
     'nriceresolverfake.cpp',

media/mtransport/nricectx.cpp

@@ -291,18 +291,68 @@ NrIceCtx::NrIceCtx(const std::string& name, Policy policy)
     nat_ (nullptr) {
 }
 
+/* static */
+RefPtr<NrIceCtx>
+NrIceCtx::Create(const std::string& name,
+                 bool allow_loopback,
+                 bool tcp_enabled,
+                 bool allow_link_local,
+                 Policy policy)
+{
+  // InitializeGlobals only executes once
+  NrIceCtx::InitializeGlobals(allow_loopback, tcp_enabled, allow_link_local);
+
+  RefPtr<NrIceCtx> ctx = new NrIceCtx(name, policy);
+
+  if (!ctx->Initialize()) {
+    return nullptr;
+  }
+
+  return ctx;
+}
+
+RefPtr<NrIceMediaStream>
+NrIceCtx::CreateStream(const std::string& id,
+                       const std::string& name,
+                       int components)
+{
+  if (streams_.count(id)) {
+    MOZ_ASSERT(false);
+    return nullptr;
+  }
+
+  RefPtr<NrIceMediaStream> stream =
+    new NrIceMediaStream(this, id, name, components);
+  streams_[id] = stream;
+  return stream;
+}
+
+void
+NrIceCtx::DestroyStream(const std::string& id) {
+  auto it = streams_.find(id);
+  if (it != streams_.end()) {
+    auto preexisting_stream = it->second;
+    streams_.erase(it);
+    preexisting_stream->Close();
+  }
+}
+
 // Handler callbacks
 int NrIceCtx::select_pair(void *obj,nr_ice_media_stream *stream,
                    int component_id, nr_ice_cand_pair **potentials,
                    int potential_ct) {
   MOZ_MTLOG(ML_DEBUG, "select pair called: potential_ct = "
             << potential_ct);
+  MOZ_ASSERT(stream->local_stream);
+  MOZ_ASSERT(!stream->local_stream->obsolete);
 
   return 0;
 }
 
 int NrIceCtx::stream_ready(void *obj, nr_ice_media_stream *stream) {
   MOZ_MTLOG(ML_DEBUG, "stream_ready called");
+  MOZ_ASSERT(!stream->local_stream);
+  MOZ_ASSERT(!stream->obsolete);
 
   // Get the ICE ctx.
   NrIceCtx *ctx = static_cast<NrIceCtx *>(obj);
@@ -319,6 +369,8 @@ int NrIceCtx::stream_ready(void *obj, nr_ice_media_stream *stream) {
 
 int NrIceCtx::stream_failed(void *obj, nr_ice_media_stream *stream) {
   MOZ_MTLOG(ML_DEBUG, "stream_failed called");
+  MOZ_ASSERT(!stream->local_stream);
+  MOZ_ASSERT(!stream->obsolete);
 
   // Get the ICE ctx
   NrIceCtx *ctx = static_cast<NrIceCtx *>(obj);
@@ -328,7 +380,7 @@ int NrIceCtx::stream_failed(void *obj, nr_ice_media_stream *stream) {
   MOZ_ASSERT(s);
 
   ctx->SetConnectionState(ICE_CTX_FAILED);
-  s -> SignalFailed(s);
+  s -> Failed();
   return 0;
 }
 
@@ -387,6 +439,7 @@ void NrIceCtx::trickle_cb(void *arg, nr_ice_ctx *ice_ctx,
                           nr_ice_media_stream *stream,
                           int component_id,
                           nr_ice_candidate *candidate) {
+  MOZ_ASSERT(!stream->obsolete);
   // Get the ICE ctx
   NrIceCtx *ctx = static_cast<NrIceCtx *>(arg);
   RefPtr<NrIceMediaStream> s = ctx->FindStream(stream);
@@ -488,40 +541,6 @@ NrIceCtx::InitializeGlobals(bool allow_loopback,
   }
 }
 
-std::string
-NrIceCtx::GetNewUfrag()
-{
-  char* ufrag;
-  int r;
-
-  if ((r=nr_ice_get_new_ice_ufrag(&ufrag))) {
-    MOZ_CRASH("Unable to get new ice ufrag");
-    return "";
-  }
-
-  std::string ufragStr = ufrag;
-  RFREE(ufrag);
-
-  return ufragStr;
-}
-
-std::string
-NrIceCtx::GetNewPwd()
-{
-  char* pwd;
-  int r;
-
-  if ((r=nr_ice_get_new_ice_pwd(&pwd))) {
-    MOZ_CRASH("Unable to get new ice pwd");
-    return "";
-  }
-
-  std::string pwdStr = pwd;
-  RFREE(pwd);
-
-  return pwdStr;
-}
-
 #define MAXADDRS 100 // mirrors setting in ice_ctx.c
 
 /* static */
@@ -570,22 +589,6 @@ NrIceCtx::SetStunAddrs(const nsTArray<NrIceStunAddr>& addrs)
 bool
 NrIceCtx::Initialize()
 {
-  std::string ufrag = GetNewUfrag();
-  std::string pwd = GetNewPwd();
-
-  return Initialize(ufrag, pwd);
-}
-
-bool
-NrIceCtx::Initialize(const std::string& ufrag,
-                     const std::string& pwd)
-{
-  MOZ_ASSERT(!ufrag.empty());
-  MOZ_ASSERT(!pwd.empty());
-  if (ufrag.empty() || pwd.empty()) {
-    return false;
-  }
-
   // Create the ICE context
   int r;
 
@@ -601,13 +604,7 @@ NrIceCtx::Initialize(const std::string& ufrag,
       break;
   }
 
-  r = nr_ice_ctx_create_with_credentials(const_cast<char *>(name_.c_str()),
-                                         flags,
-                                         const_cast<char *>(ufrag.c_str()),
-                                         const_cast<char *>(pwd.c_str()),
-                                         &ctx_);
-  MOZ_ASSERT(ufrag == ctx_->ufrag);
-  MOZ_ASSERT(pwd == ctx_->pwd);
+  r = nr_ice_ctx_create(const_cast<char *>(name_.c_str()), flags, &ctx_);
 
   if (r) {
     MOZ_MTLOG(ML_ERROR, "Couldn't create ICE ctx for '" << name_ << "'");
@@ -792,29 +789,6 @@ NrIceCtx::~NrIceCtx() {
   Destroy();
 }
 
-void
-NrIceCtx::SetStream(const std::string& id, NrIceMediaStream* stream) {
-  auto it = streams_.find(id);
-  if (it != streams_.end()) {
-    MOZ_ASSERT(!stream, "Transport ids should be unique, and set only once");
-    auto preexisting_stream = it->second;
-    streams_.erase(it);
-    preexisting_stream->Close();
-  }
-
-  if (stream) {
-    streams_[id] = stream;
-  }
-}
-
-std::string NrIceCtx::ufrag() const {
-  return ctx_->ufrag;
-}
-
-std::string NrIceCtx::pwd() const {
-  return ctx_->pwd;
-}
-
 void NrIceCtx::destroy_peer_ctx() {
   nr_ice_peer_ctx_destroy(&peer_);
 }
@@ -992,7 +966,7 @@ nsresult NrIceCtx::StartGathering(bool default_route_only, bool proxy_only) {
 
 RefPtr<NrIceMediaStream> NrIceCtx::FindStream(nr_ice_media_stream *stream) {
   for (auto& idAndStream : streams_) {
-    if (idAndStream.second->stream() == stream) {
+    if (idAndStream.second->HasStream(stream)) {
       return idAndStream.second;
     }
   }

media/mtransport/nricectx.h

@@ -68,6 +68,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "m_cpp_utils.h"
 #include "nricestunaddr.h"
+#include "nricemediastream.h"
 
 typedef struct nr_ice_ctx_ nr_ice_ctx;
 typedef struct nr_ice_peer_ctx_ nr_ice_peer_ctx;
@@ -204,7 +205,6 @@ class NrIceStats {
 };
 
 class NrIceCtx {
- friend class NrIceCtxHandler;
  public:
   enum ConnectionState { ICE_CTX_INIT,
                          ICE_CTX_CHECKING,
@@ -229,12 +229,22 @@ class NrIceCtx {
                 ICE_POLICY_ALL
   };
 
+  static RefPtr<NrIceCtx> Create(const std::string& name,
+                                 bool allow_loopback = false,
+                                 bool tcp_enabled = true,
+                                 bool allow_link_local = false,
+                                 NrIceCtx::Policy policy =
+                                   NrIceCtx::ICE_POLICY_ALL);
+
+  RefPtr<NrIceMediaStream> CreateStream(const std::string& id,
+                                        const std::string& name,
+                                        int components);
+  void DestroyStream(const std::string& id);
+
   // initialize ICE globals, crypto, and logging
   static void InitializeGlobals(bool allow_loopback = false,
                                 bool tcp_enabled = true,
                                 bool allow_link_local = false);
-  static std::string GetNewUfrag();
-  static std::string GetNewPwd();
 
   // static GetStunAddrs for use in parent process to support
   // sandboxing restrictions
@@ -242,7 +252,6 @@ class NrIceCtx {
   void SetStunAddrs(const nsTArray<NrIceStunAddr>& addrs);
 
   bool Initialize();
-  bool Initialize(const std::string& ufrag, const std::string& pwd);
 
   int SetNat(const RefPtr<TestNat>& aNat);
 
@@ -258,8 +267,6 @@ class NrIceCtx {
   // Testing only.
   void destroy_peer_ctx();
 
-  void SetStream(const std::string& id, NrIceMediaStream* stream);
-
   RefPtr<NrIceMediaStream> GetStream(const std::string& id) {
     auto it = streams_.find(id);
     if (it != streams_.end()) {
@@ -281,10 +288,6 @@ class NrIceCtx {
   // The name of the ctx
   const std::string& name() const { return name_; }
 
-  // Get ufrag and password.
-  std::string ufrag() const;
-  std::string pwd() const;
-
   // Current state
   ConnectionState connection_state() const {
     return connection_state_;

media/mtransport/nricectxhandler.cpp

@@ -1,188 +0,0 @@
-#include <sstream>
-
-#include "logging.h"
-
-// nICEr includes
-extern "C" {
-#include "nr_api.h"
-#include "ice_ctx.h"
-}
-
-// Local includes
-#include "nricectxhandler.h"
-#include "nricemediastream.h"
-#include "nriceresolver.h"
-
-namespace mozilla {
-
-MOZ_MTLOG_MODULE("mtransport")
-
-NrIceCtxHandler::NrIceCtxHandler(const std::string& name,
-                                 NrIceCtx::Policy policy)
-  : current_ctx(new NrIceCtx(name, policy)),
-    old_ctx(nullptr),
-    restart_count(0)
-{
-}
-
-RefPtr<NrIceCtxHandler>
-NrIceCtxHandler::Create(const std::string& name,
-                        bool allow_loopback,
-                        bool tcp_enabled,
-                        bool allow_link_local,
-                        NrIceCtx::Policy policy)
-{
-  // InitializeGlobals only executes once
-  NrIceCtx::InitializeGlobals(allow_loopback, tcp_enabled, allow_link_local);
-
-  RefPtr<NrIceCtxHandler> ctx = new NrIceCtxHandler(name, policy);
-
-  if (ctx == nullptr ||
-      ctx->current_ctx == nullptr ||
-      !ctx->current_ctx->Initialize()) {
-    return nullptr;
-  }
-
-  return ctx;
-}
-
-
-RefPtr<NrIceMediaStream>
-NrIceCtxHandler::CreateStream(const std::string& name, int components)
-{
-  // To make tracking NrIceMediaStreams easier during ICE restart
-  // prepend an int to the name that increments with each ICE restart
-  std::ostringstream os;
-  os << restart_count << "-" << name;
-  return NrIceMediaStream::Create(this->current_ctx, os.str(), components);
-}
-
-
-RefPtr<NrIceCtx>
-NrIceCtxHandler::CreateCtx() const
-{
-  return CreateCtx(NrIceCtx::GetNewUfrag(), NrIceCtx::GetNewPwd());
-}
-
-
-RefPtr<NrIceCtx>
-NrIceCtxHandler::CreateCtx(const std::string& ufrag,
-                           const std::string& pwd) const
-{
-  RefPtr<NrIceCtx> new_ctx = new NrIceCtx(this->current_ctx->name(),
-                                          this->current_ctx->policy());
-  if (new_ctx == nullptr) {
-    return nullptr;
-  }
-
-  if (!new_ctx->Initialize(ufrag, pwd)) {
-    return nullptr;
-  }
-
-  // copy the stun, and turn servers from the current context
-  int r = nr_ice_ctx_set_stun_servers(new_ctx->ctx_,
-                                      this->current_ctx->ctx_->stun_servers,
-                                      this->current_ctx->ctx_->stun_server_ct);
-  if (r) {
-    MOZ_MTLOG(ML_ERROR, "Error while setting STUN servers in CreateCtx"
-                        << " (likely ice restart related)");
-    return nullptr;
-  }
-
-  r = nr_ice_ctx_copy_turn_servers(new_ctx->ctx_,
-                                   this->current_ctx->ctx_->turn_servers,
-                                   this->current_ctx->ctx_->turn_server_ct);
-  if (r) {
-    MOZ_MTLOG(ML_ERROR, "Error while copying TURN servers in CreateCtx"
-                        << " (likely ice restart related)");
-    return nullptr;
-  }
-
-  // grab the NrIceResolver stashed in the nr_resolver and allocate another
-  // for the new ctx.  Note: there may not be an nr_resolver.
-  if (this->current_ctx->ctx_->resolver) {
-    NrIceResolver* resolver =
-      static_cast<NrIceResolver*>(this->current_ctx->ctx_->resolver->obj);
-    if (!resolver ||
-        NS_FAILED(new_ctx->SetResolver(resolver->AllocateResolver()))) {
-      MOZ_MTLOG(ML_ERROR, "Error while setting dns resolver in CreateCtx"
-                          << " (likely ice restart related)");
-      return nullptr;
-    }
-  }
-
-  return new_ctx;
-}
-
-
-bool
-NrIceCtxHandler::BeginIceRestart(RefPtr<NrIceCtx> new_ctx)
-{
-  MOZ_ASSERT(!old_ctx, "existing ice restart in progress");
-  if (old_ctx) {
-    MOZ_MTLOG(ML_ERROR, "Existing ice restart in progress");
-    return false; // ice restart already in progress
-  }
-
-  if (new_ctx == nullptr) {
-    return false;
-  }
-
-  ++restart_count;
-  old_ctx = current_ctx;
-  current_ctx = new_ctx;
-  return true;
-}
-
-
-void
-NrIceCtxHandler::FinalizeIceRestart()
-{
-  if (old_ctx) {
-    // Fixup the telemetry by transferring old stats to current ctx.
-    NrIceStats stats = old_ctx->Destroy();
-    current_ctx->AccumulateStats(stats);
-  }
-
-  // no harm calling this even if we're not in the middle of restarting
-  old_ctx = nullptr;
-}
-
-
-void
-NrIceCtxHandler::RollbackIceRestart()
-{
-  if (old_ctx == nullptr) {
-    return;
-  }
-  current_ctx = old_ctx;
-  old_ctx = nullptr;
-}
-
-NrIceStats NrIceCtxHandler::Destroy()
-{
-  NrIceStats stats;
-
-  // designed to be called more than once so if stats are desired, this can be
-  // called just prior to the destructor
-  if (old_ctx && current_ctx) {
-    stats = old_ctx->Destroy();
-    current_ctx->AccumulateStats(stats);
-  }
-
-  if (current_ctx) {
-    stats = current_ctx->Destroy();
-  }
-
-  old_ctx = nullptr;
-  current_ctx = nullptr;
-
-  return stats;
-}
-
-NrIceCtxHandler::~NrIceCtxHandler()
-{
-  Destroy();
-}
-
-} // close namespace

media/mtransport/nricectxhandler.h

@@ -1,50 +0,0 @@
-#ifndef nricectxhandler_h__
-#define nricectxhandler_h__
-
-#include "nricectx.h"
-
-namespace mozilla {
-
-class NrIceCtxHandler {
-public:
-  // TODO(ekr@rtfm.com): Too many bools here. Bug 1193437.
-  static RefPtr<NrIceCtxHandler> Create(const std::string& name,
-                                        bool allow_loopback = false,
-                                        bool tcp_enabled = true,
-                                        bool allow_link_local = false,
-                                        NrIceCtx::Policy policy =
-                                          NrIceCtx::ICE_POLICY_ALL);
-
-  RefPtr<NrIceMediaStream> CreateStream(const std::string& name,
-                                        int components);
-  // CreateCtx is necessary so we can create and initialize the context
-  // on main thread, but begin the ice restart mechanics on STS thread
-  RefPtr<NrIceCtx> CreateCtx() const; // for test
-  RefPtr<NrIceCtx> CreateCtx(const std::string& ufrag,
-                             const std::string& pwd) const;
-
-  RefPtr<NrIceCtx> ctx() { return current_ctx; }
-
-  bool BeginIceRestart(RefPtr<NrIceCtx> new_ctx);
-  bool IsRestarting() const { return (old_ctx != nullptr); }
-  void FinalizeIceRestart();
-  void RollbackIceRestart();
-
-  NrIceStats Destroy();
-
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(NrIceCtxHandler)
-
-private:
-  NrIceCtxHandler(const std::string& name, NrIceCtx::Policy policy);
-  NrIceCtxHandler() = delete;
-  ~NrIceCtxHandler();
-  DISALLOW_COPY_ASSIGN(NrIceCtxHandler);
-
-  RefPtr<NrIceCtx> current_ctx;
-  RefPtr<NrIceCtx> old_ctx; // for while restart is in progress
-  int restart_count; // used to differentiate streams between restarted ctx
-};
-
-} // close namespace
-
-#endif // nricectxhandler_h__

media/mtransport/nricemediastream.cpp

@@ -183,28 +183,14 @@ static UniquePtr<NrIceCandidate> MakeNrIceCandidate(const nr_ice_candidate& cand
   return out;
 }
 
-// NrIceMediaStream
-RefPtr<NrIceMediaStream>
-NrIceMediaStream::Create(NrIceCtx *ctx,
-                         const std::string& name,
-                         int components) {
-  RefPtr<NrIceMediaStream> stream =
-    new NrIceMediaStream(ctx, name, components);
-  MOZ_ASSERT(stream->ctx_ == ctx->ctx());
-
-  int r = nr_ice_add_media_stream(ctx->ctx(),
-                                  const_cast<char *>(name.c_str()),
-                                  components, &stream->stream_);
-  if (r) {
-    MOZ_MTLOG(ML_ERROR, "Couldn't create ICE media stream for '"
-              << name << "'");
-    return nullptr;
-  }
-
-  return stream;
+static bool Matches(const nr_ice_media_stream* stream,
+                    const std::string& ufrag,
+                    const std::string& pwd) {
+  return stream && (stream->ufrag == ufrag) && (stream->pwd == pwd);
 }
 
 NrIceMediaStream::NrIceMediaStream(NrIceCtx *ctx,
+                                   const std::string& id,
                                    const std::string& name,
                                    size_t components) :
       state_(ICE_CONNECTING),
@@ -213,7 +199,8 @@ NrIceMediaStream::NrIceMediaStream(NrIceCtx *ctx,
       name_(name),
       components_(components),
       stream_(nullptr),
-      has_parsed_attrs_(false)
+      old_stream_(nullptr),
+      id_(id)
 {
 }
 
@@ -222,35 +209,84 @@ NrIceMediaStream::~NrIceMediaStream() {
   // are attached to the ice ctx.
 }
 
-nsresult NrIceMediaStream::ParseAttributes(std::vector<std::string>&
-                                           attributes) {
-  if (!stream_)
-    return NS_ERROR_FAILURE;
+nsresult NrIceMediaStream::ConnectToPeer(
+    const std::string& ufrag,
+    const std::string& pwd,
+    const std::vector<std::string>& attributes) {
+  MOZ_ASSERT(stream_);
 
-  std::vector<char *> attributes_in;
-  attributes_in.reserve(attributes.size());
-  for (auto& attribute : attributes) {
-    attributes_in.push_back(const_cast<char *>(attribute.c_str()));
+  if (Matches(old_stream_, ufrag, pwd)) {
+    // Roll back to old stream, since we apparently aren't using the new one
+    // (We swap before we close so we never have stream_ == nullptr)
+    std::swap(stream_, old_stream_);
+    CloseStream(&old_stream_);
+  } else if (old_stream_) {
+    // Right now we wait for ICE to complete before closing the old stream.
+    // It might be worth it to close it sooner, but we don't want to close it
+    // right away.
+    nr_ice_media_stream_set_obsolete(old_stream_);
   }
 
-  // Still need to call nr_ice_ctx_parse_stream_attributes.
-  int r = nr_ice_peer_ctx_parse_stream_attributes(ctx_peer_,
-                                                  stream_,
-                                                  attributes_in.empty() ?
-                                                  nullptr : &attributes_in[0],
-                                                  attributes_in.size());
+  nr_ice_media_stream* peer_stream;
+  if (nr_ice_peer_ctx_find_pstream(ctx_peer_, stream_, &peer_stream)) {
+    // No peer yet
+    std::vector<char *> attributes_in;
+    attributes_in.reserve(attributes.size());
+    for (auto& attribute : attributes) {
+      MOZ_MTLOG(ML_DEBUG, "Setting " << attribute << " on stream " << name_);
+      attributes_in.push_back(const_cast<char *>(attribute.c_str()));
+    }
+
+    // Still need to call nr_ice_ctx_parse_stream_attributes.
+    int r = nr_ice_peer_ctx_parse_stream_attributes(ctx_peer_,
+                                                    stream_,
+                                                    attributes_in.empty() ?
+                                                    nullptr : &attributes_in[0],
+                                                    attributes_in.size());
+    if (r) {
+      MOZ_MTLOG(ML_ERROR, "Couldn't parse attributes for stream "
+                << name_ << "'");
+      return NS_ERROR_FAILURE;
+    }
+  }
+
+  return NS_OK;
+}
+
+nsresult NrIceMediaStream::SetIceCredentials(const std::string& ufrag,
+                                             const std::string& pwd) {
+  if (Matches(stream_, ufrag, pwd)) {
+    return NS_OK;
+  }
+
+  MOZ_MTLOG(ML_DEBUG, "Setting ICE credentials for " << name_ << " - "
+                      << ufrag << ":" << pwd);
+  CloseStream(&old_stream_);
+  old_stream_ = stream_;
+
+  std::string name(name_ + " - " + ufrag + ":" + pwd);
+
+  int r = nr_ice_add_media_stream(ctx_,
+                                  name.c_str(),
+                                  ufrag.c_str(),
+                                  pwd.c_str(),
+                                  components_, &stream_);
   if (r) {
-    MOZ_MTLOG(ML_ERROR, "Couldn't parse attributes for stream "
-              << name_ << "'");
+    MOZ_MTLOG(ML_ERROR, "Couldn't create ICE media stream for '"
+              << name_ << "': error=" << r);
+    stream_ = old_stream_;
+    old_stream_ = nullptr;
     return NS_ERROR_FAILURE;
   }
 
-  has_parsed_attrs_ = true;
+  state_ = ICE_CONNECTING;
   return NS_OK;
 }
 
 // Parse trickle ICE candidate
 nsresult NrIceMediaStream::ParseTrickleCandidate(const std::string& candidate) {
+  // TODO(bug 1490658): This needs to take ufrag into account. For now, trickle
+  // candidates will land on the most recently-created ICE stream.
   int r;
 
   MOZ_MTLOG(ML_DEBUG, "NrIceCtx(" << ctx_->label << ")/STREAM(" <<
@@ -463,7 +499,7 @@ nsresult NrIceMediaStream::GetDefaultCandidate(
   return NS_OK;
 }
 
-std::vector<std::string> NrIceMediaStream::GetCandidates() const {
+std::vector<std::string> NrIceMediaStream::GetAttributes() const {
   char **attrs = nullptr;
   int attrct;
   int r;
@@ -587,13 +623,19 @@ nsresult NrIceMediaStream::GetConsentStatus(int component_id, bool *can_send, st
   return NS_OK;
 }
 
+bool NrIceMediaStream::HasStream(nr_ice_media_stream *stream) const {
+  return (stream == stream_) || (stream == old_stream_);
+}
+
 nsresult NrIceMediaStream::SendPacket(int component_id,
                                       const unsigned char *data,
                                       size_t len) {
-  if (!stream_)
+  nr_ice_media_stream* stream = old_stream_ ? old_stream_ : stream_;
+  if (!stream) {
     return NS_ERROR_FAILURE;
+  }
 
-  int r = nr_ice_media_stream_send(ctx_peer_, stream_,
+  int r = nr_ice_media_stream_send(ctx_peer_, stream,
                                    component_id,
                                    const_cast<unsigned char *>(data), len);
   if (r) {
@@ -615,6 +657,7 @@ void NrIceMediaStream::Ready() {
   if (state_ != ICE_OPEN) {
     MOZ_MTLOG(ML_DEBUG, "Marking stream ready '" << name_ << "'");
     state_ = ICE_OPEN;
+    CloseStream(&old_stream_);
     SignalReady(this);
   }
   else {
@@ -622,16 +665,34 @@ void NrIceMediaStream::Ready() {
   }
 }
 
+void NrIceMediaStream::Failed() {
+  if (state_ != ICE_CLOSED) {
+    MOZ_MTLOG(ML_DEBUG, "Marking stream failed '" << name_ << "'");
+    state_ = ICE_CLOSED;
+    // We don't need the old stream anymore.
+    CloseStream(&old_stream_);
+    SignalFailed(this);
+  }
+}
+
 void NrIceMediaStream::Close() {
   MOZ_MTLOG(ML_DEBUG, "Marking stream closed '" << name_ << "'");
   state_ = ICE_CLOSED;
 
-  if (stream_) {
-    int r = nr_ice_remove_media_stream(ctx_, &stream_);
+  CloseStream(&old_stream_);
+  CloseStream(&stream_);
+}
+
+void
+NrIceMediaStream::CloseStream(nr_ice_media_stream **stream)
+{
+  if (*stream) {
+    int r = nr_ice_remove_media_stream(ctx_, stream);
     if (r) {
       MOZ_ASSERT(false, "Failed to remove stream");
       MOZ_MTLOG(ML_ERROR, "Failed to remove stream, error=" << r);
     }
+    *stream = nullptr;
   }
 }
 

media/mtransport/nricemediastream.h

@@ -137,9 +137,15 @@ struct NrIceCandidatePair {
 
 class NrIceMediaStream {
  public:
-  static RefPtr<NrIceMediaStream> Create(NrIceCtx *ctx,
-                                         const std::string& name,
-                                         int components);
+  NrIceMediaStream(NrIceCtx *ctx,
+                   const std::string& id,
+                   const std::string& name,
+                   size_t components);
+
+  nsresult SetIceCredentials(const std::string& ufrag, const std::string& pwd);
+  nsresult ConnectToPeer(const std::string& ufrag,
+                         const std::string& pwd,
+                         const std::vector<std::string>& peer_attrs);
   enum State { ICE_CONNECTING, ICE_OPEN, ICE_CLOSED};
 
   State state() const { return state_; }
@@ -147,8 +153,8 @@ class NrIceMediaStream {
   // The name of the stream
   const std::string& name() const { return name_; }
 
-  // Get all the candidates
-  std::vector<std::string> GetCandidates() const;
+  // Get all the ICE attributes; used for testing
+  std::vector<std::string> GetAttributes() const;
 
   nsresult GetLocalCandidates(std::vector<NrIceCandidate>* candidates) const;
   nsresult GetRemoteCandidates(std::vector<NrIceCandidate>* candidates) const;
@@ -159,10 +165,6 @@ class NrIceMediaStream {
 
   nsresult GetDefaultCandidate(int component, NrIceCandidate* candidate) const;
 
-  // Parse remote attributes
-  nsresult ParseAttributes(std::vector<std::string>& candidates);
-  bool HasParsedAttributes() const { return has_parsed_attrs_; }
-
   // Parse trickle ICE candidate
   nsresult ParseTrickleCandidate(const std::string& candidate);
 
@@ -182,8 +184,7 @@ class NrIceMediaStream {
   // The number of components
   size_t components() const { return components_; }
 
-  // The underlying nICEr stream
-  nr_ice_media_stream *stream() { return stream_; }
+  bool HasStream(nr_ice_media_stream *stream) const;
   // Signals to indicate events. API users can (and should)
   // register for these.
 
@@ -192,6 +193,7 @@ class NrIceMediaStream {
 
   // Set your state to ready. Called by the NrIceCtx;
   void Ready();
+  void Failed();
 
   // Close the stream. Called by the NrIceCtx.
   // Different from the destructor because other people
@@ -201,8 +203,6 @@ class NrIceMediaStream {
 
   // So the receiver of SignalCandidate can determine which transport
   // the candidate belongs to.
-  void SetId(const std::string& id) { id_ = id; }
-
   const std::string& GetId() const { return id_; }
 
   sigslot::signal2<NrIceMediaStream *, const std::string& >
@@ -216,22 +216,20 @@ class NrIceMediaStream {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(NrIceMediaStream)
 
  private:
-  NrIceMediaStream(NrIceCtx *ctx,
-                   const std::string& name,
-                   size_t components);
-
   ~NrIceMediaStream();
 
   DISALLOW_COPY_ASSIGN(NrIceMediaStream);
 
+  void CloseStream(nr_ice_media_stream **stream);
+
   State state_;
   nr_ice_ctx *ctx_;
   nr_ice_peer_ctx *ctx_peer_;
   const std::string name_;
   const size_t components_;
   nr_ice_media_stream *stream_;
-  std::string id_;
-  bool has_parsed_attrs_;
+  nr_ice_media_stream *old_stream_;
+  const std::string id_;
 };
 
 

media/mtransport/nriceresolver.h

@@ -51,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "nsIDNSService.h"
 #include "nsIDNSListener.h"
 #include "nsICancelable.h"
+#include "nricectx.h"
 
 typedef struct nr_resolver_ nr_resolver;
 typedef struct nr_resolver_vtbl_ nr_resolver_vtbl;

media/mtransport/test/ice_unittest.cpp

@@ -23,7 +23,20 @@
 #include "nsThreadUtils.h"
 #include "nsXPCOM.h"
 
-#include "nricectxhandler.h"
+extern "C" {
+#include "r_types.h"
+#include "async_wait.h"
+#include "async_timer.h"
+#include "r_data.h"
+#include "util.h"
+#include "r_time.h"
+}
+
+#include "ice_ctx.h"
+#include "ice_peer_ctx.h"
+#include "ice_media_stream.h"
+
+#include "nricectx.h"
 #include "nricemediastream.h"
 #include "nriceresolverfake.h"
 #include "nriceresolver.h"
@@ -38,17 +51,6 @@
 #include "stun_socket_filter.h"
 #include "mozilla/net/DNS.h"
 
-#include "ice_ctx.h"
-#include "ice_peer_ctx.h"
-#include "ice_media_stream.h"
-
-extern "C" {
-#include "async_timer.h"
-#include "r_data.h"
-#include "util.h"
-#include "r_time.h"
-}
-
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
@@ -380,9 +382,9 @@ class IceTestPeer : public sigslot::has_slots<> {
               bool allow_link_local = false,
               NrIceCtx::Policy ice_policy = NrIceCtx::ICE_POLICY_ALL) :
       name_(name),
-      ice_ctx_(NrIceCtxHandler::Create(name, allow_loopback,
-                                       enable_tcp, allow_link_local,
-                                       ice_policy)),
+      ice_ctx_(NrIceCtx::Create(name, allow_loopback,
+                                enable_tcp, allow_link_local,
+                                ice_policy)),
       offerer_(offerer),
       candidates_(),
       stream_counter_(0),
@@ -405,16 +407,16 @@ class IceTestPeer : public sigslot::has_slots<> {
       simulate_ice_lite_(false),
       nat_(new TestNat),
       test_utils_(utils) {
-    ice_ctx_->ctx()->SignalGatheringStateChange.connect(
+    ice_ctx_->SignalGatheringStateChange.connect(
         this,
         &IceTestPeer::GatheringStateChange);
-    ice_ctx_->ctx()->SignalConnectionStateChange.connect(
+    ice_ctx_->SignalConnectionStateChange.connect(
         this,
         &IceTestPeer::ConnectionStateChange);
 
     consent_timestamp_.tv_sec = 0;
     consent_timestamp_.tv_usec = 0;
-    int r = ice_ctx_->ctx()->SetNat(nat_);
+    int r = ice_ctx_->SetNat(nat_);
     (void)r;
     MOZ_ASSERT(!r);
   }
@@ -435,13 +437,28 @@ class IceTestPeer : public sigslot::has_slots<> {
     return id;
   }
 
+  void SetIceCredentials_s(NrIceMediaStream &stream) {
+    static size_t counter = 0;
+    std::ostringstream prefix;
+    prefix << name_ << "-" << counter++;
+    std::string ufrag = prefix.str() + "-ufrag";
+    std::string pwd = prefix.str() + "-pwd";
+    if (mIceCredentials.count(stream.GetId())) {
+      mOldIceCredentials[stream.GetId()] = mIceCredentials[stream.GetId()];
+    }
+    mIceCredentials[stream.GetId()] = std::make_pair(ufrag, pwd);
+    stream.SetIceCredentials(ufrag, pwd);
+  }
+
   void AddStream_s(int components) {
     std::string id = MakeTransportId(stream_counter_++);
 
-    RefPtr<NrIceMediaStream> stream = ice_ctx_->CreateStream(id, components);
-    ice_ctx_->ctx()->SetStream(id, stream);
+    RefPtr<NrIceMediaStream> stream =
+      ice_ctx_->CreateStream(id, id, components);
 
     ASSERT_TRUE(stream);
+    SetIceCredentials_s(*stream);
+
     stream->SignalCandidate.connect(this, &IceTestPeer::CandidateInitialized);
     stream->SignalReady.connect(this, &IceTestPeer::StreamReady);
     stream->SignalFailed.connect(this, &IceTestPeer::StreamFailed);
@@ -456,7 +473,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   void RemoveStream_s(size_t index) {
-    ice_ctx_->ctx()->SetStream(MakeTransportId(index), nullptr);
+    ice_ctx_->DestroyStream(MakeTransportId(index));
   }
 
   void RemoveStream(size_t index) {
@@ -467,7 +484,7 @@ class IceTestPeer : public sigslot::has_slots<> {
 
   RefPtr<NrIceMediaStream> GetStream_s(size_t index) {
     std::string id = MakeTransportId(index);
-    return ice_ctx_->ctx()->GetStream(id);
+    return ice_ctx_->GetStream(id);
   }
 
   void SetStunServer(const std::string addr, uint16_t port,
@@ -485,7 +502,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   void SetStunServers(const std::vector<NrIceStunServer> &servers) {
-    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetStunServers(servers)));
+    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->SetStunServers(servers)));
   }
 
   void UseTestStunServer() {
@@ -510,11 +527,11 @@ class IceTestPeer : public sigslot::has_slots<> {
     UniquePtr<NrIceTurnServer> server(NrIceTurnServer::Create(
         addr, port, username, password, transport));
     turn_servers.push_back(*server);
-    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetTurnServers(turn_servers)));
+    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->SetTurnServers(turn_servers)));
   }
 
   void SetTurnServers(const std::vector<NrIceTurnServer> servers) {
-    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetTurnServers(servers)));
+    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->SetTurnServers(servers)));
   }
 
   void SetFakeResolver(const std::string& ip,
@@ -527,13 +544,13 @@ class IceTestPeer : public sigslot::has_slots<> {
       ASSERT_EQ(PR_SUCCESS, status);
       fake_resolver_.SetAddr(fqdn, addr);
     }
-    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetResolver(
+    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->SetResolver(
         fake_resolver_.AllocateResolver())));
   }
 
   void SetDNSResolver() {
     ASSERT_TRUE(NS_SUCCEEDED(dns_resolver_->Init()));
-    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetResolver(
+    ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->SetResolver(
         dns_resolver_->AllocateResolver())));
   }
 
@@ -542,7 +559,7 @@ class IceTestPeer : public sigslot::has_slots<> {
 
     test_utils_->sts_target()->Dispatch(
         WrapRunnableRet(&res,
-                        ice_ctx_->ctx(),
+                        ice_ctx_,
                         &NrIceCtx::StartGathering,
                         default_route_only,
                         false),
@@ -556,7 +573,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   void SetTimerDivider(int div) {
-    ice_ctx_->ctx()->internal_SetTimerAccelarator(div);
+    ice_ctx_->internal_SetTimerAccelarator(div);
   }
 
   void SetStunResponseDelay(uint32_t delay) {
@@ -584,19 +601,19 @@ class IceTestPeer : public sigslot::has_slots<> {
 
   // Get various pieces of state
   std::vector<std::string> GetGlobalAttributes() {
-    std::vector<std::string> attrs(ice_ctx_->ctx()->GetGlobalAttributes());
+    std::vector<std::string> attrs(ice_ctx_->GetGlobalAttributes());
     if (simulate_ice_lite_) {
       attrs.push_back("ice-lite");
     }
     return attrs;
   }
 
-  std::vector<std::string> GetCandidates(size_t stream) {
+  std::vector<std::string> GetAttributes(size_t stream) {
     std::vector<std::string> v;
 
     RUN_ON_THREAD(
         test_utils_->sts_target(),
-        WrapRunnableRet(&v, this, &IceTestPeer::GetCandidates_s, stream));
+        WrapRunnableRet(&v, this, &IceTestPeer::GetAttributes_s, stream));
 
     return v;
   }
@@ -608,27 +625,31 @@ class IceTestPeer : public sigslot::has_slots<> {
     return candidate;
   }
 
-  std::vector<std::string> GetCandidates_s(size_t index) {
-    std::vector<std::string> candidates;
+  std::vector<std::string> GetAttributes_s(size_t index) {
+    std::vector<std::string> attributes;
 
     auto stream = GetStream_s(index);
     if (!stream) {
       EXPECT_TRUE(false) << "No such stream " << index;
-      return candidates;
+      return attributes;
     }
 
-    std::vector<std::string> candidates_in = stream->GetCandidates();
+    std::vector<std::string> attributes_in = stream->GetAttributes();
 
-    for (const auto& a_candidate : candidates_in) {
-      std::string candidate(FilterCandidate(a_candidate));
-      if (!candidate.empty()) {
-        std::cerr << name_ << " Returning candidate: "
-                           << candidate << std::endl;
-        candidates.push_back(candidate);
+    for (const auto& attribute : attributes_in) {
+      if (attribute.find("candidate:") != std::string::npos) {
+        std::string candidate(FilterCandidate(attribute));
+        if (!candidate.empty()) {
+          std::cerr << name_ << " Returning candidate: "
+                             << candidate << std::endl;
+          attributes.push_back(candidate);
+        }
+      } else {
+        attributes.push_back(attribute);
       }
     }
 
-    return candidates;
+    return attributes;
   }
 
   void SetExpectedTypes(NrIceCandidate::Type local,
@@ -644,13 +665,13 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   int GetCandidatesPrivateIpv4Range(size_t stream) {
-    std::vector<std::string> candidates = GetCandidates(stream);
+    std::vector<std::string> attributes = GetAttributes(stream);
 
     int host_net = 0;
-    for (const auto& c : candidates) {
-      if (c.find("typ host") != std::string::npos) {
+    for (const auto& a : attributes) {
+      if (a.find("typ host") != std::string::npos) {
         nr_transport_addr addr;
-        std::vector<std::string> tokens = split(c, ' ');
+        std::vector<std::string> tokens = split(a, ' ');
         int r = nr_str_port_to_transport_addr(tokens.at(4).c_str(), 0, IPPROTO_UDP, &addr);
         MOZ_ASSERT(!r);
         if (!r && (addr.ip_version == NR_IPV4)) {
@@ -697,23 +718,15 @@ class IceTestPeer : public sigslot::has_slots<> {
   void RestartIce() {
     test_utils_->sts_target()->Dispatch(
         WrapRunnable(this,
-                     &IceTestPeer::RestartIce_s,
-                     ice_ctx_->CreateCtx()),
+                     &IceTestPeer::RestartIce_s),
         NS_DISPATCH_SYNC);
   }
 
 
-  void RestartIce_s(RefPtr<NrIceCtx> new_ctx) {
-    ice_ctx_->BeginIceRestart(new_ctx);
-
-    // set signals for the newly restarted ctx
-    ice_ctx_->ctx()->SignalGatheringStateChange.connect(
-        this,
-        &IceTestPeer::GatheringStateChange);
-    ice_ctx_->ctx()->SignalConnectionStateChange.connect(
-        this,
-        &IceTestPeer::ConnectionStateChange);
-
+  void RestartIce_s() {
+    for (auto& stream : ice_ctx_->GetStreams()) {
+      SetIceCredentials_s(*stream);
+    }
     // take care of some local bookkeeping
     ready_ct_ = 0;
     gathering_complete_ = false;
@@ -721,19 +734,6 @@ class IceTestPeer : public sigslot::has_slots<> {
     ice_failed_ = false;
     ice_reached_checking_ = false;
     remote_ = nullptr;
-    stream_counter_ = 0;
-  }
-
-
-  void FinalizeIceRestart() {
-    test_utils_->sts_target()->Dispatch(
-        WrapRunnable(this, &IceTestPeer::FinalizeIceRestart_s),
-        NS_DISPATCH_SYNC);
-  }
-
-
-  void FinalizeIceRestart_s() {
-    ice_ctx_->FinalizeIceRestart();
   }
 
 
@@ -745,7 +745,9 @@ class IceTestPeer : public sigslot::has_slots<> {
 
 
   void RollbackIceRestart_s() {
-    ice_ctx_->RollbackIceRestart();
+    for (auto& stream : ice_ctx_->GetStreams()) {
+      mIceCredentials[stream->GetId()] = mOldIceCredentials[stream->GetId()];
+    }
   }
 
 
@@ -760,44 +762,38 @@ class IceTestPeer : public sigslot::has_slots<> {
     ice_connected_ = false;
     ice_failed_ = false;
     ice_reached_checking_ = false;
-    res = ice_ctx_->ctx()->ParseGlobalAttributes(remote->GetGlobalAttributes());
+    res = ice_ctx_->ParseGlobalAttributes(remote->GetGlobalAttributes());
     ASSERT_TRUE(NS_SUCCEEDED(res));
 
-    if (trickle_mode == TRICKLE_NONE ||
-        trickle_mode == TRICKLE_REAL) {
-      for (size_t i=0; i<stream_counter_; ++i) {
-        auto aStream = GetStream_s(i);
-        if (!aStream || aStream->HasParsedAttributes()) {
-          continue;
-        }
-        std::vector<std::string> candidates =
-            remote->GetCandidates(i);
+    for (size_t i=0; i<stream_counter_; ++i) {
+      auto aStream = GetStream_s(i);
+      if (aStream) {
+        std::vector<std::string> attributes = remote->GetAttributes(i);
 
-        for (const auto& candidate : candidates) {
-          std::cerr << name_ << " Adding remote candidate: " + candidate << std::endl;
+        for (auto it = attributes.begin(); it != attributes.end();) {
+          if (trickle_mode == TRICKLE_SIMULATE &&
+              it->find("candidate:") != std::string::npos) {
+            std::cerr << name_ << " Deferring remote candidate: " << *it
+                      << std::endl;
+            attributes.erase(it);
+          } else {
+            std::cerr << name_ << " Adding remote attribute: " + *it
+                      << std::endl;
+            ++it;
+          }
         }
-        res = aStream->ParseAttributes(candidates);
-        ASSERT_TRUE(NS_SUCCEEDED(res));
-      }
-    } else {
-      // Parse empty attributes and then trickle them out later
-      for (size_t i=0; i<stream_counter_; ++i) {
-        auto aStream = GetStream_s(i);
-        if (!aStream || aStream->HasParsedAttributes()) {
-          continue;
-        }
-        std::vector<std::string> empty_attrs;
-        std::cout << "Calling ParseAttributes on stream " << i << std::endl;
-        res = aStream->ParseAttributes(empty_attrs);
+        auto credentials = mIceCredentials[aStream->GetId()];
+        res = aStream->ConnectToPeer(
+            credentials.first, credentials.second, attributes);
         ASSERT_TRUE(NS_SUCCEEDED(res));
       }
     }
 
     if (start) {
-      ice_ctx_->ctx()->SetControlling(
+      ice_ctx_->SetControlling(
           offerer_ ? NrIceCtx::ICE_CONTROLLING : NrIceCtx::ICE_CONTROLLED);
       // Now start checks
-      res = ice_ctx_->ctx()->StartChecks(offerer_);
+      res = ice_ctx_->StartChecks(offerer_);
       ASSERT_TRUE(NS_SUCCEEDED(res));
     }
   }
@@ -829,13 +825,14 @@ class IceTestPeer : public sigslot::has_slots<> {
   std::vector<SchedulableTrickleCandidate*>& ControlTrickle(size_t stream) {
     std::cerr << "Doing controlled trickle for stream " << stream << std::endl;
 
-    std::vector<std::string> candidates =
-      remote_->GetCandidates(stream);
+    std::vector<std::string> attributes = remote_->GetAttributes(stream);
 
-    for (const auto& candidate : candidates) {
-      controlled_trickle_candidates_[stream].push_back(
-          new SchedulableTrickleCandidate(
-              this, stream, candidate, test_utils_));
+    for (const auto& attribute : attributes) {
+      if (attribute.find("candidate:") != std::string::npos) {
+        controlled_trickle_candidates_[stream].push_back(
+            new SchedulableTrickleCandidate(
+                this, stream, attribute, test_utils_));
+      }
     }
 
     return controlled_trickle_candidates_[stream];
@@ -919,7 +916,7 @@ class IceTestPeer : public sigslot::has_slots<> {
 
   void DumpAndCheckActiveCandidates_s() {
     std::cerr << name_ << " Active candidates:" << std::endl;
-    for (const auto& stream : ice_ctx_->ctx()->GetStreams()) {
+    for (const auto& stream : ice_ctx_->GetStreams()) {
       for (size_t j=0; j < stream->components(); ++j) {
         std::cerr << name_ << " Stream " << stream->GetId()
                            << " component " << j+1 << std::endl;
@@ -969,7 +966,7 @@ class IceTestPeer : public sigslot::has_slots<> {
 
   void Close() {
     test_utils_->sts_target()->Dispatch(
-      WrapRunnable(ice_ctx_->ctx(), &NrIceCtx::destroy_peer_ctx),
+      WrapRunnable(ice_ctx_, &NrIceCtx::destroy_peer_ctx),
       NS_DISPATCH_SYNC);
   }
 
@@ -999,13 +996,13 @@ class IceTestPeer : public sigslot::has_slots<> {
     nsresult res;
 
     test_utils_->sts_target()->Dispatch(
-        WrapRunnableRet(&res, ice_ctx_->ctx(), &NrIceCtx::SetControlling,
+        WrapRunnableRet(&res, ice_ctx_, &NrIceCtx::SetControlling,
                         offerer_ ?
                           NrIceCtx::ICE_CONTROLLING : NrIceCtx::ICE_CONTROLLED),
         NS_DISPATCH_SYNC);
     // Now start checks
     test_utils_->sts_target()->Dispatch(
-        WrapRunnableRet(&res, ice_ctx_->ctx(), &NrIceCtx::StartChecks,
+        WrapRunnableRet(&res, ice_ctx_, &NrIceCtx::StartChecks,
                         offerer_),
         NS_DISPATCH_SYNC);
     ASSERT_TRUE(NS_SUCCEEDED(res));
@@ -1024,14 +1021,14 @@ class IceTestPeer : public sigslot::has_slots<> {
     std::cerr << name_ << " Gathering complete" << std::endl;
     gathering_complete_ = true;
 
-    std::cerr << name_ << " CANDIDATES:" << std::endl;
-    for (const auto& stream : ice_ctx_->ctx()->GetStreams()) {
+    std::cerr << name_ << " ATTRIBUTES:" << std::endl;
+    for (const auto& stream : ice_ctx_->GetStreams()) {
       std::cerr << "Stream " << stream->GetId() << std::endl;
 
-      std::vector<std::string> candidates = stream->GetCandidates();
+      std::vector<std::string> attributes = stream->GetAttributes();
 
-      for(const auto& candidate : candidates) {
-        std::cerr << candidate << std::endl;
+      for(const auto& attribute : attributes) {
+        std::cerr << attribute << std::endl;
       }
     }
     std::cerr << std::endl;
@@ -1116,7 +1113,7 @@ class IceTestPeer : public sigslot::has_slots<> {
 
   void DumpCandidatePairs_s() {
     std::cerr << "Dumping candidate pairs for all streams [" << std::endl;
-    for (const auto& stream : ice_ctx_->ctx()->GetStreams()) {
+    for (const auto& stream : ice_ctx_->GetStreams()) {
       DumpCandidatePairs_s(stream.get());
     }
     std::cerr << "]" << std::endl;
@@ -1281,11 +1278,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   void ParseCandidate_s(size_t i, const std::string& candidate) {
     auto media_stream = GetStream_s(i);
     ASSERT_TRUE(media_stream.get()) << "No such stream " << i;
-
-    std::vector<std::string> attributes;
-
-    attributes.push_back(candidate);
-    media_stream->ParseAttributes(attributes);
+    media_stream->ParseTrickleCandidate(candidate);
   }
 
   void ParseCandidate(size_t i, const std::string& candidate)
@@ -1355,7 +1348,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   void ChangeNetworkState_s(bool online) {
-    ice_ctx_->ctx()->UpdateNetworkState(online);
+    ice_ctx_->UpdateNetworkState(online);
   }
 
   void ChangeNetworkStateToOffline() {
@@ -1377,7 +1370,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   void SetControlling(NrIceCtx::Controlling controlling) {
     nsresult res;
     test_utils_->sts_target()->Dispatch(
-        WrapRunnableRet(&res, ice_ctx_->ctx(),
+        WrapRunnableRet(&res, ice_ctx_,
                         &NrIceCtx::SetControlling,
                         controlling),
         NS_DISPATCH_SYNC);
@@ -1385,7 +1378,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   NrIceCtx::Controlling GetControlling() {
-    return ice_ctx_->ctx()->GetControlling();
+    return ice_ctx_->GetControlling();
   }
 
   void SetTiebreaker(uint64_t tiebreaker) {
@@ -1397,7 +1390,7 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   void SetTiebreaker_s(uint64_t tiebreaker) {
-    ice_ctx_->ctx()->peer()->tiebreaker = tiebreaker;
+    ice_ctx_->peer()->tiebreaker = tiebreaker;
   }
 
   void SimulateIceLite() {
@@ -1423,12 +1416,14 @@ class IceTestPeer : public sigslot::has_slots<> {
 
  private:
   std::string name_;
-  RefPtr<NrIceCtxHandler> ice_ctx_;
+  RefPtr<NrIceCtx> ice_ctx_;
   bool offerer_;
   std::map<std::string, std::vector<std::string> > candidates_;
   // Maps from stream id to list of remote trickle candidates
   std::map<size_t, std::vector<SchedulableTrickleCandidate*> >
     controlled_trickle_candidates_;
+  std::map<std::string, std::pair<std::string, std::string>> mIceCredentials;
+  std::map<std::string, std::pair<std::string, std::string>> mOldIceCredentials;
   size_t stream_counter_;
   bool shutting_down_;
   bool gathering_complete_;
@@ -1598,11 +1593,11 @@ class WebRtcIceGatherTest : public StunTest {
   bool StreamHasMatchingCandidate(unsigned int stream,
                                   const std::string& match,
                                   const std::string& match2 = "") {
-    std::vector<std::string> candidates = peer_->GetCandidates(stream);
-    for (auto& candidate : candidates) {
-      if (std::string::npos != candidate.find(match)) {
+    std::vector<std::string> attributes = peer_->GetAttributes(stream);
+    for (auto& attribute : attributes) {
+      if (std::string::npos != attribute.find(match)) {
         if (!match2.length() ||
-            std::string::npos != candidate.find(match2)) {
+            std::string::npos != attribute.find(match2)) {
           return true;
         }
       }
@@ -1610,14 +1605,14 @@ class WebRtcIceGatherTest : public StunTest {
     return false;
   }
 
-  void DumpCandidates(unsigned int stream) {
-    std::vector<std::string> candidates = peer_->GetCandidates(stream);
+  void DumpAttributes(unsigned int stream) {
+    std::vector<std::string> attributes = peer_->GetAttributes(stream);
 
-    std::cerr << "Candidates for stream " << stream << "->"
-              << candidates.size() << std::endl;
+    std::cerr << "Attributes for stream " << stream << "->"
+              << attributes.size() << std::endl;
 
-    for (const auto& c : candidates) {
-      std::cerr << "Candidate: " << c << std::endl;
+    for (const auto& a : attributes) {
+      std::cerr << "Attribute: " << a << std::endl;
     }
   }
 
@@ -1812,7 +1807,9 @@ class WebRtcIceConnectTest : public StunTest {
     ConnectCallerAndCallee(p1_.get(), p2_.get());
   }
 
-  void ConnectCallerAndCallee(IceTestPeer* caller, IceTestPeer* callee) {
+  void ConnectCallerAndCallee(IceTestPeer* caller,
+                              IceTestPeer* callee,
+                              TrickleMode mode = TRICKLE_NONE) {
     ASSERT_TRUE(caller->ready_ct() == 0);
     ASSERT_TRUE(caller->ice_connected() == 0);
     ASSERT_TRUE(caller->ice_reached_checking() == 0);
@@ -1824,19 +1821,20 @@ class WebRtcIceConnectTest : public StunTest {
     // gives them to |this|, meaning that callee->Connect(caller, ...)
     // simulates caller sending an offer to callee. Order matters here
     // because it determines which peer is controlling.
-    callee->Connect(caller, TRICKLE_NONE);
-    caller->Connect(callee, TRICKLE_NONE);
+    callee->Connect(caller, mode);
+    caller->Connect(callee, mode);
 
-    ASSERT_TRUE_WAIT(caller->ready_ct() == 1 && callee->ready_ct() == 1,
-                     kDefaultTimeout);
-    ASSERT_TRUE_WAIT(caller->ice_connected() && callee->ice_connected(),
-                     kDefaultTimeout);
+    if (mode != TRICKLE_SIMULATE) {
+      ASSERT_TRUE_WAIT(caller->ready_ct() == 1 && callee->ready_ct() == 1,
+                       kDefaultTimeout);
+      ASSERT_TRUE_WAIT(caller->ice_connected() && callee->ice_connected(),
+                       kDefaultTimeout);
+      ASSERT_TRUE(caller->ice_reached_checking());
+      ASSERT_TRUE(callee->ice_reached_checking());
 
-    ASSERT_TRUE(caller->ice_reached_checking());
-    ASSERT_TRUE(callee->ice_reached_checking());
-
-    caller->DumpAndCheckActiveCandidates();
-    callee->DumpAndCheckActiveCandidates();
+      caller->DumpAndCheckActiveCandidates();
+      callee->DumpAndCheckActiveCandidates();
+    }
   }
 
   void SetExpectedTypes(NrIceCandidate::Type local, NrIceCandidate::Type remote,
@@ -1941,15 +1939,18 @@ class WebRtcIceConnectTest : public StunTest {
     size_t previousSent = p1->sent();
     size_t previousReceived = p2->received();
 
-    test_utils_->sts_target()->Dispatch(
-        WrapRunnable(p1,
-                     &IceTestPeer::SendPacket, 0, 1,
-                     reinterpret_cast<const unsigned char *>("TEST"), 4),
-        NS_DISPATCH_SYNC);
 
     if (expect_tx_failure) {
+      test_utils_->sts_target()->Dispatch(
+          WrapRunnable(p1, &IceTestPeer::SendFailure, 0, 1),
+          NS_DISPATCH_SYNC);
       ASSERT_EQ(previousSent, p1->sent());
     } else {
+      test_utils_->sts_target()->Dispatch(
+          WrapRunnable(p1,
+                       &IceTestPeer::SendPacket, 0, 1,
+                       reinterpret_cast<const unsigned char *>("TEST"), 4),
+          NS_DISPATCH_SYNC);
       ASSERT_EQ(previousSent+1, p1->sent());
     }
     if (expect_rx_failure) {
@@ -2036,7 +2037,7 @@ class WebRtcIcePacketFilterTest : public StunTest {
     StunTest::SetUp();
 
     // Set up enough of the ICE ctx to allow the packet filter to work
-    ice_ctx_ = NrIceCtxHandler::Create("test", true);
+    ice_ctx_ = NrIceCtx::Create("test", true);
 
     nsCOMPtr<nsISocketFilterHandler> udp_handler =
       do_GetService(NS_STUN_UDP_SOCKET_FILTER_HANDLER_CONTRACTID);
@@ -2148,7 +2149,7 @@ class WebRtcIcePacketFilterTest : public StunTest {
 
   nsCOMPtr<nsISocketFilter> udp_filter_;
   nsCOMPtr<nsISocketFilter> tcp_filter_;
-  RefPtr<NrIceCtxHandler> ice_ctx_;
+  RefPtr<NrIceCtx> ice_ctx_;
 };
 }  // end namespace
 
@@ -2363,12 +2364,13 @@ TEST_F(WebRtcIceGatherTest, TestGatherDisableComponent) {
   peer_->AddStream(2);
   peer_->DisableComponent(1, 2);
   Gather();
-  std::vector<std::string> candidates =
-    peer_->GetCandidates(1);
+  std::vector<std::string> attributes = peer_->GetAttributes(1);
 
-  for (auto& candidate : candidates) {
-    size_t sp1 = candidate.find(' ');
-    ASSERT_EQ(0, candidate.compare(sp1+1, 1, "1", 1));
+  for (auto& attribute : attributes) {
+    if (attribute.find("candidate:") != std::string::npos) {
+      size_t sp1 = attribute.find(' ');
+      ASSERT_EQ(0, attribute.compare(sp1+1, 1, "1", 1));
+    }
   }
 }
 
@@ -2494,7 +2496,7 @@ TEST_F(WebRtcIceGatherTest, TestFakeStunServerNatedNoHost) {
   UseFakeStunUdpServerWithResponse("192.0.2.1", 3333);
   Gather(0);
   WaitForGather();
-  DumpCandidates(0);
+  DumpAttributes(0);
   ASSERT_FALSE(StreamHasMatchingCandidate(0, "host"));
   ASSERT_TRUE(StreamHasMatchingCandidate(0, "srflx"));
   NrIceCandidate default_candidate;
@@ -2511,7 +2513,7 @@ TEST_F(WebRtcIceGatherTest, TestFakeStunServerNoNatNoHost) {
   UseTestStunServer();
   Gather(0);
   WaitForGather();
-  DumpCandidates(0);
+  DumpAttributes(0);
   ASSERT_FALSE(StreamHasMatchingCandidate(0, "host"));
   ASSERT_TRUE(StreamHasMatchingCandidate(0, "srflx"));
 }
@@ -2601,17 +2603,15 @@ TEST_F(WebRtcIceConnectTest, TestConnectRestartIce) {
   InitPeer(p3_.get());
   p3_->AddStream(1);
 
-  p2_->AddStream(1);
   ASSERT_TRUE(GatherCallerAndCallee(p2_.get(), p3_.get()));
   std::cout << "-------------------------------------------------" << std::endl;
-  ConnectCallerAndCallee(p3_.get(), p2_.get());
-  SendReceive(p1_.get(), p2_.get()); // p1 and p2 still connected
-  SendReceive(p3_.get(), p2_.get()); // p3 and p2 are now connected
-
-  p2_->FinalizeIceRestart();
-  SendReceive(p3_.get(), p2_.get()); // p3 and p2 are still connected
-
+  ConnectCallerAndCallee(p3_.get(), p2_.get(), TRICKLE_SIMULATE);
+  SendReceive(p1_.get(), p2_.get()); // p1 and p2 are still connected
+  SendReceive(p3_.get(), p2_.get(), true, true); // p3 and p2 not yet connected
+  p2_->SimulateTrickle(0);
+  p3_->SimulateTrickle(0);
   SendReceive(p1_.get(), p2_.get(), false, true); // p1 and p2 not connected
+  SendReceive(p3_.get(), p2_.get()); // p3 and p2 are now connected
 
   p3_ = nullptr;
 }
@@ -2633,22 +2633,15 @@ TEST_F(WebRtcIceConnectTest, TestConnectRestartIceThenAbort) {
   InitPeer(p3_.get());
   p3_->AddStream(1);
 
-  p2_->AddStream(1);
   ASSERT_TRUE(GatherCallerAndCallee(p2_.get(), p3_.get()));
   std::cout << "-------------------------------------------------" << std::endl;
-  ConnectCallerAndCallee(p3_.get(), p2_.get());
-  SendReceive(p1_.get(), p2_.get()); // p1 and p2 still connected
-  SendReceive(p3_.get(), p2_.get()); // p3 and p2 are now connected
-
   p2_->RollbackIceRestart();
-  SendReceive(p1_.get(), p2_.get()); // p1 and p2 are still connected
-
-  SendReceive(p3_.get(), p2_.get(), false, true); // p3 and p2 not connected
-
+  p2_->Connect(p1_.get(), TRICKLE_NONE);
+  SendReceive(p1_.get(), p2_.get());
   p3_ = nullptr;
 }
 
-TEST_F(WebRtcIceConnectTest, TestConnectSetControllingAfterIceRestart) {
+TEST_F(WebRtcIceConnectTest, TestConnectIceRestartRoleConflict) {
   AddStream(1);
   ASSERT_TRUE(Gather());
   // Just for fun lets do this with switched rolls
@@ -2662,35 +2655,31 @@ TEST_F(WebRtcIceConnectTest, TestConnectSetControllingAfterIceRestart) {
 
   p2_->RestartIce();
   ASSERT_FALSE(p2_->gathering_complete());
-  // ICE restart should allow us to set control role again
   p2_->SetControlling(NrIceCtx::ICE_CONTROLLED);
-  ASSERT_EQ(NrIceCtx::ICE_CONTROLLED, p2_->GetControlling());
-  // But still only allowed to set control role once
-  p2_->SetControlling(NrIceCtx::ICE_CONTROLLING);
-  ASSERT_EQ(NrIceCtx::ICE_CONTROLLED, p2_->GetControlling());
+  ASSERT_EQ(NrIceCtx::ICE_CONTROLLING, p2_->GetControlling()) <<
+    "ICE restart should not allow role to change, unless ice-lite happens";
 
   mozilla::UniquePtr<IceTestPeer> p3_;
   p3_ = MakeUnique<IceTestPeer>("P3", test_utils_, true, false, false, false);
   InitPeer(p3_.get());
   p3_->AddStream(1);
-  // Set control role for p3 accordingly (w/o role conflict)
+  // Set control role for p3 accordingly (with role conflict)
   p3_->SetControlling(NrIceCtx::ICE_CONTROLLING);
   ASSERT_EQ(NrIceCtx::ICE_CONTROLLING, p3_->GetControlling());
 
-  p2_->AddStream(1);
   ASSERT_TRUE(GatherCallerAndCallee(p2_.get(), p3_.get()));
   std::cout << "-------------------------------------------------" << std::endl;
   ConnectCallerAndCallee(p3_.get(), p2_.get());
-  // Again connecting should not result in role switch
-  ASSERT_EQ(NrIceCtx::ICE_CONTROLLED, p2_->GetControlling());
-  ASSERT_EQ(NrIceCtx::ICE_CONTROLLING, p3_->GetControlling());
+  auto p2role = p2_->GetControlling();
+  ASSERT_NE(p2role, p3_->GetControlling()) << "Conflict should be resolved";
+  ASSERT_EQ(NrIceCtx::ICE_CONTROLLED, p1_->GetControlling())
+    << "P1 should be unaffected by role conflict";
 
-  p2_->FinalizeIceRestart();
   // And again we are not allowed to switch roles at this point any more
-  p2_->SetControlling(NrIceCtx::ICE_CONTROLLING);
-  ASSERT_EQ(NrIceCtx::ICE_CONTROLLED, p2_->GetControlling());
-  p3_->SetControlling(NrIceCtx::ICE_CONTROLLED);
-  ASSERT_EQ(NrIceCtx::ICE_CONTROLLING, p3_->GetControlling());
+  p1_->SetControlling(NrIceCtx::ICE_CONTROLLING);
+  ASSERT_EQ(NrIceCtx::ICE_CONTROLLED, p1_->GetControlling());
+  p3_->SetControlling(p2role);
+  ASSERT_NE(p2role, p3_->GetControlling());
 
   p3_ = nullptr;
 }

media/mtransport/test/multi_tcp_socket_unittest.cpp

@@ -24,7 +24,7 @@ extern "C" {
 
 #include "stunserver.h"
 
-#include "nricectxhandler.h"
+#include "nricectx.h"
 #include "nricemediastream.h"
 
 #define GTEST_HAS_RTTI 0
@@ -47,7 +47,7 @@ class MultiTcpSocketTest : public MtransportTest {
   void SetUp() {
     MtransportTest::SetUp();
 
-    ice_ctx_ = NrIceCtxHandler::Create("stun", true);
+    ice_ctx_ = NrIceCtx::Create("stun", true);
 
     test_utils_->sts_target()->Dispatch(
         WrapRunnableNM(&TestStunTcpServer::GetInstance, AF_INET),
@@ -108,7 +108,7 @@ class MultiTcpSocketTest : public MtransportTest {
           stun_server_addr, stun_server_port, kNrIceTransportTcp));
       stun_servers.push_back(*server);
 
-      ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetStunServers(stun_servers)));
+      ASSERT_TRUE(NS_SUCCEEDED(ice_ctx_->SetStunServers(stun_servers)));
     }
 
     r = 1;
@@ -117,7 +117,7 @@ class MultiTcpSocketTest : public MtransportTest {
         (char *)"127.0.0.1", EnsureEphemeral(port_s++), IPPROTO_TCP, &local);
       ASSERT_EQ(0, r);
 
-      r = nr_socket_multi_tcp_create(ice_ctx_->ctx()->ctx(),
+      r = nr_socket_multi_tcp_create(ice_ctx_->ctx(),
           &local, tcp_type, 1, 2048, sock);
     }
 
@@ -347,7 +347,7 @@ class MultiTcpSocketTest : public MtransportTest {
   }
   std::vector<nr_socket *> socks;
   Atomic<bool> readable;
-  RefPtr<NrIceCtxHandler> ice_ctx_;
+  RefPtr<NrIceCtx> ice_ctx_;
 };
 }
 

media/mtransport/test/test_nr_socket_ice_unittest.cpp

@@ -104,7 +104,10 @@ public:
 
     nr_ice_add_media_stream(ice_ctx_,
                             const_cast<char *>(name_.c_str()),
+                            "ufrag",
+                            "pass",
                             2, &ice_media_stream_);
+    EXPECT_EQ(2UL, GetStreamAttributes().size());
 
     nr_ice_media_stream_initialize(ice_ctx_, ice_media_stream_);
   }
@@ -144,30 +147,46 @@ public:
     ASSERT_TRUE(r == 0 || r == R_WOULDBLOCK);
   }
 
-  std::vector<std::string> GetLocalCandidates() const {
-    char attr[256];
-    std::vector<std::string> candidates;
-    nr_ice_component* comp = STAILQ_FIRST(&ice_media_stream_->components);
-    while(comp){
-      if (comp->state != NR_ICE_COMPONENT_DISABLED) {
-        nr_ice_candidate *cand = TAILQ_FIRST(&comp->candidates);
-        while(cand){
-          int r = nr_ice_format_candidate_attribute(cand, attr, 255);
-          if (r == 0) {
-            candidates.push_back(attr);
-          }
+  std::vector<std::string> GetStreamAttributes() {
+    std::vector<std::string> attributes;
+    test_utils_->sts_target()->Dispatch(
+        WrapRunnableRet(&attributes,
+                        this,
+                        &IcePeer::GetStreamAttributes_s),
+        NS_DISPATCH_SYNC);
+    return attributes;
+  }
 
-          cand = TAILQ_NEXT(cand, entry_comp);
-        }
-      }
+  std::vector<std::string> GetStreamAttributes_s() {
 
-      comp = STAILQ_NEXT(comp, entry);
+    char **attrs = nullptr;
+    int attrct;
+    std::vector<std::string> ret;
+
+    int r =
+      nr_ice_media_stream_get_attributes(ice_media_stream_, &attrs, &attrct);
+    EXPECT_EQ(0, r);
+
+    for (int i=0; i<attrct; i++) {
+      ret.push_back(std::string(attrs[i]));
+      RFREE(attrs[i]);
     }
+    RFREE(attrs);
 
-    return candidates;
+    return ret;
   }
 
   std::vector<std::string> GetGlobalAttributes() {
+    std::vector<std::string> attributes;
+    test_utils_->sts_target()->Dispatch(
+        WrapRunnableRet(&attributes,
+                        this,
+                        &IcePeer::GetGlobalAttributes_s),
+        NS_DISPATCH_SYNC);
+    return attributes;
+  }
+
+  std::vector<std::string> GetGlobalAttributes_s() {
 
     char **attrs = nullptr;
     int attrct;
@@ -203,6 +222,14 @@ public:
   }
 
   void SetRemoteAttributes(std::vector<std::string> attributes) {
+    test_utils_->sts_target()->Dispatch(
+        WrapRunnable(this,
+                     &IcePeer::SetRemoteAttributes_s,
+                     attributes),
+        NS_DISPATCH_SYNC);
+  }
+
+  void SetRemoteAttributes_s(std::vector<std::string> attributes) {
     int r;
 
     std::vector<char*> attrs;
@@ -329,11 +356,11 @@ TEST_F(TestNrSocketIceUnitTest, TestIcePeer) {
   ASSERT_NE(peer.ice_ctx_, nullptr);
   ASSERT_NE(peer.peer_ctx_, nullptr);
   ASSERT_NE(peer.ice_media_stream_, nullptr);
+  ASSERT_EQ(2UL, peer.GetStreamAttributes().size())
+    << "Should have ice-ufrag and ice-pwd";
   peer.Gather();
-  std::vector<std::string> attrs = peer.GetGlobalAttributes();
-  ASSERT_NE(attrs.size(), 0UL);
-  std::vector<std::string> candidates = peer.GetLocalCandidates();
-  ASSERT_NE(candidates.size(), 0UL);
+  ASSERT_LT(2UL, peer.GetStreamAttributes().size())
+    << "Should have ice-ufrag, ice-pwd, and at least one candidate.";
 }
 
 TEST_F(TestNrSocketIceUnitTest, TestIcePeersNoNAT) {
@@ -348,13 +375,13 @@ TEST_F(TestNrSocketIceUnitTest, TestIcePeersNoNAT) {
   peer2.Gather();
   std::vector<std::string> attrs = peer.GetGlobalAttributes();
   peer2.ParseGlobalAttributes(attrs);
-  std::vector<std::string> candidates = peer.GetLocalCandidates();
-  peer2.SetRemoteAttributes(candidates);
+  std::vector<std::string> attributes = peer.GetStreamAttributes();
+  peer2.SetRemoteAttributes(attributes);
 
   attrs = peer2.GetGlobalAttributes();
   peer.ParseGlobalAttributes(attrs);
-  candidates = peer2.GetLocalCandidates();
-  peer.SetRemoteAttributes(candidates);
+  attributes = peer2.GetStreamAttributes();
+  peer.SetRemoteAttributes(attributes);
   peer2.StartChecks();
   peer.StartChecks();
 
@@ -406,13 +433,13 @@ TEST_F(TestNrSocketIceUnitTest, TestIcePeersPacketLoss) {
   peer2.Gather();
   std::vector<std::string> attrs = peer.GetGlobalAttributes();
   peer2.ParseGlobalAttributes(attrs);
-  std::vector<std::string> candidates = peer.GetLocalCandidates();
-  peer2.SetRemoteAttributes(candidates);
+  std::vector<std::string> attributes = peer.GetStreamAttributes();
+  peer2.SetRemoteAttributes(attributes);
 
   attrs = peer2.GetGlobalAttributes();
   peer.ParseGlobalAttributes(attrs);
-  candidates = peer2.GetLocalCandidates();
-  peer.SetRemoteAttributes(candidates);
+  attributes = peer2.GetStreamAttributes();
+  peer.SetRemoteAttributes(attributes);
   peer2.StartChecks();
   peer.StartChecks();
 

media/mtransport/test/transport_unittests.cpp

@@ -26,7 +26,7 @@
 
 #include "mediapacket.h"
 #include "dtlsidentity.h"
-#include "nricectxhandler.h"
+#include "nricectx.h"
 #include "nricemediastream.h"
 #include "transportflow.h"
 #include "transportlayer.h"
@@ -443,8 +443,8 @@ class TransportTestPeer : public sigslot::has_slots<> {
         lossy_(new TransportLayerLossy()),
         dtls_(new TransportLayerDtls()),
         identity_(DtlsIdentity::Generate()),
-        ice_ctx_(NrIceCtxHandler::Create(name)),
-        streams_(), candidates_(),
+        ice_ctx_(NrIceCtx::Create(name)),
+        streams_(),
         peer_(nullptr),
         gathering_complete_(false),
         enabled_cipersuites_(),
@@ -454,7 +454,7 @@ class TransportTestPeer : public sigslot::has_slots<> {
     UniquePtr<NrIceStunServer> server(NrIceStunServer::Create(
         std::string((char *)"stun.services.mozilla.com"), 3478));
     stun_servers.push_back(*server);
-    EXPECT_TRUE(NS_SUCCEEDED(ice_ctx_->ctx()->SetStunServers(stun_servers)));
+    EXPECT_TRUE(NS_SUCCEEDED(ice_ctx_->SetStunServers(stun_servers)));
 
     dtls_->SetIdentity(identity_);
     dtls_->SetRole(offerer_ ?
@@ -610,7 +610,7 @@ class TransportTestPeer : public sigslot::has_slots<> {
     nsresult res;
 
     // Attach our slots
-    ice_ctx_->ctx()->SignalGatheringStateChange.
+    ice_ctx_->SignalGatheringStateChange.
         connect(this, &TransportTestPeer::GatheringStateChange);
 
     char name[100];
@@ -618,11 +618,10 @@ class TransportTestPeer : public sigslot::has_slots<> {
              (int)streams_.size());
 
     // Create the media stream
-    RefPtr<NrIceMediaStream> stream =
-        ice_ctx_->CreateStream(static_cast<char *>(name), 1);
+    RefPtr<NrIceMediaStream> stream = ice_ctx_->CreateStream(name, name, 1);
 
     ASSERT_TRUE(stream != nullptr);
-    ice_ctx_->ctx()->SetStream(name, stream);
+    stream->SetIceCredentials("ufrag", "pass");
     streams_.push_back(stream);
 
     // Listen for candidates
@@ -646,7 +645,7 @@ class TransportTestPeer : public sigslot::has_slots<> {
     // Start gathering
     test_utils_->sts_target()->Dispatch(
         WrapRunnableRet(&res,
-                        ice_ctx_->ctx(),
+                        ice_ctx_,
                         &NrIceCtx::StartGathering,
                         false,
                         false),
@@ -665,7 +664,6 @@ class TransportTestPeer : public sigslot::has_slots<> {
   // New candidate
   void GotCandidate(NrIceMediaStream *stream, const std::string &candidate) {
     std::cerr << "Got candidate " << candidate << std::endl;
-    candidates_[stream->name()].push_back(candidate);
   }
 
   void GatheringStateChange(NrIceCtx* ctx,
@@ -689,23 +687,23 @@ class TransportTestPeer : public sigslot::has_slots<> {
 
     // First send attributes
     test_utils_->sts_target()->Dispatch(
-      WrapRunnableRet(&res, peer_->ice_ctx_->ctx(),
+      WrapRunnableRet(&res, peer_->ice_ctx_,
                       &NrIceCtx::ParseGlobalAttributes,
-                      ice_ctx_->ctx()->GetGlobalAttributes()),
+                      ice_ctx_->GetGlobalAttributes()),
       NS_DISPATCH_SYNC);
     ASSERT_TRUE(NS_SUCCEEDED(res));
 
     for (size_t i=0; i<streams_.size(); ++i) {
       test_utils_->sts_target()->Dispatch(
-        WrapRunnableRet(&res, peer_->streams_[i], &NrIceMediaStream::ParseAttributes,
-                        candidates_[streams_[i]->name()]), NS_DISPATCH_SYNC);
+        WrapRunnableRet(&res, peer_->streams_[i], &NrIceMediaStream::ConnectToPeer,
+                        "ufrag", "pass", streams_[i]->GetAttributes()), NS_DISPATCH_SYNC);
 
       ASSERT_TRUE(NS_SUCCEEDED(res));
     }
 
     // Start checks on the other peer.
     test_utils_->sts_target()->Dispatch(
-      WrapRunnableRet(&res, peer_->ice_ctx_->ctx(), &NrIceCtx::StartChecks,
+      WrapRunnableRet(&res, peer_->ice_ctx_, &NrIceCtx::StartChecks,
                       offerer_),
       NS_DISPATCH_SYNC);
     ASSERT_TRUE(NS_SUCCEEDED(res));
@@ -831,9 +829,8 @@ class TransportTestPeer : public sigslot::has_slots<> {
   TransportLayerDtls *dtls_;
   TransportLayerIce *ice_;
   RefPtr<DtlsIdentity> identity_;
-  RefPtr<NrIceCtxHandler> ice_ctx_;
+  RefPtr<NrIceCtx> ice_ctx_;
   std::vector<RefPtr<NrIceMediaStream> > streams_;
-  std::map<std::string, std::vector<std::string> > candidates_;
   TransportTestPeer *peer_;
   bool gathering_complete_;
   unsigned char fingerprint_[TransportLayerDtls::kMaxDigestLength];

media/mtransport/test/turn_unittest.cpp

@@ -74,8 +74,7 @@ extern "C" {
 }
 
 #include "nricemediastream.h"
-#include "nricectxhandler.h"
-
+#include "nricectx.h"
 
 using namespace mozilla;
 

media/mtransport/third_party/nICEr/src/ice/ice_candidate.c

@@ -306,11 +306,7 @@ int nr_ice_candidate_destroy(nr_ice_candidate **candp)
 
     cand=*candp;
 
-    if (cand->state == NR_ICE_CAND_STATE_INITIALIZING) {
-      /* Make sure the ICE ctx isn't still waiting around for this candidate
-       * to init. */
-      nr_ice_candidate_mark_done(cand, NR_ICE_CAND_STATE_FAILED);
-    }
+    nr_ice_candidate_stop_gathering(cand);
 
     switch(cand->type){
       case HOST:
@@ -350,12 +346,6 @@ int nr_ice_candidate_destroy(nr_ice_candidate **candp)
         break;
     }
 
-    NR_async_timer_cancel(cand->delay_timer);
-    NR_async_timer_cancel(cand->ready_cb_timer);
-    if(cand->resolver_handle){
-      nr_resolver_cancel(cand->ctx->resolver,cand->resolver_handle);
-    }
-
     RFREE(cand->foundation);
     RFREE(cand->label);
     RFREE(cand);
@@ -363,6 +353,25 @@ int nr_ice_candidate_destroy(nr_ice_candidate **candp)
     return(0);
   }
 
+void nr_ice_candidate_stop_gathering(nr_ice_candidate *cand)
+  {
+    if (cand->state == NR_ICE_CAND_STATE_INITIALIZING) {
+      /* Make sure the ICE ctx isn't still waiting around for this candidate
+       * to init. */
+      nr_ice_candidate_mark_done(cand, NR_ICE_CAND_STATE_FAILED);
+    }
+
+    NR_async_timer_cancel(cand->delay_timer);
+    cand->delay_timer=0;
+    NR_async_timer_cancel(cand->ready_cb_timer);
+    cand->ready_cb_timer=0;
+
+    if(cand->resolver_handle){
+      nr_resolver_cancel(cand->ctx->resolver,cand->resolver_handle);
+      cand->resolver_handle=0;
+    }
+  }
+
 /* This algorithm is not super-fast, but I don't think we need a hash
    table just yet and it produces a small foundation string */
 static int nr_ice_get_foundation(nr_ice_ctx *ctx,nr_ice_candidate *cand)

media/mtransport/third_party/nICEr/src/ice/ice_candidate.h

@@ -110,6 +110,7 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo
 void nr_ice_candidate_compute_codeword(nr_ice_candidate *cand);
 int nr_ice_candidate_process_stun(nr_ice_candidate *cand, UCHAR *msg, int len, nr_transport_addr *faddr);
 int nr_ice_candidate_destroy(nr_ice_candidate **candp);
+void nr_ice_candidate_stop_gathering(nr_ice_candidate *cand);
 int nr_ice_format_candidate_attribute(nr_ice_candidate *cand, char *attr, int maxlen);
 int nr_ice_peer_candidate_from_attribute(nr_ice_ctx *ctx,char *attr,nr_ice_media_stream *stream,nr_ice_candidate **candp);
 int nr_ice_peer_peer_rflx_candidate_create(nr_ice_ctx *ctx,char *label, nr_ice_component *comp,nr_transport_addr *addr, nr_ice_candidate **candp);

media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c

@@ -497,7 +497,7 @@ int nr_ice_candidate_pair_do_triggered_check(nr_ice_peer_ctx *pctx, nr_ice_cand_
     return(_status);
   }
 
-int nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, int move_to_wait_state)
+void nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, int move_to_wait_state)
   {
     if(pair->state != NR_ICE_PAIR_STATE_FAILED){
       /* If it's already running we need to terminate the stun */
@@ -510,8 +510,6 @@ int nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, i
       }
       nr_ice_candidate_pair_set_state(pctx,pair,NR_ICE_PAIR_STATE_CANCELLED);
     }
-
-    return(0);
   }
 
 int nr_ice_candidate_pair_select(nr_ice_cand_pair *pair)
@@ -545,10 +543,8 @@ int nr_ice_candidate_pair_select(nr_ice_cand_pair *pair)
     return(_status);
  }
 
-int nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair, int state)
+void nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair, int state)
   {
-    int r,_status;
-
     r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/CAND-PAIR(%s): setting pair to state %s: %s",
       pctx->label,pair->codeword,nr_ice_cand_pair_states[state],pair->as_string);
 
@@ -580,13 +576,8 @@ int nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pai
 
     if(pair->state==NR_ICE_PAIR_STATE_FAILED ||
        pair->state==NR_ICE_PAIR_STATE_CANCELLED){
-      if(r=nr_ice_component_failed_pair(pair->remote->component, pair))
-        ABORT(r);
+      nr_ice_component_failed_pair(pair->remote->component, pair);
     }
-
-    _status=0;
-  abort:
-    return(_status);
   }
 
 int nr_ice_candidate_pair_dump_state(nr_ice_cand_pair *pair, FILE *out)

media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.h

@@ -83,9 +83,9 @@ struct nr_ice_cand_pair_ {
 int nr_ice_candidate_pair_create(nr_ice_peer_ctx *pctx, nr_ice_candidate *lcand,nr_ice_candidate *rcand,nr_ice_cand_pair **pairp);
 int nr_ice_candidate_pair_unfreeze(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_start(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
-int nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair, int state);
+void nr_ice_candidate_pair_set_state(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair, int state);
 int nr_ice_candidate_pair_dump_state(nr_ice_cand_pair *pair, FILE *out);
-int nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, int move_to_wait_state);
+void nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, int move_to_wait_state);
 int nr_ice_candidate_pair_select(nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_do_triggered_check(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair);
 int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair);

media/mtransport/third_party/nICEr/src/ice/ice_component.c

@@ -657,11 +657,11 @@ int nr_ice_component_initialize(struct nr_ice_ctx_ *ctx,nr_ice_component *compon
 
     /* Note: we need to recompute these because
        we have not yet computed the values in the peer media stream.*/
-    lufrag=component->stream->ufrag ? component->stream->ufrag : ctx->ufrag;
+    lufrag=component->stream->ufrag;
     assert(lufrag);
     if (!lufrag)
       ABORT(R_INTERNAL);
-    lpwd=component->stream->pwd ? component->stream->pwd :ctx->pwd;
+    lpwd=component->stream->pwd;
     assert(lpwd);
     if (!lpwd)
       ABORT(R_INTERNAL);
@@ -699,6 +699,14 @@ int nr_ice_component_initialize(struct nr_ice_ctx_ *ctx,nr_ice_component *compon
     return(_status);
   }
 
+void nr_ice_component_stop_gathering(nr_ice_component *component)
+  {
+    nr_ice_candidate *c1,*c2;
+    TAILQ_FOREACH_SAFE(c1, &component->candidates, entry_comp, c2){
+      nr_ice_candidate_stop_gathering(c1);
+    }
+  }
+
 static int nr_ice_any_peer_paired(nr_ice_candidate* cand) {
   nr_ice_peer_ctx* pctx=STAILQ_FIRST(&cand->ctx->peers);
   while(pctx && pctx->state == NR_ICE_PEER_STATE_UNPAIRED){
@@ -1254,9 +1262,7 @@ static void nr_ice_component_consent_failed(nr_ice_component *comp)
     }
     /* We are turning the consent failure into a ICE component failure to
      * alert the browser via ICE connection state change about this event. */
-    if (nr_ice_media_stream_component_failed(comp->stream, comp))
-      r_log(LOG_ICE,LOG_ERR,"ICE(%s)/STREAM(%s)/COMP(%d): failed to mark component as failed",
-        comp->ctx->label, comp->stream->label, comp->component_id);
+    nr_ice_media_stream_component_failed(comp->stream, comp);
   }
 
 static void nr_ice_component_consent_timeout_cb(NR_SOCKET s, int how, void *cb_arg)
@@ -1515,8 +1521,7 @@ int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pa
                p2->state == NR_ICE_PAIR_STATE_CANCELLED);
         r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/COMP(%d)/CAND-PAIR(%s): cancelling FROZEN/WAITING pair %s in trigger check queue because CAND-PAIR(%s) was nominated.",comp->stream->pctx->label,comp->stream->label,comp->component_id,p2->codeword,p2->as_string,pair->codeword);
 
-        if(r=nr_ice_candidate_pair_cancel(pair->pctx,p2,0))
-          ABORT(r);
+        nr_ice_candidate_pair_cancel(pair->pctx,p2,0);
       }
 
       p2=TAILQ_NEXT(p2,triggered_check_queue_entry);
@@ -1529,8 +1534,7 @@ int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pa
           (p2->state == NR_ICE_PAIR_STATE_WAITING))) {
         r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/COMP(%d)/CAND-PAIR(%s): cancelling FROZEN/WAITING pair %s because CAND-PAIR(%s) was nominated.",comp->stream->pctx->label,comp->stream->label,comp->component_id,p2->codeword,p2->as_string,pair->codeword);
 
-        if(r=nr_ice_candidate_pair_cancel(pair->pctx,p2,0))
-          ABORT(r);
+        nr_ice_candidate_pair_cancel(pair->pctx,p2,0);
       }
 
       p2=TAILQ_NEXT(p2,check_queue_entry);
@@ -1540,8 +1544,7 @@ int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pa
     if(r=nr_ice_component_setup_consent(comp))
       ABORT(r);
 
-    if(r=nr_ice_media_stream_component_nominated(comp->stream,comp))
-      ABORT(r);
+    nr_ice_media_stream_component_nominated(comp->stream,comp);
 
     _status=0;
   abort:
@@ -1577,12 +1580,12 @@ static int nr_ice_component_have_all_pairs_failed(nr_ice_component *comp)
     return(1);
   }
 
-int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
+void nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair)
   {
-    return nr_ice_component_check_if_failed(comp);
+    nr_ice_component_check_if_failed(comp);
   }
 
-int nr_ice_component_check_if_failed(nr_ice_component *comp)
+void nr_ice_component_check_if_failed(nr_ice_component *comp)
   {
     if (comp->state == NR_ICE_COMPONENT_RUNNING) {
       /* Don't do anything to streams that aren't currently running */
@@ -1591,11 +1594,9 @@ int nr_ice_component_check_if_failed(nr_ice_component *comp)
       if (!comp->stream->pctx->trickle_grace_period_timer &&
           nr_ice_component_have_all_pairs_failed(comp)) {
         r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/COMP(%d): All pairs are failed, and grace period has elapsed. Marking component as failed.",comp->stream->pctx->label,comp->stream->label,comp->component_id);
-        return nr_ice_media_stream_component_failed(comp->stream,comp);
+        nr_ice_media_stream_component_failed(comp->stream,comp);
       }
     }
-
-    return(0);
   }
 
 int nr_ice_component_select_pair(nr_ice_peer_ctx *pctx, nr_ice_component *comp)

media/mtransport/third_party/nICEr/src/ice/ice_component.h

@@ -86,13 +86,14 @@ typedef STAILQ_HEAD(nr_ice_component_head_,nr_ice_component_) nr_ice_component_h
 int nr_ice_component_create(struct nr_ice_media_stream_ *stream, int component_id, nr_ice_component **componentp);
 int nr_ice_component_destroy(nr_ice_component **componentp);
 int nr_ice_component_initialize(struct nr_ice_ctx_ *ctx,nr_ice_component *component);
+void nr_ice_component_stop_gathering(nr_ice_component *component);
 int nr_ice_component_maybe_prune_candidate(nr_ice_ctx *ctx, nr_ice_component *comp, nr_ice_candidate *c1, int *was_pruned);
 int nr_ice_component_pair_candidate(nr_ice_peer_ctx *pctx, nr_ice_component *pcomp, nr_ice_candidate *lcand, int pair_all_remote);
 int nr_ice_component_pair_candidates(nr_ice_peer_ctx *pctx, nr_ice_component *lcomp, nr_ice_component *pcomp);
 int nr_ice_component_service_pre_answer_requests(nr_ice_peer_ctx *pctx, nr_ice_component *pcomp, char *username, int *serviced);
 int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
-int nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
-int nr_ice_component_check_if_failed(nr_ice_component *comp);
+void nr_ice_component_failed_pair(nr_ice_component *comp, nr_ice_cand_pair *pair);
+void nr_ice_component_check_if_failed(nr_ice_component *comp);
 int nr_ice_component_select_pair(nr_ice_peer_ctx *pctx, nr_ice_component *comp);
 int nr_ice_component_set_failed(nr_ice_component *comp);
 int nr_ice_component_finalize(nr_ice_component *lcomp, nr_ice_component *rcomp);

media/mtransport/third_party/nICEr/src/ice/ice_ctx.c

@@ -338,28 +338,6 @@ int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out)
 
 #define MAXADDRS 100 /* Ridiculously high */
 int nr_ice_ctx_create(char *label, UINT4 flags, nr_ice_ctx **ctxp)
-  {
-    int r,_status;
-    char *ufrag = 0;
-    char *pwd = 0;
-
-    if (r=nr_ice_get_new_ice_ufrag(&ufrag))
-      ABORT(r);
-    if (r=nr_ice_get_new_ice_pwd(&pwd))
-      ABORT(r);
-
-    if (r=nr_ice_ctx_create_with_credentials(label, flags, ufrag, pwd, ctxp))
-      ABORT(r);
-
-    _status=0;
-  abort:
-    RFREE(ufrag);
-    RFREE(pwd);
-
-    return(_status);
-  }
-
-int nr_ice_ctx_create_with_credentials(char *label, UINT4 flags, char *ufrag, char *pwd, nr_ice_ctx **ctxp)
   {
     nr_ice_ctx *ctx=0;
     int r,_status;
@@ -375,11 +353,6 @@ int nr_ice_ctx_create_with_credentials(char *label, UINT4 flags, char *ufrag, ch
     if(!(ctx->label=r_strdup(label)))
       ABORT(R_NO_MEMORY);
 
-    if(!(ctx->ufrag=r_strdup(ufrag)))
-      ABORT(r);
-    if(!(ctx->pwd=r_strdup(pwd)))
-      ABORT(r);
-
     /* Get the STUN servers */
     if(r=NR_reg_get_child_count(NR_ICE_REG_STUN_SRV_PRFX,
       (unsigned int *)&ctx->stun_server_ct)||ctx->stun_server_ct==0) {
@@ -494,8 +467,6 @@ static void nr_ice_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg)
       RFREE(f1);
       f1=f2;
     }
-    RFREE(ctx->pwd);
-    RFREE(ctx->ufrag);
 
     STAILQ_FOREACH_SAFE(id1, &ctx->ids, entry, id2){
       STAILQ_REMOVE(&ctx->ids,id1,nr_ice_stun_id_,entry);
@@ -832,11 +803,11 @@ int nr_ice_gather(nr_ice_ctx *ctx, NR_async_cb done_cb, void *cb_arg)
     return(_status);
   }
 
-int nr_ice_add_media_stream(nr_ice_ctx *ctx,char *label,int components, nr_ice_media_stream **streamp)
+int nr_ice_add_media_stream(nr_ice_ctx *ctx,const char *label,const char *ufrag,const char *pwd,int components, nr_ice_media_stream **streamp)
   {
     int r,_status;
 
-    if(r=nr_ice_media_stream_create(ctx,label,components,streamp))
+    if(r=nr_ice_media_stream_create(ctx,label,ufrag,pwd,components,streamp))
       ABORT(r);
 
     STAILQ_INSERT_TAIL(&ctx->streams,*streamp,entry);
@@ -875,36 +846,9 @@ int nr_ice_remove_media_stream(nr_ice_ctx *ctx,nr_ice_media_stream **streamp)
 
 int nr_ice_get_global_attributes(nr_ice_ctx *ctx,char ***attrsp, int *attrctp)
   {
-    char **attrs=0;
-    int _status;
-    char *tmp=0;
-
-    if(!(attrs=RCALLOC(sizeof(char *)*2)))
-      ABORT(R_NO_MEMORY);
-
-    if(!(tmp=RMALLOC(100)))
-      ABORT(R_NO_MEMORY);
-    snprintf(tmp,100,"ice-ufrag:%s",ctx->ufrag);
-    attrs[0]=tmp;
-
-    if(!(tmp=RMALLOC(100)))
-      ABORT(R_NO_MEMORY);
-    snprintf(tmp,100,"ice-pwd:%s",ctx->pwd);
-    attrs[1]=tmp;
-
-    *attrctp=2;
-    *attrsp=attrs;
-
-    _status=0;
-  abort:
-    if (_status){
-      if (attrs){
-        RFREE(attrs[0]);
-        RFREE(attrs[1]);
-      }
-      RFREE(attrs);
-    }
-    return(_status);
+    *attrctp=0;
+    *attrsp=0;
+    return(0);
   }
 
 static int nr_ice_random_string(char *str, int len)

media/mtransport/third_party/nICEr/src/ice/ice_ctx.h

@@ -123,9 +123,6 @@ struct nr_ice_ctx_ {
   UINT4 flags;
   char *label;
 
-  char *ufrag;
-  char *pwd;
-
   UINT4 Ta;
 
   nr_ice_stun_server *stun_servers;           /* The list of stun servers */
@@ -181,7 +178,7 @@ int nr_ice_set_local_addresses(nr_ice_ctx *ctx, nr_local_addr* stun_addrs, int s
 int nr_ice_gather(nr_ice_ctx *ctx, NR_async_cb done_cb, void *cb_arg);
 int nr_ice_add_candidate(nr_ice_ctx *ctx, nr_ice_candidate *cand);
 void nr_ice_gather_finished_cb(NR_SOCKET s, int h, void *cb_arg);
-int nr_ice_add_media_stream(nr_ice_ctx *ctx,char *label,int components, nr_ice_media_stream **streamp);
+int nr_ice_add_media_stream(nr_ice_ctx *ctx,const char *label,const char *ufrag,const char *pwd,int components, nr_ice_media_stream **streamp);
 int nr_ice_remove_media_stream(nr_ice_ctx *ctx,nr_ice_media_stream **streamp);
 int nr_ice_get_global_attributes(nr_ice_ctx *ctx,char ***attrsp, int *attrctp);
 int nr_ice_ctx_deliver_packet(nr_ice_ctx *ctx, nr_ice_component *comp, nr_transport_addr *source_addr, UCHAR *data, int len);

media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c

@@ -44,7 +44,7 @@ static char *nr_ice_media_stream_states[]={"INVALID",
 
 int nr_ice_media_stream_set_state(nr_ice_media_stream *str, int state);
 
-int nr_ice_media_stream_create(nr_ice_ctx *ctx,char *label,int components, nr_ice_media_stream **streamp)
+int nr_ice_media_stream_create(nr_ice_ctx *ctx,const char *label,const char *ufrag,const char *pwd,int components, nr_ice_media_stream **streamp)
   {
     int r,_status;
     nr_ice_media_stream *stream=0;
@@ -57,6 +57,12 @@ int nr_ice_media_stream_create(nr_ice_ctx *ctx,char *label,int components, nr_ic
     if(!(stream->label=r_strdup(label)))
       ABORT(R_NO_MEMORY);
 
+    if(!(stream->ufrag=r_strdup(ufrag)))
+      ABORT(R_NO_MEMORY);
+
+    if(!(stream->pwd=r_strdup(pwd)))
+      ABORT(R_NO_MEMORY);
+
     stream->ctx=ctx;
 
     STAILQ_INIT(&stream->components);
@@ -73,6 +79,9 @@ int nr_ice_media_stream_create(nr_ice_ctx *ctx,char *label,int components, nr_ic
     stream->disconnected = 0;
     stream->component_ct=components;
     stream->ice_state = NR_ICE_MEDIA_STREAM_UNPAIRED;
+    stream->obsolete = 0;
+    stream->r2l_user = 0;
+    stream->l2r_user = 0;
     *streamp=stream;
 
     _status=0;
@@ -143,12 +152,13 @@ int nr_ice_media_stream_initialize(nr_ice_ctx *ctx, nr_ice_media_stream *stream)
 
 int nr_ice_media_stream_get_attributes(nr_ice_media_stream *stream, char ***attrsp, int *attrctp)
   {
-    int attrct=0;
+    int attrct=2;
     nr_ice_component *comp;
     char **attrs=0;
     int index=0;
     nr_ice_candidate *cand;
     int r,_status;
+    char *tmp=0;
 
     *attrctp=0;
 
@@ -168,11 +178,6 @@ int nr_ice_media_stream_get_attributes(nr_ice_media_stream *stream, char ***attr
       comp=STAILQ_NEXT(comp,entry);
     }
 
-    if(attrct < 1){
-      r_log(LOG_ICE,LOG_ERR,"ICE-STREAM(%s): Failed to find any components for stream",stream->label);
-      ABORT(R_FAILED);
-    }
-
     /* Make the array we'll need */
     if(!(attrs=RCALLOC(sizeof(char *)*attrct)))
       ABORT(R_NO_MEMORY);
@@ -208,6 +213,17 @@ int nr_ice_media_stream_get_attributes(nr_ice_media_stream *stream, char ***attr
       comp=STAILQ_NEXT(comp,entry);
     }
 
+    /* Now, ufrag and pwd */
+    if(!(tmp=RMALLOC(100)))
+      ABORT(R_NO_MEMORY);
+    snprintf(tmp,100,"ice-ufrag:%s",stream->ufrag);
+    attrs[index++]=tmp;
+
+    if(!(tmp=RMALLOC(100)))
+      ABORT(R_NO_MEMORY);
+    snprintf(tmp,100,"ice-pwd:%s",stream->pwd);
+    attrs[index++]=tmp;
+
     *attrsp=attrs;
     *attrctp=attrct;
 
@@ -392,6 +408,11 @@ int nr_ice_media_stream_start_checks(nr_ice_peer_ctx *pctx, nr_ice_media_stream
       ABORT(R_INTERNAL);
     }
 
+    if (stream->local_stream->obsolete) {
+      assert(0);
+      ABORT(R_INTERNAL);
+    }
+
     /* Even if the stream is completed already remote can still create a new
      * triggered check request which needs to fire, but not change our stream
      * state. */
@@ -587,6 +608,42 @@ int nr_ice_media_stream_set_state(nr_ice_media_stream *str, int state)
     return(0);
   }
 
+void nr_ice_media_stream_stop_checking(nr_ice_media_stream *str)
+  {
+    nr_ice_cand_pair *p;
+    nr_ice_component *comp;
+
+    /* Cancel candidate pairs */
+    p=TAILQ_FIRST(&str->check_list);
+    while(p){
+      nr_ice_candidate_pair_cancel(p->pctx,p,0);
+      p=TAILQ_NEXT(p,check_queue_entry);
+    }
+
+    if(str->timer) {
+      NR_async_timer_cancel(str->timer);
+      str->timer = 0;
+    }
+
+    /* Cancel consent timers in case it is running already */
+    comp=STAILQ_FIRST(&str->components);
+    while(comp){
+      nr_ice_component_consent_destroy(comp);
+      comp=STAILQ_NEXT(comp,entry);
+    }
+  }
+
+void nr_ice_media_stream_set_obsolete(nr_ice_media_stream *str)
+  {
+    nr_ice_component *c1,*c2;
+    str->obsolete = 1;
+
+    STAILQ_FOREACH_SAFE(c1, &str->components, entry, c2){
+      nr_ice_component_stop_gathering(c1);
+    }
+
+    nr_ice_media_stream_stop_checking(str);
+  }
 
 void nr_ice_media_stream_refresh_consent_all(nr_ice_media_stream *stream)
   {
@@ -626,7 +683,9 @@ void nr_ice_media_stream_set_disconnected(nr_ice_media_stream *stream, int disco
     stream->disconnected = disconnected;
 
     if (disconnected == NR_ICE_MEDIA_STREAM_DISCONNECTED) {
-      nr_ice_peer_ctx_disconnected(stream->pctx);
+      if (!stream->local_stream->obsolete) {
+        nr_ice_peer_ctx_disconnected(stream->pctx);
+      }
     } else {
       nr_ice_peer_ctx_check_if_connected(stream->pctx);
     }
@@ -658,9 +717,8 @@ int nr_ice_media_stream_check_if_connected(nr_ice_media_stream *stream)
 
 /* S OK, this component has a nominated. If every component has a nominated,
    the stream is ready */
-int nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_component *component)
+void nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_component *component)
   {
-    int r,_status;
     nr_ice_component *comp;
 
     comp=STAILQ_FIRST(&stream->components);
@@ -675,7 +733,7 @@ int nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_c
 
     /* At least one un-nominated component */
     if(comp)
-      goto done;
+      return;
 
     /* All done... */
     r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/ICE-STREAM(%s): all active components have nominated candidate pairs",stream->pctx->label,stream->label);
@@ -687,25 +745,16 @@ int nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_c
       stream->timer=0;
     }
 
-    if (stream->pctx->handler) {
+    if (stream->pctx->handler && !stream->local_stream->obsolete) {
       stream->pctx->handler->vtbl->stream_ready(stream->pctx->handler->obj,stream->local_stream);
     }
 
     /* Now tell the peer_ctx that we're connected */
-    if(r=nr_ice_peer_ctx_check_if_connected(stream->pctx))
-      ABORT(r);
-
-  done:
-    _status=0;
-  abort:
-    return(_status);
+    nr_ice_peer_ctx_check_if_connected(stream->pctx);
   }
 
-int nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_component *component)
+void nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_component *component)
   {
-    int r,_status;
-    nr_ice_cand_pair *p2;
-
     component->state=NR_ICE_COMPONENT_FAILED;
 
     /* at least one component failed in this media stream, so the entire
@@ -713,35 +762,14 @@ int nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_comp
 
     nr_ice_media_stream_set_state(stream,NR_ICE_MEDIA_STREAM_CHECKS_FAILED);
 
-    /* OK, we need to cancel off everything on this component */
-    p2=TAILQ_FIRST(&stream->check_list);
-    while(p2){
-      if(r=nr_ice_candidate_pair_cancel(p2->pctx,p2,0))
-        ABORT(r);
+    nr_ice_media_stream_stop_checking(stream);
 
-      p2=TAILQ_NEXT(p2,check_queue_entry);
-    }
-
-    /* Cancel our timer */
-    if(stream->timer){
-      NR_async_timer_cancel(stream->timer);
-      stream->timer=0;
-    }
-
-    /* Cancel consent timers in case it is running already */
-    nr_ice_component_consent_destroy(component);
-
-    if (stream->pctx->handler) {
+    if (stream->pctx->handler && !stream->local_stream->obsolete) {
       stream->pctx->handler->vtbl->stream_failed(stream->pctx->handler->obj,stream->local_stream);
     }
 
-    /* Now tell the peer_ctx that we're connected */
-    if(r=nr_ice_peer_ctx_check_if_connected(stream->pctx))
-      ABORT(r);
-
-    _status=0;
-  abort:
-    return(_status);
+    /* Now tell the peer_ctx that we've failed */
+    nr_ice_peer_ctx_check_if_connected(stream->pctx);
   }
 
 int nr_ice_media_stream_get_best_candidate(nr_ice_media_stream *str, int component, nr_ice_candidate **candp)

media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h

@@ -55,6 +55,10 @@ struct nr_ice_media_stream_ {
   Data r2l_pass;   /* The password for incoming requests */
   Data l2r_pass;   /* The password for outcoming requests */
   int ice_state;
+  /* The stream is being replaced by another, so it will not continue any ICE
+   * processing. If this stream is connected already, traffic can continue to
+   * flow for a limited time while the new stream gets ready. */
+  int obsolete;
 
 #define NR_ICE_MEDIA_STREAM_UNPAIRED           1
 #define NR_ICE_MEDIA_STREAM_CHECKS_FROZEN      2
@@ -78,7 +82,7 @@ struct nr_ice_media_stream_ {
 
 typedef STAILQ_HEAD(nr_ice_media_stream_head_,nr_ice_media_stream_) nr_ice_media_stream_head;
 
-int nr_ice_media_stream_create(struct nr_ice_ctx_ *ctx,char *label, int components, nr_ice_media_stream **streamp);
+int nr_ice_media_stream_create(struct nr_ice_ctx_ *ctx,const char *label,const char *ufrag,const char *pwd,int components, nr_ice_media_stream **streamp);
 int nr_ice_media_stream_destroy(nr_ice_media_stream **streamp);
 int nr_ice_media_stream_finalize(nr_ice_media_stream *lstr,nr_ice_media_stream *rstr);
 int nr_ice_media_stream_initialize(struct nr_ice_ctx_ *ctx, nr_ice_media_stream *stream);
@@ -90,13 +94,15 @@ int nr_ice_media_stream_service_pre_answer_requests(nr_ice_peer_ctx *pctx,nr_ice
 int nr_ice_media_stream_unfreeze_pairs(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream);
 int nr_ice_media_stream_unfreeze_pairs_foundation(nr_ice_media_stream *stream, char *foundation);
 int nr_ice_media_stream_dump_state(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream,FILE *out);
-int nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_component *component);
-int nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_component *component);
+void nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_component *component);
+void nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_component *component);
 void nr_ice_media_stream_refresh_consent_all(nr_ice_media_stream *stream);
 void nr_ice_media_stream_disconnect_all_components(nr_ice_media_stream *stream);
 void nr_ice_media_stream_set_disconnected(nr_ice_media_stream *stream, int disconnected);
 int nr_ice_media_stream_check_if_connected(nr_ice_media_stream *stream);
 int nr_ice_media_stream_set_state(nr_ice_media_stream *str, int state);
+void nr_ice_media_stream_stop_checking(nr_ice_media_stream *str);
+void nr_ice_media_stream_set_obsolete(nr_ice_media_stream *str);
 int nr_ice_media_stream_get_best_candidate(nr_ice_media_stream *str, int component, nr_ice_candidate **candp);
 int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, UCHAR *data, int len);
 int nr_ice_media_stream_get_active(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_ice_candidate **local, nr_ice_candidate **remote);

media/mtransport/third_party/nICEr/src/ice/ice_parser.c

@@ -380,6 +380,7 @@ nr_ice_peer_ctx_parse_media_stream_attribute(nr_ice_peer_ctx *pctx, nr_ice_media
       if (*str == '\0')
         ABORT(R_BAD_DATA);
 
+      RFREE(stream->ufrag);
       if ((r=grab_token(&str, &stream->ufrag)))
         ABORT(r);
     }
@@ -392,6 +393,7 @@ nr_ice_peer_ctx_parse_media_stream_attribute(nr_ice_peer_ctx *pctx, nr_ice_media
       if (*str == '\0')
         ABORT(R_BAD_DATA);
 
+      RFREE(stream->pwd);
       if ((r=grab_token(&str, &stream->pwd)))
         ABORT(r);
     }
@@ -490,6 +492,7 @@ nr_ice_peer_ctx_parse_global_attributes(nr_ice_peer_ctx *pctx, char **attrs, int
         }
         else if (!strncasecmp(str, "ice-lite", 8)) {
             pctx->peer_lite = 1;
+            pctx->controlling = 0;
 
             fast_forward(&str, 8);
         }
@@ -506,11 +509,6 @@ nr_ice_peer_ctx_parse_global_attributes(nr_ice_peer_ctx *pctx, char **attrs, int
             skip_whitespace(&str);
             if (*str == '\0')
                 ABORT(R_BAD_DATA);
-
-            RFREE(pctx->peer_ufrag);
-            pctx->peer_ufrag = 0;
-            if ((r=grab_token(&str, &pctx->peer_ufrag)))
-                ABORT(r);
         }
         else if (!strncasecmp(str, "ice-pwd:", 8)) {
             fast_forward(&str, 8);
@@ -520,11 +518,6 @@ nr_ice_peer_ctx_parse_global_attributes(nr_ice_peer_ctx *pctx, char **attrs, int
             skip_whitespace(&str);
             if (*str == '\0')
                 ABORT(R_BAD_DATA);
-
-            RFREE(pctx->peer_pwd);
-            pctx->peer_pwd = 0;
-            if ((r=grab_token(&str, &pctx->peer_pwd)))
-                ABORT(r);
         }
         else if (!strncasecmp(str, "ice-options:", 12)) {
             fast_forward(&str, 12);

media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c

@@ -99,7 +99,7 @@ int nr_ice_peer_ctx_parse_stream_attributes(nr_ice_peer_ctx *pctx, nr_ice_media_
     /*
       Note: use component_ct from our own stream since components other
       than this offered by the other side are unusable */
-    if(r=nr_ice_media_stream_create(pctx->ctx,stream->label,stream->component_ct,&pstream))
+    if(r=nr_ice_media_stream_create(pctx->ctx,stream->label,"","",stream->component_ct,&pstream))
       ABORT(r);
 
     /* Match up the local and remote components */
@@ -120,12 +120,12 @@ int nr_ice_peer_ctx_parse_stream_attributes(nr_ice_peer_ctx *pctx, nr_ice_media_
 
     /* Now that we have the ufrag and password, compute all the username/password
        pairs */
-    lufrag=stream->ufrag?stream->ufrag:pctx->ctx->ufrag;
-    lpwd=stream->pwd?stream->pwd:pctx->ctx->pwd;
+    lufrag=stream->ufrag;
+    lpwd=stream->pwd;
     assert(lufrag);
     assert(lpwd);
-    rufrag=pstream->ufrag?pstream->ufrag:pctx->peer_ufrag;
-    rpwd=pstream->pwd?pstream->pwd:pctx->peer_pwd;
+    rufrag=pstream->ufrag;
+    rpwd=pstream->pwd;
     if (!rufrag || !rpwd)
       ABORT(R_BAD_DATA);
 
@@ -470,8 +470,6 @@ static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg)
 
     NR_async_timer_cancel(pctx->connected_cb_timer);
     RFREE(pctx->label);
-    RFREE(pctx->peer_ufrag);
-    RFREE(pctx->peer_pwd);
 
     STAILQ_FOREACH_SAFE(str1, &pctx->peer_streams, entry, str2){
       STAILQ_REMOVE(&pctx->peer_streams,str1,nr_ice_media_stream_,entry);
@@ -534,10 +532,7 @@ int nr_ice_peer_ctx_start_checks2(nr_ice_peer_ctx *pctx, int allow_non_first)
     pctx->connected_cb_timer = 0;
     pctx->checks_started = 0;
 
-    if((r=nr_ice_peer_ctx_check_if_connected(pctx))) {
-      r_log(LOG_ICE,LOG_ERR,"ICE(%s): peer (%s) initial connected check failed",pctx->ctx->label,pctx->label);
-      ABORT(r);
-    }
+    nr_ice_peer_ctx_check_if_connected(pctx);
 
     if (pctx->reported_connected) {
       r_log(LOG_ICE,LOG_ERR,"ICE(%s): peer (%s) in %s all streams were done",pctx->ctx->label,pctx->label,__FUNCTION__);
@@ -717,29 +712,30 @@ static void nr_ice_peer_ctx_fire_connected(NR_SOCKET s, int how, void *cb_arg)
 
 /* Examine all the streams to see if we're
    maybe miraculously connected */
-int nr_ice_peer_ctx_check_if_connected(nr_ice_peer_ctx *pctx)
+void nr_ice_peer_ctx_check_if_connected(nr_ice_peer_ctx *pctx)
   {
-    int _status;
     nr_ice_media_stream *str;
     int failed=0;
     int succeeded=0;
 
     str=STAILQ_FIRST(&pctx->peer_streams);
     while(str){
-      if(str->ice_state==NR_ICE_MEDIA_STREAM_CHECKS_CONNECTED){
-        succeeded++;
-      }
-      else if(str->ice_state==NR_ICE_MEDIA_STREAM_CHECKS_FAILED){
-        failed++;
-      }
-      else{
-        break;
+      if (!str->local_stream->obsolete){
+        if(str->ice_state==NR_ICE_MEDIA_STREAM_CHECKS_CONNECTED){
+          succeeded++;
+        }
+        else if(str->ice_state==NR_ICE_MEDIA_STREAM_CHECKS_FAILED){
+          failed++;
+        }
+        else{
+          break;
+        }
       }
       str=STAILQ_NEXT(str,entry);
     }
 
     if(str)
-      goto done;  /* Something isn't done */
+      return;  /* Something isn't done */
 
     /* OK, we're finished, one way or another */
     r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s): all checks completed success=%d fail=%d",pctx->label,succeeded,failed);
@@ -752,10 +748,6 @@ int nr_ice_peer_ctx_check_if_connected(nr_ice_peer_ctx *pctx)
       assert(!pctx->connected_cb_timer);
       NR_ASYNC_TIMER_SET(0,nr_ice_peer_ctx_fire_connected,pctx,&pctx->connected_cb_timer);
     }
-
-  done:
-    _status=0;
-    return(_status);
   }
 
 

media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h

@@ -52,8 +52,6 @@ struct nr_ice_peer_ctx_ {
   UCHAR controlling_conflict_resolved;
   UINT8 tiebreaker;
 
-  char *peer_ufrag;
-  char *peer_pwd;
   int peer_lite;
   int peer_ice_mismatch;
 
@@ -89,7 +87,7 @@ void nr_ice_peer_ctx_disconnected(nr_ice_peer_ctx *pctx);
 void nr_ice_peer_ctx_connected(nr_ice_peer_ctx *pctx);
 int nr_ice_peer_ctx_dump_state(nr_ice_peer_ctx *pctx,FILE *out);
 int nr_ice_peer_ctx_log_state(nr_ice_peer_ctx *pctx);
-int nr_ice_peer_ctx_check_if_connected(nr_ice_peer_ctx *pctx);
+void nr_ice_peer_ctx_check_if_connected(nr_ice_peer_ctx *pctx);
 int nr_ice_peer_ctx_find_component(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component_id, nr_ice_component **compp);
 int nr_ice_peer_ctx_deliver_packet_maybe(nr_ice_peer_ctx *pctx, nr_ice_component *comp, nr_transport_addr *source_addr, UCHAR *data, int len);
 int nr_ice_peer_ctx_disable_component(nr_ice_peer_ctx *pctx, nr_ice_media_stream *lstream, int component_id);

media/mtransport/transportlayerice.cpp

@@ -85,8 +85,7 @@ namespace mozilla {
 MOZ_MTLOG_MODULE("mtransport")
 
 TransportLayerIce::TransportLayerIce()
-    : stream_(nullptr), component_(0),
-      old_stream_(nullptr)
+    : stream_(nullptr), component_(0)
 {
   // setup happens later
 }
@@ -108,17 +107,6 @@ void TransportLayerIce::SetParameters(RefPtr<NrIceMediaStream> stream,
     return;
   }
 
-  // If SetParameters is called and we already have a stream_, this means
-  // we're handling an ICE restart.  We need to hold the old stream until
-  // we know the new stream is working.
-  if (stream_ && !old_stream_ && (stream_ != stream)) {
-    // Here we leave the old stream's signals connected until we don't need
-    // it anymore.  They will be disconnected if ice restart is successful.
-    old_stream_ = stream_;
-    MOZ_MTLOG(ML_INFO, LAYER_INFO << "SetParameters save old stream("
-                                  << old_stream_->name() << ")");
-  }
-
   stream_ = stream;
   component_ = component;
 
@@ -135,48 +123,11 @@ void TransportLayerIce::PostSetup() {
   }
 }
 
-void TransportLayerIce::ResetOldStream() {
-  if (old_stream_ == nullptr) {
-    return; // no work to do
-  }
-  // ICE restart successful on the new stream, we can forget the old stream now
-  MOZ_MTLOG(ML_INFO, LAYER_INFO << "ResetOldStream(" << old_stream_->name()
-                                << ")");
-  old_stream_->SignalReady.disconnect(this);
-  old_stream_->SignalFailed.disconnect(this);
-  old_stream_->SignalPacketReceived.disconnect(this);
-  old_stream_ = nullptr;
-}
-
-void TransportLayerIce::RestoreOldStream() {
-  if (old_stream_ == nullptr) {
-    return; // no work to do
-  }
-  // ICE restart rollback, we need to restore the old stream
-  MOZ_MTLOG(ML_INFO, LAYER_INFO << "RestoreOldStream(" << old_stream_->name()
-                                << ")");
-  stream_->SignalReady.disconnect(this);
-  stream_->SignalFailed.disconnect(this);
-  stream_->SignalPacketReceived.disconnect(this);
-  stream_ = old_stream_;
-  old_stream_ = nullptr;
-
-  if (stream_->state() == NrIceMediaStream::ICE_OPEN) {
-    IceReady(stream_);
-  } else if (stream_->state() == NrIceMediaStream::ICE_CLOSED) {
-    IceFailed(stream_);
-  }
-  // No events are fired when the stream is ICE_CONNECTING.  If the
-  // restored stream is ICE_CONNECTING, IceReady/IceFailed will fire
-  // later.
-}
-
 TransportResult TransportLayerIce::SendPacket(MediaPacket& packet) {
   CheckThread();
-  // use old_stream_ until stream_ is ready
-  nsresult res = (old_stream_?old_stream_:stream_)->SendPacket(component_,
-                                                               packet.data(),
-                                                               packet.len());
+  nsresult res = stream_->SendPacket(component_,
+                                     packet.data(),
+                                     packet.len());
 
   if (!NS_SUCCEEDED(res)) {
     return (res == NS_BASE_STREAM_WOULD_BLOCK) ?

media/mtransport/transportlayerice.h

@@ -58,9 +58,6 @@ class TransportLayerIce : public TransportLayer {
 
   RefPtr<NrIceMediaStream> stream_;
   int component_;
-
-  // used to hold the old stream
-  RefPtr<NrIceMediaStream> old_stream_;
 };
 
 }  // close namespace