context capture screenshot: add optional focus top-level context step

[thiagowfx]

CDP's Page.captureScreenshot command1 is blocked until the active context gets focus.

Follow-up-of: #183
Spec: https://w3c.github.io/webdriver-bidi/#command-browsingContext-captureScreenshot

---

Preview | Diff

[thiagowfx]

This PR was created as per @sadym-chromium's suggestion given our chromium mapper implementation.

[whimboo]

I'm not that happy about that change given that it might cause side-effects users aren't aware of. For example web pages in background tabs will in most cases get throttled and as such will behave differently as when they are in foreground.

So I can see especially issues when the focus is not set back to the originally focused tab. I think that this PR needs a discussion.

[thiagowfx]

The alternative is to focus on whatever was focused previously, but we do not yet have the ability to determine that, correct? Maybe #171 can help with that.

[whimboo]

If some implementation specific steps have to be taken then such code could also revert the focus after the screenshot has been done. That's nothing the client has to be aware of. But again this is causing side-effects that you should be aware of.

[jgraham]

Arguably if Chrome can only take screenshots when the context has focus, it should return unsupported operation if the context doesn't have focus (this is explicitly allowed by the current spec), and we should have a browsingContext.focus() command that clients can call so that we're not unexpectedly changing focus in some browsers but not others.

[thiagowfx]

Will add an agenda item for this in the monthly meeting.

[thiagowfx]

Related: puppeteer/puppeteer#2254

[thiagowfx]

Once #453 is landed, do you think it would be feasible to change the WPT tests for capture screenshot to first call browsingContext.bringToFront before taking a screenshot?

If the answer is no: Then what if we added more tests instead? We could keep the current ones + add extra ones that perform the focus before taking screenshot.

[whimboo]

I think it would be fine to have screenshot tests that test foreground and background. Maybe the background tests only test the basic features then and all the available variants are done in a foreground tab.

[thiagowfx]

We will instead leverage #453 to explicitly focus. Otherwise the implementation will throw an error.

WPT tests should be updated accordingly once the command is added to the spec (as whimboo@ seems to agree above).

[whimboo]

Note that while our tests can easily call the extra activate command and test both scenarios, Webdriver BiDi users will wonder why they have to explicitly activate the tab before taking a screenshot. As such it would be good if Chrome could fix that issue and allow capturing screenshots in background tabs.

[thiagowfx]

I think it would be fine to have screenshot tests that test foreground and background.

For completeness: https://bugs.chromium.org/p/chromium/issues/detail?id=32667 to support background screenshots

[thiagowfx]

This issue was closed because of: (i) web-platform-tests/wpt#40965 and (ii) WG consensus that this is probably not a good idea.

From @jgraham via IRC:

"optional" anything is bad, and that given changing focus has clear observable side effects, it's not an exception to that.