Run plain mochitests in Headless Mode on Linux

RESOLVED FIXED in Firefox 58

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bdahl, Assigned: bdahl)

Tracking

unspecified
Firefox 58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(11 attachments, 1 obsolete attachment)

9.64 KB, text/plain
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jmaher
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jmaher
: review+
Details
Priority: -- → P1
Depends on: 1386130
See Also: → 1405867
See Also: → 1405868
See Also: → 1405869
See Also: → 1405870
See Also: → 1405871
See Also: → 1405872
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 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 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 on attachment 8915414 [details]
Bug 1399956 - Add headless flag for mochitests.

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

these changes look great.
Attachment #8915414 - Flags: review?(jmaher) → review+
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.
Attachment #8915416 - Flags: review?(jmaher) → review+
(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?
Flags: needinfo?(jmaher)
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.
Flags: needinfo?(jmaher)
Comment on attachment 8915415 [details]
Bug 1399956 - Disable some mochitests in headless.

https://reviewboard.mozilla.org/r/186610/#review193740
Attachment #8915415 - Flags: review?(jmuizelaar) → review+
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.
Attachment #8915407 - Flags: review?(jmuizelaar) → review+
(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 on attachment 8915408 [details]
Bug 1399956 - Support synthesized mouse events.

https://reviewboard.mozilla.org/r/186596/#review193776
Attachment #8915408 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8915409 [details]
Bug 1399956 - Add basic hard coded GTK theme in headless.

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

Yuck.
Attachment #8915409 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8915410 [details]
Bug 1399956 - Add logging to headless widget.

https://reviewboard.mozilla.org/r/186600/#review193782
Attachment #8915410 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8915411 [details]
Bug 1399956 - Default to san-serif for headless font.

https://reviewboard.mozilla.org/r/186602/#review193784
Attachment #8915411 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8915412 [details]
Bug 1399956 - Support synthesized scroll events.

https://reviewboard.mozilla.org/r/186604/#review193788
Attachment #8915412 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8915413 [details]
Bug 1399956 - Support synthesized touch events.

https://reviewboard.mozilla.org/r/186606/#review193790
Attachment #8915413 - Flags: review?(jmuizelaar) → review+
Summary: Run plain mochitests in Headless Mode → Run plain mochitests in Headless Mode on Linux
See Also: → 1408220
Pushed by bdahl@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.