1042253 - Enable devtools/webconsole tests with e10s

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Comment 1

[Mihai Sucan [:msucan]]

Attachment: bug1042253-wip1.diff

Comment 2

[Mihai Sucan [:msucan]]

Attachment: bug1042253-wip2.diff

More tests fixed.

Comment 3

[Mihai Sucan [:msucan]]

Attachment: bug1042253-wip3.diff

More tests running in e10s mode.

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: webconsole-e10s-WIP4.patch

Rebased, fixed a couple more tests, and enabled a few that were fine

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: webconsole-e10s-WIP5.patch

Fixed a couple of more, found a weird one that passes when run alone but fails when run in the group, so I left it semicolon'd out for now with a comment in the browser.ini file

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: webconsole-e10s-WIP6.patch

Fixed 18 additional console tests.

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: webconsole-e10s-WIP7.patch

More tests enabled

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Taking a break from working on these tests.

Comment 9

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP8.patch

Fix one more test, fire events in content and not from the test.

Comment 10

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP9.patch

About 10 more tests.

Comment 11

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP10.patch

Fixes a few more. Getting an error from a previously-enabled test:

737 INFO Full message: TypeError: toolbox.getPanel(...) is undefined
3738 INFO Full stack: runner@chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_webconsole_split_persist.js:41:21

Someone else was having problems in bug 992679 with using toolbox.getPanel("webconsole") to get a reference to the split console, when it's definitely open. Brian, any ideas there?

78 down, 118 to go

Comment 12

[Brian Grinstead [:bgrins]]

