1399956 - Run plain mochitests in Headless Mode on Linux

Status: RESOLVED FIXED

(Firefox :: Headless, enhancement, P1)

Description

[Brendan Dahl [:bdahl]]

Initial try run:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad210ea0a85ce88ebe3efc957e602d4c0704824c

Comment 1

[Brendan Dahl [:bdahl]]

Attachment: current_mochitest_plain_failures.txt

Comment 2

[Brendan Dahl [:bdahl]]

Attachment: current_mochitest_plain_failures.txt

Latest try run: 64 failures  (down from 288)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=060a470e380c4a04aad0a8dcb5f9bd8141e07e83&selectedJob=132363396

Comment 3

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Track top level windows in headless mode for window ordering.

This better emulates a window manager and triggers the
required activated/deactivated events on the proper windows as
they are closed or hidden.

Review commit: https://reviewboard.mozilla.org/r/186594/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186594/

Comment 4

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Support synthesized mouse events.

Fixes:
 - test_pointerlock-api.html
 - test_group_mouseevents.html

Review commit: https://reviewboard.mozilla.org/r/186596/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186596/

Comment 5

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Add basic hard coded GTK theme in headless.

Many tests rely on the minimum size of widgets to
be set. Also, the precise size seems to matter for some
tests that have hard coded offsets.

Review commit: https://reviewboard.mozilla.org/r/186598/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186598/

Comment 6

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Add logging to headless widget.

Use similar logging to the various nsWindows to make
debugging easier.

Review commit: https://reviewboard.mozilla.org/r/186600/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186600/

Comment 7

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Default to san-serif for headless font.

Fixes problem with the initial computed value of font-family
being empty and not matching the later value in
test_value_storage.html.

Review commit: https://reviewboard.mozilla.org/r/186602/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186602/

Comment 8

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Support synthesized scroll events.

Fixes test_bug1151667.html

Review commit: https://reviewboard.mozilla.org/r/186604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186604/

Comment 9

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Support synthesized touch events.

Fixes test_group_pointerevents.html

Review commit: https://reviewboard.mozilla.org/r/186606/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186606/

Comment 10

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Add headless flag for mochitests.

Review commit: https://reviewboard.mozilla.org/r/186608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186608/

Comment 11

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Disable some mochitests in headless.

These tests will need more work and are low priority.

Review commit: https://reviewboard.mozilla.org/r/186610/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186610/

Comment 12

[Brendan Dahl [:bdahl]]

Attachment: Bug 1399956 - Add headless mochitests to taskcluster.

Review commit: https://reviewboard.mozilla.org/r/186612/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186612/

Comment 13

[Brendan Dahl [:bdahl]]

Try run (ignore non-linux runs for now, and also the 'h8' intermittent will be fixed in bug 1386130) : https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f9c0892515378d406c0444c558fa7a7e6bb2b6e&group_state=expanded&selectedJob=134964486

Comment 14

[Brendan Dahl [:bdahl]]

Comment on attachment 8915408 [details]
Bug 1399956 - Support synthesized mouse events.

https://reviewboard.mozilla.org/r/186596/#review191670

::: widget/headless/HeadlessWidget.h:16
(Diff revision 1)
>  #include "nsBaseWidget.h"
>  #include "CompositorWidget.h"
>  
> +// The various synthesized event values are hardcoded to avoid pulling
> +// in the platform specific widget code.
> +#if defined(MOZ_WIDGET_GTK)

I'm not a huge fan of this and am open to suggestions. I was hoping to define something like NATIVE_MOUSE_DOWN_MESSAGE and use that in JS (apz_test_native_event_utils.js) and C++ (here), but there doesn't seem to be a great way to do that when the value depends on the platform. I started adding attributes to nsIDOMWindowUtils.idl, but that's tied to the window which also didn't seem like a great solution either.

Comment 15

[Brendan Dahl [:bdahl]]

Comment on attachment 8915409 [details]
Bug 1399956 - Add basic hard coded GTK theme in headless.

https://reviewboard.mozilla.org/r/186598/#review191672

::: widget/headless/HeadlessThemeGTK.cpp:1
(Diff revision 1)
> +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-

