Bug 1561150: Follow-up: Fix buggy chrome scripts in form autofill tests. r=me,test-only
☠☠ backed out by f9bf5e4b0b4f ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Thu, 27 Jun 2019 15:44:55 -0700
changeset 540552 19e0edc9207746c7987b98f3c7787b7c363d651d
parent 540551 0b3e2164f1283b639782b41b7fd640986d3feca5
child 540553 c35aff2a933694722bc4ef64a3b53e787c662d6a
push id11529
push userarchaeopteryx@coole-files.de
push dateThu, 04 Jul 2019 15:22:33 +0000
treeherdermozilla-beta@ebb510a784b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme, test-only
bugs1561150
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1561150: Follow-up: Fix buggy chrome scripts in form autofill tests. r=me,test-only These tests mostly worked by accident in the past because assertions from chrome scripts were not processed after a chrome script was destroyed, and assertions were not treated as assertions anyway. But once those issues were fixed, they started failing because of races in the RPC code in these tests, which caused the first resolved call of a given method to resolve all other calls as well, and not wait for them to finish and their assertions to fire before ending the test.
browser/extensions/formautofill/test/mochitest/formautofill_common.js
browser/extensions/formautofill/test/mochitest/formautofill_parent_utils.js
--- a/browser/extensions/formautofill/test/mochitest/formautofill_common.js
+++ b/browser/extensions/formautofill/test/mochitest/formautofill_common.js
@@ -171,76 +171,69 @@ function checkMenuEntries(expectedValues
   is(actualValues.length, expectedLength, " Checking length of expected menu");
   for (let i = 0; i < expectedValues.length; i++) {
     is(actualValues[i], expectedValues[i], " Checking menu entry #" + i);
   }
 }
 
 function invokeAsyncChromeTask(message, response, payload = {}) {
   info(`expecting the chrome task finished: ${message}`);
-  return new Promise(resolve => {
-    formFillChromeScript.sendAsyncMessage(message, payload);
-    formFillChromeScript.addMessageListener(response, function onReceived(data) {
-      formFillChromeScript.removeMessageListener(response, onReceived);
-
-      resolve(data);
-    });
-  });
+  return formFillChromeScript.sendQuery(message, payload);
 }
 
 async function addAddress(address) {
-  await invokeAsyncChromeTask("FormAutofillTest:AddAddress", "FormAutofillTest:AddressAdded", {address});
+  await invokeAsyncChromeTask("FormAutofillTest:AddAddress", {address});
   await sleep();
 }
 
 async function removeAddress(guid) {
-  return invokeAsyncChromeTask("FormAutofillTest:RemoveAddress", "FormAutofillTest:AddressRemoved", {guid});
+  return invokeAsyncChromeTask("FormAutofillTest:RemoveAddress", {guid});
 }
 
 async function updateAddress(guid, address) {
-  return invokeAsyncChromeTask("FormAutofillTest:UpdateAddress", "FormAutofillTest:AddressUpdated", {address, guid});
+  return invokeAsyncChromeTask("FormAutofillTest:UpdateAddress", {address, guid});
 }
 
 async function checkAddresses(expectedAddresses) {
-  return invokeAsyncChromeTask("FormAutofillTest:CheckAddresses", "FormAutofillTest:areAddressesMatching", {expectedAddresses});
+  return invokeAsyncChromeTask("FormAutofillTest:CheckAddresses", {expectedAddresses});
 }
 
 async function cleanUpAddresses() {
-  return invokeAsyncChromeTask("FormAutofillTest:CleanUpAddresses", "FormAutofillTest:AddressesCleanedUp");
+  return invokeAsyncChromeTask("FormAutofillTest:CleanUpAddresses");
 }
 
 async function addCreditCard(creditcard) {
-  await invokeAsyncChromeTask("FormAutofillTest:AddCreditCard", "FormAutofillTest:CreditCardAdded", {creditcard});
+  await invokeAsyncChromeTask("FormAutofillTest:AddCreditCard", {creditcard});
   await sleep();
 }
 
 async function removeCreditCard(guid) {
-  return invokeAsyncChromeTask("FormAutofillTest:RemoveCreditCard", "FormAutofillTest:CreditCardRemoved", {guid});
+  return invokeAsyncChromeTask("FormAutofillTest:RemoveCreditCard", {guid});
 }
 
 async function checkCreditCards(expectedCreditCards) {
-  return invokeAsyncChromeTask("FormAutofillTest:CheckCreditCards", "FormAutofillTest:areCreditCardsMatching", {expectedCreditCards});
+  return invokeAsyncChromeTask("FormAutofillTest:CheckCreditCards", {expectedCreditCards});
 }
 
 async function cleanUpCreditCards() {
-  return invokeAsyncChromeTask("FormAutofillTest:CleanUpCreditCards", "FormAutofillTest:CreditCardsCleanedUp");
+  return invokeAsyncChromeTask("FormAutofillTest:CleanUpCreditCards");
 }
 
 async function cleanUpStorage() {
   await cleanUpAddresses();
   await cleanUpCreditCards();
 }
 
 async function canTestOSKeyStoreLogin() {
-  let {canTest} = await invokeAsyncChromeTask("FormAutofillTest:CanTestOSKeyStoreLogin", "FormAutofillTest:CanTestOSKeyStoreLoginResult");
+  let {canTest} = await invokeAsyncChromeTask("FormAutofillTest:CanTestOSKeyStoreLogin");
   return canTest;
 }
 
 async function waitForOSKeyStoreLogin(login = false) {
-  await invokeAsyncChromeTask("FormAutofillTest:OSKeyStoreLogin", "FormAutofillTest:OSKeyStoreLoggedIn", {login});
+  await invokeAsyncChromeTask("FormAutofillTest:OSKeyStoreLogin", {login});
 }
 
 function patchRecordCCNumber(record) {
   const number = record["cc-number"];
   const ccNumberFmt = {
     affix: "****",
     label: number.substr(-4),
   };
@@ -297,25 +290,23 @@ function formAutoFillCommonSetup() {
   formFillChromeScript.addMessageListener("onpopupshown", ({results}) => {
     gLastAutoCompleteResults = results;
     if (gPopupShownListener) {
       gPopupShownListener({results});
     }
   });
 
   add_task(async function setup() {
-    formFillChromeScript.sendAsyncMessage("setup");
     info(`expecting the storage setup`);
-    await formFillChromeScript.promiseOneMessage("setup-finished");
+    await formFillChromeScript.sendQuery("setup");
   });
 
   SimpleTest.registerCleanupFunction(async () => {
-    formFillChromeScript.sendAsyncMessage("cleanup");
     info(`expecting the storage cleanup`);
-    await formFillChromeScript.promiseOneMessage("cleanup-finished");
+    await formFillChromeScript.sendQuery("cleanup");
 
     formFillChromeScript.destroy();
     expectingPopup = null;
   });
 
   document.addEventListener("DOMContentLoaded", function() {
     defaultTextColor = window.getComputedStyle(document.querySelector("input"))
       .getPropertyValue("color");
--- a/browser/extensions/formautofill/test/mochitest/formautofill_parent_utils.js
+++ b/browser/extensions/formautofill/test/mochitest/formautofill_parent_utils.js
@@ -42,17 +42,17 @@ var ParentUtils = {
           assert.ok(subject.wrappedJSObject.guid, "should have a guid");
         }
         Services.obs.removeObserver(observer, obsTopic);
         resolve();
       }, topic);
     });
   },
 