(In reply to Heather Arthur  [:harth] from comment #11)
> Created attachment 8477784 [details] [diff] [review]
> webconsole-e10s-WIP10.patch
> 
> Fixes a few more. Getting an error from a previously-enabled test:
> 
> 737 INFO Full message: TypeError: toolbox.getPanel(...) is undefined
> 3738 INFO Full stack:
> runner@chrome://mochitests/content/browser/browser/devtools/webconsole/test/
> browser_webconsole_split_persist.js:41:21
> 
> Someone else was having problems in bug 992679 with using
> toolbox.getPanel("webconsole") to get a reference to the split console, when
> it's definitely open. Brian, any ideas there?
> 
> 78 down, 118 to go

This is weird, buttonsPromise is throwing an exception (from the _buildButtons call), which is maybe causing the promise.all call to not wait for the split console or something?  If I comment the buttonsPromise line out here it passes: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/framework/toolbox.js#282.

There seem to be two relevant errors from buildButtons (I'll file a new bug for it)

106 INFO console.error:
107 INFO   Message: TypeError: window is null
108 INFO   Stack:
109 INFO     exports.items<.state.isChecked@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/commands/paintflashing.js:106:15
110 INFO CommandUtils.createButtons/</</onChange@resource:///modules/devtools/DeveloperToolbar.jsm:156:27
111 INFO CommandUtils.createButtons/</<@resource:///modules/devtools/DeveloperToolbar.jsm:167:1
112 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
113 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
114 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
115 INFO CommandUtils.createButtons/<@resource:///modules/devtools/DeveloperToolbar.jsm:99:14
116 INFO exports.promiseEach/</callNext@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:277:19
117 INFO exports.promiseEach/</callNext/onSuccess@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:269:11
118 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
119 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
120 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
121 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
122 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
123 INFO exports.promiseEach/</callNext@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:278:7
124 INFO exports.promiseEach/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:281:5
125 INFO Promise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/promise.js:48:5
126 INFO exports.promiseEach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:258:1
127 INFO CommandUtils.createButtons@resource:///modules/devtools/DeveloperToolbar.jsm:97:1
128 INFO Toolbox.prototype._buildButtons/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:601:14
129 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
130 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
131 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
132 INFO Toolbox.prototype._buildButtons@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:599:12
133 INFO Toolbox.prototype.open/</domReady@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:267:30

and

9 INFO console.error:
10 INFO   Message: TypeError: window is null
11 INFO   Stack:
12 INFO     exports.items<.state.isChecked@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/commands/paintflashing.js:106:15
13 INFO CommandUtils.createButtons/</</onChange@resource:///modules/devtools/DeveloperToolbar.jsm:156:27
14 INFO CommandUtils.createButtons/</<@resource:///modules/devtools/DeveloperToolbar.jsm:167:1
15 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
16 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
17 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
18 INFO CommandUtils.createButtons/<@resource:///modules/devtools/DeveloperToolbar.jsm:99:14
19 INFO exports.promiseEach/</callNext@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:277:19
20 INFO exports.promiseEach/</callNext/onSuccess@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:269:11
21 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
22 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
23 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
24 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
25 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:58:9
26 INFO exports.promiseEach/</callNext@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:278:7
27 INFO exports.promiseEach/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:281:5
28 INFO Promise@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/promise.js:48:5
29 INFO exports.promiseEach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/util.js:258:1
30 INFO CommandUtils.createButtons@resource:///modules/devtools/DeveloperToolbar.jsm:97:1
31 INFO Toolbox.prototype._buildButtons/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:601:14
32 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
33 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
34 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
35 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:11
36 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
37 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
38 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:11
39 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
40 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
41 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
42 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:11
43 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
44 INFO then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
45 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
46 INFO resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:11
47 INFO Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:865:23
48 INFO this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:7

Comment 13

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP11.patch

10 more.

88 down, 108 to go

Comment 14

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP12.patch

101 down, 95 to go

Comment 15

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP13.patch

128 down, 68 to go

Comment 16

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP14.patch

Rebased.

Comment 17

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP15.patch

146 down, 50 to go

Comment 18

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP16.patch

I've done about all I can do. There are about 40 left. Many of them are tough ones. We could knock a bunch out if we figured out how to get around some common patterns though.

I had to comment out a bunch that need expectUncaughtException() in regular mode, but fail with that in e10s mode as the errors thrown from content don't reach the test or something like that.

A bunch of the unconverted tests are trying to send mouse events to a content button, which doesn't work under e10s.

Others I left are just trying to access variables from the content window.

Additionally, console.log() from content may not be showing up in the browser console, according to the test browser_console.js.

Comment 19

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP17.patch

Rebased. Comments above stand.

Comment 20

[Brian Grinstead [:bgrins]]

(In reply to Heather Arthur  [:harth] from comment #18)
> Created attachment 8483893 [details] [diff] [review]
> webconsole-e10s-WIP16.patch
> 
> I've done about all I can do. There are about 40 left. Many of them are
> tough ones. We could knock a bunch out if we figured out how to get around
> some common patterns though.
> 
> I had to comment out a bunch that need expectUncaughtException() in regular
> mode, but fail with that in e10s mode as the errors thrown from content
> don't reach the test or something like that.
> 
> A bunch of the unconverted tests are trying to send mouse events to a
> content button, which doesn't work under e10s.
> 
> Others I left are just trying to access variables from the content window.
> 
> Additionally, console.log() from content may not be showing up in the
> browser console, according to the test browser_console.js.

Great!  I'd say it's ready for review if browser.ini is updated to remove the semicolons and instead add skip-if = e10s for the still affected tests.

Comment 21

[Chris Peterson [:cpeterson]]

(In reply to Heather Arthur  [:harth] from comment #18)
> I've done about all I can do. There are about 40 left. Many of them are
> tough ones. We could knock a bunch out if we figured out how to get around
> some common patterns though.
> 
> I had to comment out a bunch that need expectUncaughtException() in regular
> mode, but fail with that in e10s mode as the errors thrown from content
> don't reach the test or something like that.
> 
> A bunch of the unconverted tests are trying to send mouse events to a
> content button, which doesn't work under e10s.

Heather, are these e10s issues with ours test frameworks? Do you need any guidance from e10s developers on fixing the common problem patterns?


> Additionally, console.log() from content may not be showing up in the
> browser console, according to the test browser_console.js.

I think this is bug 1060093.

Comment 22

[Heather Arthur [:harth]]

(In reply to Chris Peterson (:cpeterson) from comment #21)
> (In reply to Heather Arthur  [:harth] from comment #18)
> > I had to comment out a bunch that need expectUncaughtException() in regular
> > mode, but fail with that in e10s mode as the errors thrown from content
> > don't reach the test or something like that.
> > 
> > A bunch of the unconverted tests are trying to send mouse events to a
> > content button, which doesn't work under e10s.
> 
> Heather, are these e10s issues with ours test frameworks? Do you need any
> guidance from e10s developers on fixing the common problem patterns?

These issues I think would be common to all tests. expectUncaughtException() is a SimpleTest thing, and the mouse events are sent using EventUtils.

the expectUncaughtException() issue would be fine if we completely switched to running e10s only, as we could just take them all out.

> > Additionally, console.log() from content may not be showing up in the
> > browser console, according to the test browser_console.js.
> 
> I think this is bug 1060093.

What's odd is that the logs show up in the content console, but don't show up in the browser console. At least in the test. So I'm not sure it's the same.

Comment 23

[Heather Arthur [:harth]]

Update: I tried converting all the commented-out tests to skip-if, and curiously got a crash in e10s mode when running the lot.

Comment 24

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP18.patch

Here's the patch with skip-ifs instead of comments. I just ran them again. Maybe what I was seeing was a tab crash. In one test, a test crashes and the test times out.

Comment 25

[Brian Grinstead [:bgrins]]

(In reply to Heather Arthur  [:harth] from comment #24)
> Created attachment 8486092 [details] [diff] [review]
> webconsole-e10s-WIP18.patch
> 
> Here's the patch with skip-ifs instead of comments. I just ran them again.
> Maybe what I was seeing was a tab crash. In one test, a test crashes and the
> test times out.

Went ahead and pushed to try with e10s enabled to see what happens there: https://tbpl.mozilla.org/?tree=Try&rev=1f368856f6c9

Comment 26

[Brian Grinstead [:bgrins]]

(In reply to Heather Arthur  [:harth] from comment #24)
> Created attachment 8486092 [details] [diff] [review]
> webconsole-e10s-WIP18.patch
> 
> Here's the patch with skip-ifs instead of comments. I just ran them again.
> Maybe what I was seeing was a tab crash. In one test, a test crashes and the
> test times out.

Looks like there is a crash on browser_webconsole_split_persist.js [0] - https://tbpl.mozilla.org/php/getParsedLog.php?id=47641876&full=1&branch=try.

I'd suggest just skipping that test with a comment (and bucketing it with the other difficult ones) if it gets the suite running again.

Comment 27

[Brian Grinstead [:bgrins]]

I see a few more errors and leaks on the linux debug build: https://tbpl.mozilla.org/php/getParsedLog.php?id=47640037&full=1&branch=try

INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_bug664688_sandbox_update_after_navigation.js | uncaught exception - TypeError: contentWindow is null at chrome://browser/content/browser.js:4509
INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_webconsole_bug_658368_time_methods.js | Test timed out - expected PASS
INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/webconsole/test/browser_webconsole_clickable_urls.js | Test timed out - expected PASS

Leaks:

TEST-UNEXPECTED-FAIL | browser/devtools/webconsole/test/browser_webconsole_split_escape_key.js | leaked 2 window(s) until shutdown [url = about:blank]
TEST-UNEXPECTED-FAIL | browser/devtools/webconsole/test/browser_webconsole_split_focus.js | leaked 2 window(s) until shutdown [url = about:blank]
TEST-UNEXPECTED-FAIL | browser/devtools/webconsole/test/browser_webconsole_output_table.js | leaked 1 window(s) until shutdown [url = http://example.com/browser/browser/devtools/webconsole/test/test-console-table.html]

Comment 28

[Panos Astithas (he/him) [:past] (please ni?)]

I wouldn't worry about the leaks yet, failed tests usually don't clean up after themselves.

Comment 29

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP19.patch

Rebased. Now we need to convert the non-e10s tests to loadTab() and openConsole()-as-a-promise. I've started on that.

Comment 30

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP20.patch

Sorry, huge patch. This rewrites all the webconsole tests to use new promise-y helper methods (loadTab and openConsole). It also changes some tests to work on e10s. Some are disabled with e10s as they are intermittently failing or would take to long to fix this round.

Still getting leaks from various tests on debug builds (https://tbpl.mozilla.org/?tree=Try&rev=65f822e21b1c), does it still stand that that doesn't matter?

Comment 31

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8502045 [details] [diff] [review]
webconsole-e10s-WIP20.patch

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

What I meant above was that we should only start working on fixing the leaks after we've made sure that no other test failures occur. This seems to be the case in some of the oranges I looked at in the last try run, but if you prefer to just disable the leaky tests for now, I'm also fine with that. It's always good to make concrete steps forward.

Related to that, there are a lot of oranges in unrelated areas, like the style inspector that are obscuring the results of the try run. Didn't we disable all the failing tests/suites or is it that your try push just wasn't rebased on the latest fx-team tip?

Comment 32

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-WIP21.patch

As much as I'd like to get this in the bag, I'm going to focus on fixing some e10s functionality in other places.

These pass locally for me, but there are several crashes and memory leaks on try. Whenever I disable the leaks, leaks pop up in other tests and whenever I disable a crash, a crash pops up somewhere else.

This probably means the crashes are due to running out of memory, I think.

Latest run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=934899582a7d

Comment 33

[Chris Peterson [:cpeterson]]

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

Comment 34

[Heather Arthur [:harth]]

Attachment: webconsole-e10s-tests-v22.patch

Okay, this is it.

head.js now defines loadTab() and removes addTab(), as well as makes openConsole() promise-y. A lot of changes are because of that. A lot of tests were also cleaned up to use an asyncTest() helper and yields.

Passes plain try: https://tbpl.mozilla.org/?tree=Try&rev=e51bf4cc7904
I've disabled the tests that time out here: https://tbpl.mozilla.org/?tree=Try&rev=da4892fceaac

There are still some leaks, but they exist throughout our code and this is bug 1094749.

This leaves about 40 tests disabled. 10 of these are due to the expectUncaughtException() calls I mentioned in comment 18, which I'll file a followup for. about 8 are disabled just for Linux debug timeouts.

Comment 35

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8534511 [details] [diff] [review]
webconsole-e10s-tests-v22.patch

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

My eyes are bleeding after reviewing all of this, but I'm so excited about the task-based rewrite! W00t!

::: browser/devtools/webconsole/test/browser_warn_user_about_replaced_api.js
@@ +22,5 @@
> +  let hud2 = yield openConsole();
> +
> +  yield testWarningPresent(hud2);
> +
> +  Services.prefs.clearUserPref(PREF)

Nit: missing semicolon.

::: browser/devtools/webconsole/test/browser_webconsole_autocomplete_in_debugger_stackframe.js
@@ +137,5 @@
>    openDebugger().then(() => {
>      gStackframes.selectFrame(1);
>  
>      info("openConsole");
> +    executeSoon(() => openConsole().then(testDriver.next()));

then() should have a lambda that calls next() (not use its return value as a callback):

then(() => testDriver.next())

::: browser/devtools/webconsole/test/browser_webconsole_autocomplete_popup_close_on_tab_switch.js
@@ +27,5 @@
> +  panel.addEventListener("popupshown", function popupOpened() {
> +    panel.removeEventListener("popupshown", popupOpened, false);
> +    loadTab("data:text/html;charset=utf-8,<p>testing autocomplete closes").then(() => {
> +      finished.resolve();
> +    });

Nit: then(() => finished.resolve()) is shorter.

::: browser/devtools/webconsole/test/browser_webconsole_bug_595934_message_categories.js
@@ +1,1 @@
> +

Hmm?

::: browser/devtools/webconsole/test/browser_webconsole_bug_597103_deactivateHUDForContext_unfocused_window.js
@@ +1,1 @@
> +

Look, there it is again!

::: browser/devtools/webconsole/test/head.js
@@ +17,5 @@
> +
> +/*
> +
> +  loadTab(TEST_URI).then(() => {
> +    openConsole().then(() => {

Did you mean to add a comment here, like "common test setup code"?

@@ +399,4 @@
>      }
>    }
>  
> +  setTimeout(wait, 100);

Wouldn't executeSoon() be better in this case? It doesn't seem like we actually need the 100 msec delay (although I don't know if we ever succeed or fail in this first call either; if not, a timeout is better).

::: browser/devtools/webconsole/test/test-console-output-events.html
@@ +19,5 @@
>    document.addEventListener("mousemove", eventLogger);
>    document.addEventListener("keypress", eventLogger);
> +
> +  synthesizeKeyPress("a", {shiftKey: true});
> +  synthesizeMouseMove();

The test expects these events in the reverse order, is this order intentional?

Comment 36

[Heather Arthur [:harth]]

Attachment: For check in

Fixed to comments. Thanks for doing the huge review Panos.

Plain try:
https://tbpl.mozilla.org/?tree=Try&rev=67337d785dda

e10s:
https://tbpl.mozilla.org/?tree=Try&rev=56e43e6ed63d
(super orange, but they are known or leaks probably due to bug 1073352)

Comment 37

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing, sorry :(

Comment 38

[Heather Arthur [:harth]]

Attachment: Rebased, for check in

Rebased.

Try: https://tbpl.mozilla.org/?tree=Try&rev=577b61c32713
e10s: https://tbpl.mozilla.org/?tree=Try&rev=1032157053d9

Comment 39

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/84880f27bab3

Comment 40

[Heather Arthur [:harth]]

Leaving the rest of the tests to someone else.

Comment 41

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/84880f27bab3

Comment 42

[Panos Astithas (he/him) [:past] (please ni?)]

Filed bug 1112599 to track enabling the rest of the web console tests.