Bug 1375568 - [Form Autofill] Reduce cross-JSM function calls between FormAutofillUtils and FormAutofillHeuristics. r=MattN draft
authorLuke Chang <lchang@mozilla.com>
Fri, 21 Jul 2017 14:50:20 +0800
changeset 614130 f09b5a0819d2ba7e09e99a08528e5cd64226c9e1
parent 614015 5928d905c0bc0b28f5488b236444c7d7991cf8d4
child 614131 6e1a06e8640f2507b2124dfaeb93b13169ad3af1
push id69932
push userbmo:lchang@mozilla.com
push dateMon, 24 Jul 2017 07:36:51 +0000
reviewersMattN
bugs1375568
milestone56.0a1
Bug 1375568 - [Form Autofill] Reduce cross-JSM function calls between FormAutofillUtils and FormAutofillHeuristics. r=MattN MozReview-Commit-ID: D2gEBjKYXT3
browser/extensions/formautofill/FormAutofillHeuristics.jsm
browser/extensions/formautofill/FormAutofillUtils.jsm
browser/extensions/formautofill/test/unit/test_extractLabelStrings.js
browser/extensions/formautofill/test/unit/test_findLabelElements.js
--- a/browser/extensions/formautofill/FormAutofillHeuristics.jsm
+++ b/browser/extensions/formautofill/FormAutofillHeuristics.jsm
@@ -16,16 +16,18 @@ Cu.import("resource://gre/modules/Servic
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://formautofill/FormAutofillUtils.jsm");
 
 this.log = null;
 FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);
 
 const PREF_HEURISTICS_ENABLED = "extensions.formautofill.heuristics.enabled";
 
