1092097 - [e10s] Common frame script for mochitest-devtools e10s tests

Status: NEW

(DevTools :: General, defect)

Description

[Patrick Brosset <:pbro>]

See bug 985597 comment 54 and bug 985597 comment 55 for some background on this.

For some of our mochitest-devtools tests, we need frame-scripts.
Some of the things done in these scripts are specific to the tool directory the test is in, but other things are quite generic and could be used by any test.
So there is benefit in sharing some of this code.

We already have a shared frame script that has some interesting message listeners:
browser/devtools/shared/frame-script-utils.js

In bug 985597 attachment 8508708 [details] [diff] [review] I created a new frame script to test the highlighter. It contains a few highlighter-specific things that I don't expect any other test dirs will need, and it also contains a few other things that could be shared:

- Test:ElementFromPoint : to get a CPOW at a given coordinates
- Test:SynthesizeMouse : since the even synthesizer doesn't work on content in e10s
- and maybe others.

We have several options to share frame scripts here:

1 - have only one and put all the code in that one : huge file, hard to maintain
2 - have a common one and one tool-specific per tool dir, and have the common one import all the other ones
3 - same as above but let each tool dir import its own tool-specific frame script in head.js

I vote for 3, I don't think there's a huge value in having the common script import all the other ones. Loading a frame script is one line of code.

Comment 1

[Brian Grinstead [:bgrins]]

> 1 - have only one and put all the code in that one : huge file, hard to
> maintain
> 2 - have a common one and one tool-specific per tool dir, and have the
> common one import all the other ones
> 3 - same as above but let each tool dir import its own tool-specific frame
> script in head.js
> 
> I vote for 3, I don't think there's a huge value in having the common script
> import all the other ones. Loading a frame script is one line of code.

My 2 cents: I vote for option 2 because there is a mental overhead in deciding which frame script your function is in.  There is typically be a helper function like 'executeInContent' within that test's head.js file.  If we had to have 'executeInContent' and 'executeInContentShared' then every time you need to call a frame script you have to think about which file it was in.  If it's all in 1 file then our head.js functions can be exact duplicates, which could be helpful since they currently need to be copy pasted :(.

The possible downside depending on how you see it is that you'd have to prefix your message names with the suite or type of functionality that they are doing to prevent conflicts. ie: instead of "Test:GetHighlighterAttribute" it would be "Inspector:GetHighlighterAttribute" or "Highlighter:GetAttribute".

Comment 2

[Chris Peterson [:cpeterson]]

Moving DevTools test bugs to e10s milestone M7 (i.e. not blocking e10s merge to Aurora).

Comment 3

[Joe Walker [:jwalker]]

This is a test enhancement. It shouldn't block e10s.