1023862 - Electrolysis (e10s) support for the requestAutocomplete testing framework

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[:Paolo Amadini]

Bug 1021060 adds support for xpcshell, mochitest-chrome, and browser-chrome tests for the requestAutocomplete feature. However, those tests are not compatible with Android, B2G and Electrolysis.

The mochitest-chrome tests will probably need to be converted to mochitest-plain in a way that preserves their ability to operate in a privileged way in both the parent and the content process.

The xpcshell test suite would need to support running some tests in the content process where needed.

All the frameworks will require various functions that should be shared:
* Initialization in the parent process
* Initialization in the content process
* Common utility functions for the parent and/or content process
* Execution of the test in either the parent or content process
* Asynchronous execution of all the tests (add_task)
* Termination matching initialization in both processes

Comment 1

[:Paolo Amadini]

For example, we'll need a shared Preferences API that uses pushPrefEnv or a similar asynchronous technique to ensure the preferences are updated in both processes before continuing.

Comment 2

[:Paolo Amadini]

I think we should use this bug for e10s support specifically, as Android testing will be done by porting the mochitest-browser-chrome framework, and later with some Java UI testing.

Comment 3

[:Paolo Amadini]

Attachment: Work in progress

This adds e10s support to mochitest-chrome. Test files and the common head file can use a function called "add_task_in_parent" to add a function that can be executed at that point in the parent process.

Each mochitest is split in two XHTML and JS files. This technique is already used elsewhere in the tree (the "browser-element" tests), though they introduce some redundancy that is not needed here.

The infrastructure needs more comments, but the usage in the test files would look like it currently is.

Comment 4

[Marco Mucci [:MarcoM]]

Added to Iteration 33.2

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8451634 [details] [diff] [review]
Work in progress

Review of attachment 8451634 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: toolkit/components/formautofill/test/chrome/head_parent.js
@@ +27,5 @@
> +
> +let add_task = taskFn => {};
> +let add_task_in_parent_process = function (taskFn) {
> +  let taskId = taskFn.name; // Should be the file and line number
> +  addMessageListener("start_task_" + taskId, function () {

We should probably do something smarter when there is no function name. e.g. fail with an error message or extract a more unique name.

Comment 6

[:Paolo Amadini]

Attachment: The patch

Hi Matt, this patch adds all the code comments for the framework, and I've also moved some files around to make their purpose clearer:

- All the "loader" files are part of the framework itself, should not normally require editing if the required features are there.
- All the "head" files contain shared functions for Form Autofill, that may be declared using the functions available in the framework, like add_task.

There is a small exception for "head.js" in browser-chrome, since loading must start from that file, but it is only one line of code and explained there.

I figured out how to get rid of the duplicate "xpcshell.ini" after re-reading the manifest parser documentation. Basically we need to load the common files using loadSubScript instead of the "head" directive.

Comment 7

[:Paolo Amadini]

https://hg.mozilla.org/integration/fx-team/rev/15c19c592c12

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15c19c592c12