-  async _operateRecord(collectionName, type, msgData, contentMsg) {
+  async _operateRecord(collectionName, type, msgData) {
     let times, topic;
 
     if (collectionName == ADDRESSES_COLLECTION_NAME) {
       switch (type) {
         case "add": {
           Services.cpmm.sendAsyncMessage("FormAutofill:SaveAddress", msgData);
           break;
         }
@@ -78,46 +78,43 @@ var ParentUtils = {
           times = msgData.guids.length;
           Services.cpmm.sendAsyncMessage("FormAutofill:RemoveCreditCards", msgData);
           break;
         }
       }
     }
 
     await this._storageChangeObserved({type, times, topic});
-    sendAsyncMessage(contentMsg);
   },
 
-  async operateAddress(type, msgData, contentMsg) {
+  async operateAddress(type, msgData) {
     await this._operateRecord(ADDRESSES_COLLECTION_NAME, ...arguments);
   },
 
-  async operateCreditCard(type, msgData, contentMsg) {
+  async operateCreditCard(type, msgData) {
     await this._operateRecord(CREDITCARDS_COLLECTION_NAME, ...arguments);
   },
 
   async cleanUpAddresses() {
     const guids = (await this._getRecords(ADDRESSES_COLLECTION_NAME)).map(record => record.guid);
 
     if (guids.length == 0) {
-      sendAsyncMessage("FormAutofillTest:AddressesCleanedUp");
       return;
     }
 
     await this.operateAddress("remove", {guids}, "FormAutofillTest:AddressesCleanedUp");
   },
 
   async cleanUpCreditCards() {
     if (!FormAutofill.isAutofillCreditCardsAvailable) {
       return;
     }
     const guids = (await this._getRecords(CREDITCARDS_COLLECTION_NAME)).map(record => record.guid);
 
     if (guids.length == 0) {
-      sendAsyncMessage("FormAutofillTest:CreditCardsCleanedUp");
       return;
     }
 
     await this.operateCreditCard("remove", {guids}, "FormAutofillTest:CreditCardsCleanedUp");
   },
 
   setup() {
     OSKeyStoreTestUtils.setup();
@@ -162,87 +159,78 @@ var ParentUtils = {
         return false;
       }
     }
 
     return true;
   },
 
   async checkAddresses({expectedAddresses}) {
-    const areMatched = await this._checkRecords(ADDRESSES_COLLECTION_NAME, expectedAddresses);
-
-    sendAsyncMessage("FormAutofillTest:areAddressesMatching", areMatched);
+    return this._checkRecords(ADDRESSES_COLLECTION_NAME, expectedAddresses);
   },
 
   async checkCreditCards({expectedCreditCards}) {
-    const areMatched = await this._checkRecords(CREDITCARDS_COLLECTION_NAME, expectedCreditCards);
-
-    sendAsyncMessage("FormAutofillTest:areCreditCardsMatching", areMatched);
+    return this._checkRecords(CREDITCARDS_COLLECTION_NAME, expectedCreditCards);
   },
 
   observe(subject, topic, data) {
     assert.ok(topic === "formautofill-storage-changed");
     sendAsyncMessage("formautofill-storage-changed", {subject: null, topic, data});
   },
 };
 
 Services.obs.addObserver(ParentUtils, "formautofill-storage-changed");
 
 Services.mm.addMessageListener("FormAutofill:FieldsIdentified", () => {
-  sendAsyncMessage("FormAutofillTest:FieldsIdentified");
+  return null;
 });
 
 addMessageListener("FormAutofillTest:AddAddress", (msg) => {
-  ParentUtils.operateAddress("add", msg, "FormAutofillTest:AddressAdded");
+  return ParentUtils.operateAddress("add", msg);
 });
 
 addMessageListener("FormAutofillTest:RemoveAddress", (msg) => {
-  ParentUtils.operateAddress("remove", msg, "FormAutofillTest:AddressRemoved");
+  return ParentUtils.operateAddress("remove", msg);
 });
 
 addMessageListener("FormAutofillTest:UpdateAddress", (msg) => {
-  ParentUtils.operateAddress("update", msg, "FormAutofillTest:AddressUpdated");
+  return ParentUtils.operateAddress("update", msg);
 });
 
 addMessageListener("FormAutofillTest:CheckAddresses", (msg) => {
-  ParentUtils.checkAddresses(msg);
+  return ParentUtils.checkAddresses(msg);
 });
 
 addMessageListener("FormAutofillTest:CleanUpAddresses", (msg) => {
-  ParentUtils.cleanUpAddresses();
+  return ParentUtils.cleanUpAddresses();
 });
 
 addMessageListener("FormAutofillTest:AddCreditCard", (msg) => {
-  ParentUtils.operateCreditCard("add", msg, "FormAutofillTest:CreditCardAdded");
+  return ParentUtils.operateCreditCard("add", msg);
 });
 
 addMessageListener("FormAutofillTest:RemoveCreditCard", (msg) => {
-  ParentUtils.operateCreditCard("remove", msg, "FormAutofillTest:CreditCardRemoved");
+  return ParentUtils.operateCreditCard("remove", msg);
 });
 
 addMessageListener("FormAutofillTest:CheckCreditCards", (msg) => {
-  ParentUtils.checkCreditCards(msg);
+  return ParentUtils.checkCreditCards(msg);
 });
 
 addMessageListener("FormAutofillTest:CleanUpCreditCards", (msg) => {
-  ParentUtils.cleanUpCreditCards();
+  return ParentUtils.cleanUpCreditCards();
 });
 
 addMessageListener("FormAutofillTest:CanTestOSKeyStoreLogin", (msg) => {
-  sendAsyncMessage("FormAutofillTest:CanTestOSKeyStoreLoginResult",
-    {canTest: OSKeyStoreTestUtils.canTestOSKeyStoreLogin()});
+  return {canTest: OSKeyStoreTestUtils.canTestOSKeyStoreLogin()};
 });
 
 addMessageListener("FormAutofillTest:OSKeyStoreLogin", async (msg) => {
   await OSKeyStoreTestUtils.waitForOSKeyStoreLogin(msg.login);
-  sendAsyncMessage("FormAutofillTest:OSKeyStoreLoggedIn");
 });
 
 addMessageListener("setup", async () => {
   ParentUtils.setup();
-  sendAsyncMessage("setup-finished", {});
 });
 
 addMessageListener("cleanup", async () => {
   await ParentUtils.cleanup();
-
-  sendAsyncMessage("cleanup-finished", {});
 });