1406590 - -screenshot=path/to/name.png hangs the browser

Status: RESOLVED FIXED

(Firefox :: Headless, enhancement, P3)

Description

[Myk Melez [:myk] [@mykmelez]]

This works:

> /path/to/firefox -screenshot path/to/name.png https://developer.mozilla.com

But this hangs the browser process:

> /path/to/firefox -screenshot=path/to/name.png https://developer.mozilla.com

Both of them should do the same thing: save the screenshot to the specified path and exit the process.

Comment 1

[Brendan Dahl [:bdahl]]

And this also works (two dashes):

> /path/to/firefox --screenshot=path/to/name.png https://developer.mozilla.com

I'm guessing there's a bug in the JS argument parser, as the c++ seems to recognize that we should be in headless mode, but we don't enter the JS screenshot code.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: support single-dash arguments with values

This is actually an issue in nsCommandLine::Init, which splits an argument containing an equals sign if it starts with two dashes but not if it starts with a single dash.

Chrome, by comparison, handles arguments with both the single and double-dash forms of its "screenshot" argument:

> > /Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --headless --disable-gpu -screenshot=foo.png https://www.chromestatus.com/
> [1019/131005.978555:INFO:headless_shell.cc(468)] Written to file foo.png.
> > /Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --headless --disable-gpu --screenshot=foo.png https://www.chromestatus.com/
> [1019/131012.573300:INFO:headless_shell.cc(468)] Written to file foo.png.

Here's a patch that makes Firefox do the same.  I've also added tests of both variants for both the "screenshot" and the "window-size" arguments.

I've tested on Mac but not yet Windows nor Linux.  Awaiting the results of this tryserver run to request review:

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

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: support single-dash arguments with values

I fixed the Windows-specific test failures on tryserver by disabling the assertions for variants that nsCommandLine doesn't support on Windows.  This patch now passes its headless screenshot tests on all three platforms (confirmed locally on Windows).

Aside: jryans wondered on IRC why we don't support --arg in nsCommandLine on Windows.  I reviewed the summaries of all bugs (open and closed) that mention nsCommandLine or nsICommandLine, and I couldn't a relevant reference, so I filed bug 1410564 on that issue.

Here's one more tryserver push to confirm:

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0820a044d45f
support single-dash arguments with values; r=mossop

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/0820a044d45f