Closed Bug 1023862 Opened 10 years ago Closed 10 years ago

Electrolysis (e10s) support for the requestAutocomplete testing framework

Categories

(Toolkit :: Form Manager, defect)

defect
Not set
normal
Points:
5

Tracking

()

RESOLVED FIXED
mozilla34
Iteration:
34.1
Tracking Status
e10s + ---

People

(Reporter: Paolo, Assigned: Paolo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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
Flags: firefox-backlog+
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.
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
Attached patch Work in progress (obsolete) — Splinter Review
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-]
Iteration: 33.2 → 33.3
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+
Iteration: 33.3 → 34.1
Attached patch The patchSplinter Review
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)
Attachment #8461672 - Flags: review?(MattN+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/15c19c592c12
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: