Electrolysis (e10s) support for the requestAutocomplete testing framework

RESOLVED FIXED in mozilla34

Status

()

Toolkit
Form Manager
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks: 1 bug)

Trunk
mozilla34
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(e10s+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
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

Updated

4 years ago
Flags: firefox-backlog+
(Assignee)

Comment 1

4 years ago
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.
(Assignee)

Comment 2

4 years ago
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.
Summary: Electrolysis (e10s) and Android support for the requestAutocomplete testing framework → Electrolysis (e10s) support for the requestAutocomplete testing framework
(Assignee)

Comment 3

4 years ago
Created attachment 8451634 [details] [diff] [review]
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.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #8451634 - Flags: feedback?(MattN+bmo)
Added to Iteration 33.2
Iteration: --- → 33.2
Points: --- → 5
QA Whiteboard: [qa-]
Whiteboard: p=5 [qa-]

Updated

4 years ago
Iteration: 33.2 → 33.3
(Assignee)

Updated

4 years ago
Depends on: 1020607
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.
Attachment #8451634 - Flags: feedback?(MattN+bmo) → feedback+

Updated

4 years ago
Iteration: 33.3 → 34.1
(Assignee)

Comment 6

4 years ago
Created attachment 8461672 [details] [diff] [review]
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.
Attachment #8451634 - Attachment is obsolete: true
Attachment #8461672 - Flags: review?(MattN+bmo)
tracking-e10s: --- → +
Attachment #8461672 - Flags: review?(MattN+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/15c19c592c12
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.