I went back and forth on this. We could make a single headless theme across platforms, but then we don't closely match each platform. If the hardcoded approach is okay with you I'll write a xpcshell test to make sure headless and gdk stay in sync.

FWIW: Since we link gdk, I was hoping I could maybe use the gdk theme, but it requires a display to be setup (which we obviously don't want).

Comment 16

[Code Review Bot [:reviewbot]]

Comment on attachment 8915409 [details]
Bug 1399956 - Add basic hard coded GTK theme in headless.

https://reviewboard.mozilla.org/r/186598/#review191674


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: widget/headless/HeadlessThemeGTK.cpp:16
(Diff revision 1)
> +namespace mozilla {
> +namespace widget {
> +
> +NS_IMPL_ISUPPORTS_INHERITED(HeadlessThemeGTK, nsNativeTheme, nsITheme)
> +
> +HeadlessThemeGTK::HeadlessThemeGTK()

Warning: Use '= default' to define a trivial default constructor [clang-tidy: modernize-use-equals-default]

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8915414 [details]
Bug 1399956 - Add headless flag for mochitests.

https://reviewboard.mozilla.org/r/186608/#review191752

these changes look great.

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8915416 [details]
Bug 1399956 - Add headless mochitests to taskcluster.

https://reviewboard.mozilla.org/r/186612/#review191754

::: taskcluster/ci/test/tests.yml:823
(Diff revision 1)
>          by-test-platform:
>              linux64-qr/.*: 1
>              windows10-64-asan.*: 3
>              default: default
>  
> +mochitest-headless:

should this be mochitest-plain-headless ?  I ask because possibly mochitest-chrome could benefit.

Comment 19

[Brendan Dahl [:bdahl]]

Comment on attachment 8915408 [details]
Bug 1399956 - Support synthesized mouse events.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186596/diff/1-2/

Comment 20

[Brendan Dahl [:bdahl]]

Comment on attachment 8915409 [details]
Bug 1399956 - Add basic hard coded GTK theme in headless.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186598/diff/1-2/

Comment 21

[Brendan Dahl [:bdahl]]

Comment on attachment 8915410 [details]
Bug 1399956 - Add logging to headless widget.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186600/diff/1-2/

Comment 22

[Brendan Dahl [:bdahl]]

Comment on attachment 8915411 [details]
Bug 1399956 - Default to san-serif for headless font.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186602/diff/1-2/

Comment 23

[Brendan Dahl [:bdahl]]

Comment on attachment 8915412 [details]
Bug 1399956 - Support synthesized scroll events.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186604/diff/1-2/

Comment 24

[Brendan Dahl [:bdahl]]

Comment on attachment 8915413 [details]
Bug 1399956 - Support synthesized touch events.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186606/diff/1-2/

Comment 25

[Brendan Dahl [:bdahl]]

Comment on attachment 8915414 [details]
Bug 1399956 - Add headless flag for mochitests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186608/diff/1-2/

Comment 26

[Brendan Dahl [:bdahl]]

Comment on attachment 8915415 [details]
Bug 1399956 - Disable some mochitests in headless.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186610/diff/1-2/

Comment 27

[Brendan Dahl [:bdahl]]

Comment on attachment 8915416 [details]
Bug 1399956 - Add headless mochitests to taskcluster.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186612/diff/1-2/

Comment 28

[Brendan Dahl [:bdahl]]

(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #18)
> Comment on attachment 8915416 [details]
> Bug 1399956 - Add headless mochitests to taskcluster.
> 
> https://reviewboard.mozilla.org/r/186612/#review191754
> 
> ::: taskcluster/ci/test/tests.yml:823
> (Diff revision 1)
> >          by-test-platform:
> >              linux64-qr/.*: 1
> >              windows10-64-asan.*: 3
> >              default: default
> >  
> > +mochitest-headless:
> 
> should this be mochitest-plain-headless ?  I ask because possibly
> mochitest-chrome could benefit.

I was following the previous convention of mochitest plain being just "mochitest" in the yml file. If I add mochitest chrome it will be mochitest-chrome-headless.

Any pref?

Comment 29

[Joel Maher ( :jmaher ) (UTC -8)]

the more descriptive we can be the better, for sure mochitest-chrome-headless, I would prefer mochitest-plain-headless, and best judgement is <testname>-headless.

Comment 30

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915415 [details]
Bug 1399956 - Disable some mochitests in headless.

https://reviewboard.mozilla.org/r/186610/#review193740

Comment 31

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915407 [details]
Bug 1399956 - Track top level windows in headless mode for window ordering.

https://reviewboard.mozilla.org/r/186594/#review193744

::: widget/headless/HeadlessWidget.h:117
(Diff revision 1)
>    // to dispatch (de)activation events properly.
>    void RaiseWindow();
> -  static HeadlessWidget* sActiveWindow;
> +  // The top level widgets are tracked for window ordering. They are
> +  // stored in order of activation where the last element is always the
> +  // currently active widget.
> +  static StaticAutoPtr<nsTArray<HeadlessWidget*>> sActiveWindows;

Let's use a static UniquePtr<> instead.

Comment 32

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #31)
> Comment on attachment 8915407 [details]
> Bug 1399956 - Track top level windows in headless mode for window ordering.
> 
> https://reviewboard.mozilla.org/r/186594/#review193744
> 
> ::: widget/headless/HeadlessWidget.h:117
> (Diff revision 1)
> >    // to dispatch (de)activation events properly.
> >    void RaiseWindow();
> > -  static HeadlessWidget* sActiveWindow;
> > +  // The top level widgets are tracked for window ordering. They are
> > +  // stored in order of activation where the last element is always the
> > +  // currently active widget.
> > +  static StaticAutoPtr<nsTArray<HeadlessWidget*>> sActiveWindows;
> 
> Let's use a static UniquePtr<> instead.

Actually, I'm wrong keep it as StaticAutoPtr

Comment 33

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915408 [details]
Bug 1399956 - Support synthesized mouse events.

https://reviewboard.mozilla.org/r/186596/#review193776

Comment 34

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915409 [details]
Bug 1399956 - Add basic hard coded GTK theme in headless.

https://reviewboard.mozilla.org/r/186598/#review193780

Yuck.

Comment 35

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915410 [details]
Bug 1399956 - Add logging to headless widget.

https://reviewboard.mozilla.org/r/186600/#review193782

Comment 36

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915411 [details]
Bug 1399956 - Default to san-serif for headless font.

https://reviewboard.mozilla.org/r/186602/#review193784

Comment 37

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915412 [details]
Bug 1399956 - Support synthesized scroll events.

https://reviewboard.mozilla.org/r/186604/#review193788

Comment 38

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8915413 [details]
Bug 1399956 - Support synthesized touch events.

https://reviewboard.mozilla.org/r/186606/#review193790

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d809866fa62
Track top level windows in headless mode for window ordering. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bebff2a56e3
Support synthesized mouse events. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8fcf9b9152
Add basic hard coded GTK theme in headless. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9fb2b57d667
Add logging to headless widget. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/d41de10195a0
Default to san-serif for headless font. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3484a545375
Support synthesized scroll events. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/a031371ca87e
Support synthesized touch events. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/bae327f6d86c
Add headless flag for mochitests. r=jmaher
https://hg.mozilla.org/integration/mozilla-inbound/rev/9fe990f0398b
Disable some mochitests in headless. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/81d911b75946
Add headless mochitests to taskcluster. r=jmaher

Comment 40

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/0d809866fa62
https://hg.mozilla.org/mozilla-central/rev/4bebff2a56e3
https://hg.mozilla.org/mozilla-central/rev/ef8fcf9b9152
https://hg.mozilla.org/mozilla-central/rev/b9fb2b57d667
https://hg.mozilla.org/mozilla-central/rev/d41de10195a0
https://hg.mozilla.org/mozilla-central/rev/d3484a545375
https://hg.mozilla.org/mozilla-central/rev/a031371ca87e
https://hg.mozilla.org/mozilla-central/rev/bae327f6d86c
https://hg.mozilla.org/mozilla-central/rev/9fe990f0398b
https://hg.mozilla.org/mozilla-central/rev/81d911b75946