Bug 1348757 - Apply weakref for the element in cache. r?MattN draft
authorsteveck-chung <schung@mozilla.com>
Wed, 08 Mar 2017 14:38:08 +0800
changeset 552235 2adb9c7c7a9b95aa9494a071aa1fe827038d8f1b
parent 551502 cc53710589fb500610495da5258b7b9221edf681
child 554277 19bd2d4dcbf59769e775b5ac5a085e57fd4bcaa6
child 561967 e800774cff3ae1f2e0b9e6e828452d474f52c9b1
child 562515 1c4d3334f1443180ecfe5bde433470ae18f18b3f
push id51293
push userbmo:schung@mozilla.com
push dateTue, 28 Mar 2017 06:10:14 +0000
reviewersMattN
bugs1348757
milestone55.0a1
Bug 1348757 - Apply weakref for the element in cache. r?MattN MozReview-Commit-ID: Ezo5kLzf4UK
browser/extensions/formautofill/FormAutofillContent.jsm
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/test/unit/test_autofillFormFields.js
browser/extensions/formautofill/test/unit/test_collectFormFields.js
browser/extensions/formautofill/test/unit/test_getFormInputDetails.js
--- a/browser/extensions/formautofill/FormAutofillContent.jsm
+++ b/browser/extensions/formautofill/FormAutofillContent.jsm
@@ -298,17 +298,18 @@ var FormAutofillContent = {
    * @param {HTMLInputElement} element Focused input which triggered profile searching
    * @returns {Object|null}
    *          Return target input's information that cloned from content cache
    *          (or return null if the information is not found in the cache).
    */
   getInputDetails(element) {
     let formDetails = this.getFormDetails(element);
     for (let detail of formDetails) {
-      if (element == detail.element) {
+      let detailElement = detail.elementWeakRef.get();
+      if (detailElement && element == detailElement) {
         return detail;
       }
     }
     return null;
   },
 
   /**
    * Get the form's handler from cache which is created after page identified.
@@ -371,19 +372,25 @@ var FormAutofillContent = {
       if (formHandler.fieldDetails.length < AUTOFILL_FIELDS_THRESHOLD) {
         this.log.debug("Ignoring form since it has only", formHandler.fieldDetails.length,
                        "field(s)");
         return;
       }
 
       this._formsDetails.set(form.rootElement, formHandler);
       this.log.debug("Adding form handler to _formsDetails:", formHandler);
-      formHandler.fieldDetails.forEach(detail => this._markAsAutofillField(detail.element));
+      formHandler.fieldDetails.forEach(detail =>
+        this._markAsAutofillField(detail.elementWeakRef.get())
+      );
     });
   },
 
   _markAsAutofillField(field) {
+    if (!field) {
+      return;
+    }
+
     formFillController.markAsAutofillField(field);
   },
 };
 
 
 FormAutofillContent.init();
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -80,17 +80,17 @@ FormAutofillHandler.prototype = {
         continue;
       }
 
       let formatWithElement = {
         section: info.section,
         addressType: info.addressType,
         contactType: info.contactType,
         fieldName: info.fieldName,
-        element, // TODO: Apply Cu.getWeakReference and use get API for strong ref.
+        elementWeakRef: Cu.getWeakReference(element),
       };
 
       this.fieldDetails.push(formatWithElement);
     }
 
     log.debug("Collected details on", this.fieldDetails.length, "fields");
   },
 
@@ -108,20 +108,20 @@ FormAutofillHandler.prototype = {
 
     this.filledProfileGUID = profile.guid;
     for (let fieldDetail of this.fieldDetails) {
       // Avoid filling field value in the following cases:
       // 1. the focused input which is filled in FormFillController.
       // 2. a non-empty input field
       // 3. the invalid value set
 
-      if (fieldDetail.element === focusedInput ||
-          fieldDetail.element.value) {
+      let element = fieldDetail.elementWeakRef.get();
+      if (!element || element === focusedInput || element.value) {
         continue;
       }
 
       let value = profile[fieldDetail.fieldName];
       if (value) {
-        fieldDetail.element.setUserInput(value);
+        element.setUserInput(value);
       }
     }
   },
 };
--- a/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
@@ -136,16 +136,17 @@ const TESTCASES = [
     fieldDetails: [
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name", "element": {}},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name", "element": {}},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address", "element": {}},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2", "element": {}},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country", "element": {}},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email", "element": {}},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel", "element": {}},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "organization", "element": null},
     ],
     profileData: {
       "guid": "123",
       "street-address": "",
       "address-level2": "",
       "country": "",
       "email": "foo@mozilla.com",
       "tel": "1234567",
@@ -170,17 +171,17 @@ for (let tc of TESTCASES) {
                                                 testcase.document);
       let form = doc.querySelector("form");
       let handler = new FormAutofillHandler(form);
       let onChangePromises = [];
 
       handler.fieldDetails = testcase.fieldDetails;
       handler.fieldDetails.forEach((field, index) => {
         let element = doc.querySelectorAll("input")[index];
-        field.element = element;
+        field.elementWeakRef = Cu.getWeakReference(element);
         if (!testcase.profileData[field.fieldName]) {
           // Avoid waiting for `change` event of a input with a blank value to
           // be filled.
           return;
         }
         onChangePromises.push(new Promise(resolve => {
           element.addEventListener("change", () => {
             let id = element.id;
--- a/browser/extensions/formautofill/test/unit/test_collectFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_collectFormFields.js
@@ -20,69 +20,79 @@ const TESTCASES = [
     document: `<form><input id="given-name" autocomplete="given-name">
                <input id="family-name" autocomplete="family-name">
                <input id="street-addr" autocomplete="street-address">
                <input id="city" autocomplete="address-level2">
                <input id="country" autocomplete="country">
                <input id="email" autocomplete="email">
                <input id="tel" autocomplete="tel"></form>`,
     fieldDetails: [
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "street-address", "element": {}},
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "address-level2", "element": {}},
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "country", "element": {}},
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "email", "element": {}},
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel", "element": {}},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
     ],
   },
   {
     description: "Form with autocomplete properties and 2 tokens",
     document: `<form><input id="given-name" autocomplete="shipping given-name">
                <input id="family-name" autocomplete="shipping family-name">
                <input id="street-addr" autocomplete="shipping street-address">
                <input id="city" autocomplete="shipping address-level2">
                <input id="country" autocomplete="shipping country">
                <input id='email' autocomplete="shipping email">
                <input id="tel" autocomplete="shipping tel"></form>`,
     fieldDetails: [
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel", "element": {}},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
     ],
   },
   {
     description: "Form with autocomplete properties and profile is partly matched",
     document: `<form><input id="given-name" autocomplete="shipping given-name">
                <input id="family-name" autocomplete="shipping family-name">
                <input id="street-addr" autocomplete="shipping street-address">
                <input id="city" autocomplete="shipping address-level2">
                <input id="country" autocomplete="shipping country">
                <input id='email' autocomplete="shipping email">
                <input id="tel" autocomplete="shipping tel"></form>`,
     fieldDetails: [
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email", "element": {}},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel", "element": {}},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
     ],
   },
 ];
 
 for (let tc of TESTCASES) {
   (function() {
     let testcase = tc;
     add_task(function* () {
       do_print("Starting testcase: " + testcase.description);
 
       let doc = MockDocument.createTestDocument("http://localhost:8080/test/",
                                                 testcase.document);
       let form = doc.querySelector("form");
+
+      testcase.fieldDetails.forEach((detail) => {
+        detail.elementWeakRef = Cu.getWeakReference(doc.querySelector(
+          "input[autocomplete*='" + detail.fieldName + "']"));
+      });
       let handler = new FormAutofillHandler(form);
 
       handler.collectFormFields();
 
-      Assert.deepEqual(handler.fieldDetails, testcase.fieldDetails,
-                         "Check the fieldDetails were set correctly");
+      handler.fieldDetails.forEach((detail, index) => {
+        Assert.equal(detail.section, testcase.fieldDetails[index].section);
+        Assert.equal(detail.addressType, testcase.fieldDetails[index].addressType);
+        Assert.equal(detail.contactType, testcase.fieldDetails[index].contactType);
+        Assert.equal(detail.fieldName, testcase.fieldDetails[index].fieldName);
+        Assert.equal(detail.elementWeakRef.get(), testcase.fieldDetails[index].elementWeakRef.get());
+      });
     });
   })();
 }
--- a/browser/extensions/formautofill/test/unit/test_getFormInputDetails.js
+++ b/browser/extensions/formautofill/test/unit/test_getFormInputDetails.js
@@ -65,40 +65,48 @@ const TESTCASES = [
         {"section": "", "addressType": "", "contactType": "", "fieldName": "street-address"},
         {"section": "", "addressType": "", "contactType": "", "fieldName": "email"},
         {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
       ],
     }],
   },
 ];
 
+function inputDetailAssertion(detail, expected) {
+  Assert.equal(detail.section, expected.section);
+  Assert.equal(detail.addressType, expected.addressType);
+  Assert.equal(detail.contactType, expected.contactType);
+  Assert.equal(detail.fieldName, expected.fieldName);
+  Assert.equal(detail.elementWeakRef.get(), expected.elementWeakRef.get());
+}
 
 TESTCASES.forEach(testcase => {
   add_task(function* () {
     do_print("Starting testcase: " + testcase.description);
 
     let doc = MockDocument.createTestDocument(
               "http://localhost:8080/test/", testcase.document);
     FormAutofillContent.identifyAutofillFields(doc);
 
     for (let i in testcase.targetInput) {
       let input = doc.getElementById(testcase.targetInput[i]);
 
       // Put the input element reference to `element` to make sure the result of
       // `getInputDetails` contains the same input element.
-      testcase.expectedResult[i].input.element = input;
-      Assert.deepEqual(FormAutofillContent.getInputDetails(input),
-                       testcase.expectedResult[i].input,
-                       "Check if returned input information is correct.");
+      testcase.expectedResult[i].input.elementWeakRef = Cu.getWeakReference(input);
+
+      inputDetailAssertion(FormAutofillContent.getInputDetails(input),
+                           testcase.expectedResult[i].input);
 
       let formDetails = testcase.expectedResult[i].form;
       for (let formDetail of formDetails) {
         // Compose a query string to get the exact reference of the input
         // element, e.g. #form1 > input[autocomplete="street-address"]
         let queryString = "#" + testcase.expectedResult[i].formId + " > input[autocomplete=" + formDetail.fieldName + "]";
-        formDetail.element = doc.querySelector(queryString);
+        formDetail.elementWeakRef = Cu.getWeakReference(doc.querySelector(queryString));
       }
-      Assert.deepEqual(FormAutofillContent.getFormDetails(input),
-                       formDetails,
-                       "Check if returned form information is correct.");
+
+      FormAutofillContent.getFormDetails(input).forEach((detail, index) => {
+        inputDetailAssertion(detail, formDetails[index]);
+      });
     }
   });
 });