+const ALLOWED_TYPES = Array.from(FormAutofillUtils.ALLOWED_TYPES);
+
 /**
  * Returns the autocomplete information of fields according to heuristics.
  */
 this.FormAutofillHeuristics = {
   FIELD_GROUPS: {
     NAME: [
       "name",
       "given-name",
@@ -132,71 +134,158 @@ this.FormAutofillHeuristics = {
         result.fieldName = fieldName;
         return result;
       }
     }
     return null;
   },
 
   getInfo(element, fieldDetails) {
-    if (!FormAutofillUtils.isFieldEligibleForAutofill(element)) {
+    let autocomplete = element.autocomplete;
+    let tagName = element.tagName;
+    let type = element.type;
+
+    if (autocomplete == "off") {
+      return null;
+    } else if (tagName == "INPUT") {
+      if (!ALLOWED_TYPES.includes(type)) {
+        return null;
+      }
+    } else if (tagName != "SELECT") {
       return null;
     }
 
-    let info = element.getAutocompleteInfo();
     // An input[autocomplete="on"] will not be early return here since it stll
     // needs to find the field name.
-    if (info && info.fieldName && info.fieldName != "on") {
+    let info = autocomplete && autocomplete != "on" ? element.getAutocompleteInfo() : null;
+    if (info && info.fieldName) {
       return info;
     }
 
     if (!this._prefEnabled) {
       return null;
     }
 
     // "email" type of input is accurate for heuristics to determine its Email
     // field or not. However, "tel" type is used for ZIP code for some web site
     // (e.g. HomeDepot, BestBuy), so "tel" type should be not used for "tel"
     // prediction.
-    if (element.type == "email") {
+    if (type == "email") {
       return {
         fieldName: "email",
         section: "",
         addressType: "",
         contactType: "",
       };
     }
 
-    let existingFieldNames = fieldDetails ? fieldDetails.map(i => i.fieldName) : "";
+    let existingFieldNames = fieldDetails ? fieldDetails.map(i => i.fieldName) : [];
 
     for (let elementString of [element.id, element.name]) {
       let fieldNameResult = this._matchStringToFieldName(elementString,
                                                          existingFieldNames);
       if (fieldNameResult) {
         return fieldNameResult;
       }
     }
-    let labels = FormAutofillUtils.findLabelElements(element);
+    let labels = this.findLabelElements(element);
     if (!labels || labels.length == 0) {
       log.debug("No label found for", element);
       return null;
     }
     for (let label of labels) {
-      let strings = FormAutofillUtils.extractLabelStrings(label);
+      let strings = this.extractLabelStrings(label);
       for (let string of strings) {
         let fieldNameResult = this._matchStringToFieldName(string,
                                                            existingFieldNames);
         if (fieldNameResult) {
           return fieldNameResult;
         }
       }
     }
 
     return null;
   },
+
+  // The tag name list is from Chromium except for "STYLE":
+  // eslint-disable-next-line max-len
+  // https://cs.chromium.org/chromium/src/components/autofill/content/renderer/form_autofill_util.cc?l=216&rcl=d33a171b7c308a64dc3372fac3da2179c63b419e
+  EXCLUDED_TAGS: ["SCRIPT", "NOSCRIPT", "OPTION", "STYLE"],
+  /**
+   * Extract all strings of an element's children to an array.
+   * "element.textContent" is a string which is merged of all children nodes,
+   * and this function provides an array of the strings contains in an element.
+   *
+   * @param  {Object} element
+   *         A DOM element to be extracted.
+   * @returns {Array}
+   *          All strings in an element.
+   */
+  extractLabelStrings(element) {
+    let strings = [];
+    let _extractLabelStrings = (el) => {
+      if (this.EXCLUDED_TAGS.includes(el.tagName)) {
+        return;
+      }
+
+      if (el.nodeType == Ci.nsIDOMNode.TEXT_NODE ||
+          el.childNodes.length == 0) {
+        let trimmedText = el.textContent.trim();
+        if (trimmedText) {
+          strings.push(trimmedText);
+        }
+        return;
+      }
+
+      for (let node of el.childNodes) {
+        if (node.nodeType != Ci.nsIDOMNode.ELEMENT_NODE &&
+            node.nodeType != Ci.nsIDOMNode.TEXT_NODE) {
+          continue;
+        }
+        _extractLabelStrings(node);
+      }
+    };
+    _extractLabelStrings(element);
+    return strings;
+  },
+
+  findLabelElements(element) {
+    let document = element.ownerDocument;
+    let id = element.id;
+    let labels = [];
+    // TODO: querySelectorAll is inefficient here. However, bug 1339726 is for
+    // a more efficient implementation from DOM API perspective. This function
+    // should be refined after input.labels API landed.
+    for (let label of document.querySelectorAll("label[for]")) {
+      if (id == label.htmlFor) {
+        labels.push(label);
+      }
+    }
+
+    if (labels.length > 0) {
+      log.debug("Label found by ID", id);
+      return labels;
+    }
+
+    let parent = element.parentNode;
+    if (!parent) {
+      return [];
+    }
+    do {
+      if (parent.tagName == "LABEL" &&
+          parent.control == element &&
+          !parent.hasAttribute("for")) {
+        log.debug("Label found in input's parent or ancestor.");
+        return [parent];
+      }
+      parent = parent.parentNode;
+    } while (parent);
+
+    return [];
+  },
 };
 
 XPCOMUtils.defineLazyGetter(this.FormAutofillHeuristics, "RULES", () => {
   let sandbox = {};
   let scriptLoader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
                        .getService(Ci.mozIJSSubScriptLoader);
   const HEURISTICS_REGEXP = "chrome://formautofill/content/heuristicsRegexp.js";
   scriptLoader.loadSubScript(HEURISTICS_REGEXP, sandbox, "utf-8");
@@ -205,9 +294,8 @@ XPCOMUtils.defineLazyGetter(this.FormAut
 
 XPCOMUtils.defineLazyGetter(this.FormAutofillHeuristics, "_prefEnabled", () => {
   return Services.prefs.getBoolPref(PREF_HEURISTICS_ENABLED);
 });
 
 Services.prefs.addObserver(PREF_HEURISTICS_ENABLED, () => {
   this.FormAutofillHeuristics._prefEnabled = Services.prefs.getBoolPref(PREF_HEURISTICS_ENABLED);
 });
-
--- a/browser/extensions/formautofill/FormAutofillUtils.jsm
+++ b/browser/extensions/formautofill/FormAutofillUtils.jsm
@@ -113,92 +113,16 @@ this.FormAutofillUtils = {
       }
     } else if (!(element instanceof Ci.nsIDOMHTMLSelectElement)) {
       return false;
     }
 
     return true;
   },
 
-  // The tag name list is from Chromium except for "STYLE":
-  // eslint-disable-next-line max-len
-  // https://cs.chromium.org/chromium/src/components/autofill/content/renderer/form_autofill_util.cc?l=216&rcl=d33a171b7c308a64dc3372fac3da2179c63b419e
-  EXCLUDED_TAGS: ["SCRIPT", "NOSCRIPT", "OPTION", "STYLE"],
-  /**
-   * Extract all strings of an element's children to an array.
-   * "element.textContent" is a string which is merged of all children nodes,
-   * and this function provides an array of the strings contains in an element.
-   *
-   * @param  {Object} element
-   *         A DOM element to be extracted.
-   * @returns {Array}
-   *          All strings in an element.
-   */
-  extractLabelStrings(element) {
-    let strings = [];
-    let _extractLabelStrings = (el) => {
-      if (this.EXCLUDED_TAGS.includes(el.tagName)) {
-        return;
-      }
-
-      if (el.nodeType == Ci.nsIDOMNode.TEXT_NODE ||
-          el.childNodes.length == 0) {
-        let trimmedText = el.textContent.trim();
-        if (trimmedText) {
-          strings.push(trimmedText);
-        }
-        return;
-      }
-
-      for (let node of el.childNodes) {
-        if (node.nodeType != Ci.nsIDOMNode.ELEMENT_NODE &&
-            node.nodeType != Ci.nsIDOMNode.TEXT_NODE) {
-          continue;
-        }
-        _extractLabelStrings(node);
-      }
-    };
-    _extractLabelStrings(element);
-    return strings;
-  },
-
-  findLabelElements(element) {
-    let document = element.ownerDocument;
-    let labels = [];
-    // TODO: querySelectorAll is inefficient here. However, bug 1339726 is for
-    // a more efficient implementation from DOM API perspective. This function
-    // should be refined after input.labels API landed.
-    for (let label of document.querySelectorAll("label[for]")) {
-      if (element.id == label.htmlFor) {
-        labels.push(label);
-      }
-    }
-
-    if (labels.length > 0) {
-      log.debug("Label found by ID", element.id);
-      return labels;
-    }
-
-    let parent = element.parentNode;
-    if (!parent) {
-      return [];
-    }
-    do {
-      if (parent.tagName == "LABEL" &&
-          parent.control == element &&
-          !parent.hasAttribute("for")) {
-        log.debug("Label found in input's parent or ancestor.");
-        return [parent];
-      }
-      parent = parent.parentNode;
-    } while (parent);
-
-    return [];
-  },
-
   loadDataFromScript(url, sandbox = {}) {
     let scriptLoader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
                          .getService(Ci.mozIJSSubScriptLoader);
     scriptLoader.loadSubScript(url, sandbox, "utf-8");
     return sandbox;
   },
 
   /**
--- a/browser/extensions/formautofill/test/unit/test_extractLabelStrings.js
+++ b/browser/extensions/formautofill/test/unit/test_extractLabelStrings.js
@@ -1,11 +1,11 @@
 "use strict";
 
-Cu.import("resource://formautofill/FormAutofillUtils.jsm");
+Cu.import("resource://formautofill/FormAutofillHeuristics.jsm");
 
 const TESTCASES = [
   {
     description: "A label element contains one input element.",
     document: `<label id="typeA"> label type A
                  <!-- This comment should not be extracted. -->
                  <input type="text">
                  <script>FOO</script>
@@ -54,13 +54,13 @@ const TESTCASES = [
 TESTCASES.forEach(testcase => {
   add_task(async function() {
     do_print("Starting testcase: " + testcase.description);
 
     let doc = MockDocument.createTestDocument(
       "http://localhost:8080/test/", testcase.document);
 
     let element = doc.getElementById(testcase.inputId);
-    let strings = FormAutofillUtils.extractLabelStrings(element);
+    let strings = FormAutofillHeuristics.extractLabelStrings(element);
 
     Assert.deepEqual(strings, testcase.expectedStrings);
   });
 });
--- a/browser/extensions/formautofill/test/unit/test_findLabelElements.js
+++ b/browser/extensions/formautofill/test/unit/test_findLabelElements.js
@@ -1,11 +1,11 @@
 "use strict";
 
-Cu.import("resource://formautofill/FormAutofillUtils.jsm");
+Cu.import("resource://formautofill/FormAutofillHeuristics.jsm");
 
 const TESTCASES = [
   {
     description: "Input contains in a label element.",
     document: `<form>
                  <label id="labelA"> label type A
                    <input id="typeA" type="text">
                  </label>
@@ -78,13 +78,13 @@ const TESTCASES = [
 TESTCASES.forEach(testcase => {
   add_task(async function() {
     do_print("Starting testcase: " + testcase.description);
 
     let doc = MockDocument.createTestDocument(
       "http://localhost:8080/test/", testcase.document);
 
     let input = doc.getElementById(testcase.inputId);
-    let labels = FormAutofillUtils.findLabelElements(input);
+    let labels = FormAutofillHeuristics.findLabelElements(input);
 
     Assert.deepEqual(labels.map(l => l.id), testcase.expectedLabelIds);
   });
 });