Bug 1589938, convert FormHistory modules into JSWindowActors, allowing the test test_form_submission.html to work with fission enabled, r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D49969

--HG--
rename : toolkit/components/satchel/FormSubmitChild.jsm => toolkit/components/satchel/FormHistoryChild.jsm
extra : moz-landing-system : lando

toolkit/components/satchel/FormAutoComplete.jsm

@@ -39,17 +39,14 @@ function isAutocompleteDisabled(aField) {
 function FormHistoryClient({ formField, inputName }) {
   if (formField && inputName != this.SEARCHBAR_ID) {
     let window = formField.ownerGlobal;
-    this.mm = window.docShell.messageManager;
+    this.windowGlobal = window.getWindowGlobalChild();
-  } else {
+  } else if (inputName == this.SEARCHBAR_ID && formField) {
-    if (inputName == this.SEARCHBAR_ID && formField) {
+    throw new Error(
-      throw new Error(
+      "FormHistoryClient constructed with both a " +
-        "FormHistoryClient constructed with both a " +
+        "formField and an inputName. This is not " +
-          "formField and an inputName. This is not " +
+        "supported, and only empty results will be " +
-          "supported, and only empty results will be " +
+        "returned."
-          "returned."
+    );
-      );
-    }
-    this.mm = Services.cpmm;
   }
 
   this.inputName = inputName;
@@ -59,15 +56,16 @@ function FormHistoryClient({ formField, inputName }) {
 FormHistoryClient.prototype = {
   SEARCHBAR_ID: "searchbar-history",
 
-  // It is assumed that nsFormAutoComplete only uses / cares about
+  cancelled: false,
-  // one FormHistoryClient at a time, and won't attempt to have
-  // multiple in-flight searches occurring with the same FormHistoryClient.
-  // We use an ID number per instantiated FormHistoryClient to make
-  // sure we only respond to messages that were meant for us.
-  id: 0,
-  callback: null,
   inputName: "",
-  mm: null,
+
+  getActor() {
+    if (this.windowGlobal) {
+      return this.windowGlobal.getActor("FormHistory");
+    }
+
+    return null;
+  },
 
   /**
    * Query FormHistory for some results.
@@ -83,14 +81,50 @@ FormHistoryClient.prototype = {
    *        argument (the found entries).
    */
   requestAutoCompleteResults(searchString, params, callback) {
-    this.mm.sendAsyncMessage("FormHistory:AutoCompleteSearchAsync", {
+    this.cancelled = false;
-      id: this.id,
-      searchString,
-      params,
-    });
 
-    this.mm.addMessageListener("FormHistory:AutoCompleteSearchResults", this);
+    // Use the actor if possible, otherwise for the searchbar,
-    this.callback = callback;
+    // use the more roundabout per-process message manager which has
+    // no sendQuery method.
+    let actor = this.getActor();
+    if (actor) {
+      actor
+        .sendQuery("FormHistory:AutoCompleteSearchAsync", {
+          searchString,
+          params,
+        })
+        .then(
+          results => {
+            this.handleAutoCompleteResults(results, callback);
+          },
+          () => this.cancel()
+        );
+    } else {
+      this.callback = callback;
+      Services.cpmm.addMessageListener(
+        "FormHistory:AutoCompleteSearchResults",
+        this
+      );
+      Services.cpmm.sendAsyncMessage("FormHistory:AutoCompleteSearchAsync", {
+        id: this.id,
+        searchString,
+        params,
+      });
+    }
+  },
+
+  handleAutoCompleteResults(results, callback) {
+    if (this.cancelled) {
+      return;
+    }
+
+    if (!callback) {
+      Cu.reportError("FormHistoryClient received response with no callback");
+      return;
+    }
+
+    callback(results);
+    this.cancel();
   },
 
   /**
@@ -99,7 +133,14 @@ FormHistoryClient.prototype = {
    * called from this FormHistoryClient.
    */
   cancel() {
-    this.clearListeners();
+    if (this.callback) {
+      Services.cpmm.removeMessageListener(
+        "FormHistory:AutoCompleteSearchResults",
+        this
+      );
+      this.callback = null;
+    }
+    this.cancelled = true;
   },
 
   /**
@@ -115,34 +156,19 @@ FormHistoryClient.prototype = {
    *        The guid for the item being removed.
    */
   remove(value, guid) {
-    this.mm.sendAsyncMessage("FormHistory:RemoveEntry", {
+    let actor = this.getActor() || Services.cpmm;
+    actor.sendAsyncMessage("FormHistory:RemoveEntry", {
       inputName: this.inputName,
       value,
       guid,
     });
   },
 
-  // Private methods
-
   receiveMessage(msg) {
     let { id, results } = msg.data;
-    if (id != this.id) {
+    if (id == this.id) {
-      return;
+      this.handleAutoCompleteResults(results, this.callback);
     }
-    if (!this.callback) {
-      Cu.reportError("FormHistoryClient received message with no callback");
-      return;
-    }
-    this.callback(results);
-    this.clearListeners();
-  },
-
-  clearListeners() {
-    this.mm.removeMessageListener(
-      "FormHistory:AutoCompleteSearchResults",
-      this
-    );
-    this.callback = null;
   },
 };
 

toolkit/components/satchel/FormHistoryChild.jsm

@@ -4,10 +4,10 @@
 
 "use strict";
 
-var EXPORTED_SYMBOLS = ["FormSubmitChild"];
+var EXPORTED_SYMBOLS = ["FormHistoryChild"];
 
-const { ActorChild } = ChromeUtils.import(
+const { XPCOMUtils } = ChromeUtils.import(
-  "resource://gre/modules/ActorChild.jsm"
+  "resource://gre/modules/XPCOMUtils.jsm"
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
@@ -22,46 +22,18 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/PrivateBrowsingUtils.jsm"
 );
 
-class FormSubmitChild extends ActorChild {
+XPCOMUtils.defineLazyPreferenceGetter(this, "gDebug", "browser.formfill.debug");
-  constructor(dispatcher) {
+XPCOMUtils.defineLazyPreferenceGetter(this, "gEnabled", "browser.formfill.enable");
-    super(dispatcher);
 
-    this.QueryInterface = ChromeUtils.generateQI([
+function log(message) {
-      Ci.nsIObserver,
+  if (!gDebug) {
-      Ci.nsISupportsWeakReference,
+    return;
-    ]);
-
-    Services.prefs.addObserver("browser.formfill.", this);
-    this.updatePrefs();
-  }
-
-  cleanup() {
-    Services.prefs.removeObserver("browser.formfill.", this);
-  }
-
-  updatePrefs() {
-    this.debug = Services.prefs.getBoolPref("browser.formfill.debug");
-    this.enabled = Services.prefs.getBoolPref("browser.formfill.enable");
-  }
-
-  log(message) {
-    if (!this.debug) {
-      return;
-    }
-    dump("satchelFormListener: " + message + "\n");
-    Services.console.logStringMessage("satchelFormListener: " + message);
-  }
-
-  /* ---- nsIObserver interface ---- */
-
-  observe(subject, topic, data) {
-    if (topic == "nsPref:changed") {
-      this.updatePrefs();
-    } else {
-      this.log("Oops! Unexpected notification: " + topic);
-    }
   }
+  dump("satchelFormListener: " + message + "\n");
+  Services.console.logStringMessage("satchelFormListener: " + message);
+}
 
+class FormHistoryChild extends JSWindowActorChild {
   handleEvent(event) {
     switch (event.type) {
       case "DOMFormBeforeSubmit": {
@@ -77,13 +49,13 @@ class FormSubmitChild extends ActorChild {
   onDOMFormBeforeSubmit(event) {
     let form = event.target;
     if (
-      !this.enabled ||
+      !gEnabled ||
       PrivateBrowsingUtils.isContentWindowPrivate(form.ownerGlobal)
     ) {
       return;
     }
 
-    this.log("Form submit observer notified.");
+    log("Form submit observer notified.");
 
     if (
       form.hasAttribute("autocomplete") &&
@@ -127,7 +99,7 @@ class FormSubmitChild extends ActorChild {
 
       // Don't save credit card numbers.
       if (CreditCard.isValidNumber(value)) {
-        this.log("skipping saving a credit card number");
+        log("skipping saving a credit card number");
         continue;
       }
 
@@ -137,19 +109,19 @@ class FormSubmitChild extends ActorChild {
       }
 
       if (name == "searchbar-history") {
-        this.log('addEntry for input name "' + name + '" is denied');
+        log('addEntry for input name "' + name + '" is denied');
         continue;
       }
 
       // Limit stored data to 200 characters.
       if (name.length > 200 || value.length > 200) {
-        this.log("skipping input that has a name/value too large");
+        log("skipping input that has a name/value too large");
         continue;
       }
 
       // Limit number of fields stored per form.
       if (entries.length >= 100) {
-        this.log("not saving any more entries for this form.");
+        log("not saving any more entries for this form.");
         break;
       }
 
@@ -157,7 +129,7 @@ class FormSubmitChild extends ActorChild {
     }
 
     if (entries.length) {
-      this.log("sending entries to parent process for form " + form.id);
+      log("sending entries to parent process for form " + form.id);
       this.sendAsyncMessage("FormHistory:FormSubmitEntries", entries);
     }
   }

toolkit/components/satchel/FormHistoryParent.jsm

@@ -0,0 +1,72 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+"use strict";
+
+var EXPORTED_SYMBOLS = ["FormHistoryParent"];
+
+ChromeUtils.defineModuleGetter(
+  this,
+  "FormHistory",
+  "resource://gre/modules/FormHistory.jsm"
+);
+
+class FormHistoryParent extends JSWindowActorParent {
+  receiveMessage(message) {
+    switch (message.name) {
+      case "FormHistory:FormSubmitEntries": {
+        let entries = message.data;
+        let changes = entries.map(entry => ({
+          op: "bump",
+          fieldname: entry.name,
+          value: entry.value,
+        }));
+
+        FormHistory.update(changes);
+        break;
+      }
+
+      case "FormHistory:AutoCompleteSearchAsync":
+        return this.autoCompleteSearch(message);
+
+      case "FormHistory:RemoveEntry":
+        this.removeEntry(message);
+        break;
+    }
+
+    return undefined;
+  }
+
+  autoCompleteSearch(message) {
+    let { searchString, params } = message.data;
+
+    return new Promise((resolve, reject) => {
+      let results = [];
+      let processResults = {
+        handleResult: aResult => {
+          results.push(aResult);
+        },
+        handleCompletion: aReason => {
+          if (!aReason) {
+            resolve(results);
+          } else {
+            reject();
+          }
+        },
+      };
+
+      FormHistory.getAutoCompleteResults(searchString, params, processResults);
+    });
+  }
+
+  removeEntry(message) {
+    let { inputName, value, guid } = message.data;
+    FormHistory.update({
+      op: "remove",
+      fieldname: inputName,
+      value,
+      guid,
+    });
+  }
+}

toolkit/components/satchel/FormHistoryStartup.jsm

@@ -50,32 +50,20 @@ FormHistoryStartup.prototype = {
     Services.obs.addObserver(this, "idle-daily", true);
     Services.obs.addObserver(this, "formhistory-expire-now", true);
 
-    Services.mm.addMessageListener("FormHistory:FormSubmitEntries", this);
+    Services.ppmm.addMessageListener(
-
+      "FormHistory:AutoCompleteSearchAsync",
-    // For each of these messages, we could receive them from content,
+      this
-    // or we might receive them from the ppmm if the searchbar is
+    );
-    // having its history queried.
+    Services.ppmm.addMessageListener("FormHistory:RemoveEntry", this);
-    for (let manager of [Services.mm, Services.ppmm]) {
-      manager.addMessageListener("FormHistory:AutoCompleteSearchAsync", this);
-      manager.addMessageListener("FormHistory:RemoveEntry", this);
-    }
   },
 
   receiveMessage(message) {
     switch (message.name) {
-      case "FormHistory:FormSubmitEntries": {
-        let entries = message.data;
-        let changes = entries.map(entry => ({
-          op: "bump",
-          fieldname: entry.name,
-          value: entry.value,
-        }));
-
-        FormHistory.update(changes);
-        break;
-      }
-
       case "FormHistory:AutoCompleteSearchAsync": {
+        // This case is only used for the search field. There is a
+        // similar algorithm in FormHistoryParent.jsm that uses
+        // sendQuery for other form fields.
+
         let { id, searchString, params } = message.data;
 
         if (this.pendingQuery) {
@@ -83,19 +71,7 @@ FormHistoryStartup.prototype = {
           this.pendingQuery = null;
         }
 
-        let mm;
         let query = null;
-        // MessageListenerManager is a Mozilla-only interface, so disable the eslint error
-        // for it.
-        // eslint-disable-next-line no-undef
-        if (message.target instanceof MessageListenerManager) {
-          // The target is the PPMM, meaning that the parent process
-          // is requesting FormHistory data on the searchbar.
-          mm = message.target;
-        } else {
-          // Otherwise, the target is a <xul:browser>.
-          mm = message.target.messageManager;
-        }
 
         let results = [];
         let processResults = {
@@ -109,10 +85,13 @@ FormHistoryStartup.prototype = {
             if (query === this.pendingQuery) {
               this.pendingQuery = null;
               if (!aReason) {
-                mm.sendAsyncMessage("FormHistory:AutoCompleteSearchResults", {
+                message.target.sendAsyncMessage(
-                  id,
+                  "FormHistory:AutoCompleteSearchResults",
-                  results,
+                  {
-                });
+                    id,
+                    results,
+                  }
+                );
               }
             }
           },

toolkit/components/satchel/moz.build

@@ -42,7 +42,8 @@ XPCOM_MANIFESTS += [
 include('/ipc/chromium/chromium-config.mozbuild')
 
 FINAL_TARGET_FILES.actors += [
-    'FormSubmitChild.jsm',
+    'FormHistoryChild.jsm',
+    'FormHistoryParent.jsm',
 ]
 
 FINAL_LIBRARY = 'xul'

toolkit/components/satchel/test/mochitest.ini

@@ -16,7 +16,6 @@ skip-if = (verify && debug && (os == 'win')) || (os == 'mac') # Bug 1514249
 [test_form_autocomplete_validation_at_input_event.html]
 [test_form_autocomplete_with_list.html]
 [test_form_submission.html]
-fail-if = fission
 [test_form_submission_cap.html]
 [test_form_submission_cap2.html]
 [test_input_valid_state_with_autocomplete.html]

toolkit/components/satchel/test/subtst_form_submission_1.html

@@ -6,23 +6,12 @@
 
 <body>
 
-<form id="subform2" onsubmit="return checkSubmit(100)">
+<form id="subform2" onsubmit="return false">
   <input id="subtest2" type="text" name="subtest2">
   <button type="submit">Submit</button>
 </form>
 
 <script>
-  /* exported checkSubmit clickButton */
-  function checkSubmit(num) {
-    return SpecialPowers.wrap(parent).wrappedJSObject.checkSubmit(num);
-  }
-
-  function clickButton(num) {
-    if (num == 100) {
-      document.querySelectorAll("button")[0].click();
-    }
-  }
-
   // set the input's value (can't use a default value, as satchel will ignore it)
   document.getElementById("subtest2").value = "subtestValue";
 </script>

toolkit/components/satchel/test/test_form_submission.html

@@ -490,8 +490,18 @@ function submitForm(formNum) {
       // from an HTTPS domain in an iframe.
       if (nextFormNum == 100) {
         ok(true, "submitting iframe test " + nextFormNum);
-        SpecialPowers.wrap(document.getElementById("iframe").contentWindow)
+
-        .wrappedJSObject.clickButton(nextFormNum);
+        // Need to call checkSubmit first, as the iframe's document can be in another
+        // process and won't be able to notify back before the submit occurs.
+        checkSubmit(100);
+
+        let browsingContext = SpecialPowers.unwrap(
+          SpecialPowers.wrap(document.getElementById("iframe")).browsingContext);
+        SpecialPowers.spawn(browsingContext, [], () => {
+          /* eslint-disable no-undef */
+          content.document.querySelectorAll("button")[0].click();
+          /* eslint-enable no-undef */
+        });
       } else {
         let button = getFormSubmitButton(nextFormNum);
         button.click();

toolkit/modules/ActorManagerParent.jsm

@@ -263,6 +263,20 @@ let ACTORS = {
     allFrames: true,
   },
 
+  FormHistory: {
+    parent: {
+      moduleURI: "resource://gre/actors/FormHistoryParent.jsm",
+    },
+    child: {
+      moduleURI: "resource://gre/actors/FormHistoryChild.jsm",
+      events: {
+        DOMFormBeforeSubmit: {},
+      },
+    },
+
+    allFrames: true,
+  },
+
   InlineSpellChecker: {
     parent: {
       moduleURI: "resource://gre/actors/InlineSpellCheckerParent.jsm",
@@ -367,16 +381,6 @@ let LEGACY_ACTORS = {
     },
   },
 
-  FormSubmit: {
-    child: {
-      module: "resource://gre/actors/FormSubmitChild.jsm",
-      allFrames: true,
-      events: {
-        DOMFormBeforeSubmit: {},
-      },
-    },
-  },
-
   KeyPressEventModelChecker: {
     child: {
       module: "resource://gre/actors/KeyPressEventModelCheckerChild.jsm",