WebDriver/RemoteProtocol/Meetings/2019/11/01

Agenda

• Status of Milestone 1 (alpha release of Puppeteer support) [henrik]
  • 44 Total; 43 Open (97.73%); 1 Resolved (2.27%); 0 Verified (0%);
  • Bugs have been triaged for Gutenberg and puppeteer-examples support
  • A couple of meta bugs still need real implementation bug for release tracking
  • Do we have to keep track of the Gutenberg and puppeteer-examples meta bugs, or close them?

• Security review update [ato]
  • Bare-bones review process documentation

• Inclusion of updates into the Firefox Headlines Newsletter [henrik]

• Update on Firefox support for Puppeteer? [maja]

• Do we have a story in how we want to do debug/trace logging? [henrik]
  • I would like to see a way to enable that for local debugging purposes
  • Would be kinda helpful for received events and observer notifications
  • Currently sometimes a command hangs due to an unresolved promise and it's not visible

Roster

Present

whimboo, mmucci, ato, maja_zf (for parts), AutomatedTester

Regrets

Minutes

Status of Milestone 1

whimboo

I’ve gone through all the bugs and compared them with the list of methods and events used by Gutenberg and Puppeteer

As a result I’ve updated all the whiteboard entries.

I’ve not gone into great detail on everything, but the 44 meta bugs we have should cover the API surface we need for Gutenberg.

Should we keep the puppeteer-gutenberg and puppeteer-examples support?

I would suggest we do

What about the MVP bug?

ato

Yes, it might be good to keep both the Gutenberg and examples meta bugs, especially if we cannot get all of the fixes into the alpha release

And we should get rid of the MVP bug

AutomatedTestter

I work off mmucci’s dashboards anyway, so I’m fine with it.

whimboo

OK, I’m fine with it too.

AutomatedTester

Question about the dashboard: there are currently only 15 bugs

whimboo

that is because we have half of them as meta bugs which aren't tracked in the dashboard

ato

It doesn't make sense to create implementation bugs right away

mmucci

If you want to introduce new work but you

ato

What matters about the dashboard is predicting how long work will take. Maybe we should look at what kind of work is involved in some important features and file implementation bugs. Otherwise this will work itself out.

Security review update

ato

The update is that I don't really have an update!

We have to wait for the write up from the security team

There will be a secondary code review in the code which starts up Firefox

whimboo

should we get some review from a firefox peer?

ato

each team is in responsibility of their own code

maja_zf

maybe someone from the devtools team could be chosen?

ato

sure, anyone with a fresh pari of eyes

but so far I haven't heard anything back for a week

[discussion about mach and how the Puppeteer tests are started]

Newsletter

whimboo

Should we try to include updates about the remote agent into the Firefox newsletter?

AutomatedTester

Yes?

whimboo

It gets released every second week I think.

If you have ideas about how we can introduce the remote agent to the newsletter, I would appreciate that.

ato

Yeah, I mean this would be great. We should communicate more clearly what we’re trying to achieve with the new remote protocol.

Update on Firefox support for Puppeteer?

maja_zf

https://github.com/andreastt/puppeteer/pull/1

I'm working on the code in Launcher.js, about 5% chrome specific, and doing some refactoring to be able to also use it for Firefox.

A user would set the PUPPETEER_PRODUCT environment variable to select Firefox.

Some more code for setting up the profile including preferences then gets run.

ato

The way how Chrome is configured is different?

With Firefox we write a lot of preferences to the profile.

maja_zf

Chrome just uses command-line flags.

Maybe in the future we can have a mozprofile Node.js library to allow users to configure the used profile.

ato

Would it make sense to have such a library because in the future we might want to use a WebDriver-way of configuring a profile?

maja_zf

The profile creation code is not exposed.

There’s a PUPPETEER_PRODUCT variable and we should be able to have some state so we can reference this throughout.

whimboo

I assume that the code paths for Juggler (puppeteer-firefox) are still kept in place?

maja_zf

I’m pretending the Juggler code doesn’t exist and that it will go away at some point.

The bulk of the work here has been to generalise Launcher.js so it can be used by many browsers.

They do some stuff for parsing the output of the process

I’ll add an explainer on how to use it so you can try it out.

I’m hoping to get lots of comments from you and maybe merge that into our fork, and re-vendor it into central so we can start running it in CI.

Once we’ve concluded our internal team discussion for what it should look like, we can submit a PR to Google.

ato

I think you should push your work to my fork as soon as possible and vendor it in.

I think we should have a policy that you don’t need reviews when you revendor the Puppeteer folder.

The vendor aspect is purely a formality.

There’s no reason it couldn’t be automatic when you push a change to the GitHub repo.

The important thing is that we have a way of testing our changes with Firefox before we submit PRs, so that we know a change works properly with Firefox.

There’s currently no upstream support for testing Firefox, so when we submit a new PR, that PR will be tested against Chrome-only.

The point with having the tests vendored into central is so that we can test it with Firefox.

maja_zf

Puppeteer do test Firefox on Travis with the Juggler stuff.

They call npm run unit with some environment variable that says ‘firefox’, and that points to Juggler.

With my PR I would change that they run Firefox with the new code.

Testing with Firefox with the up-to-date way of doing things will also be a result of this.

whimboo

What we should not try to do is replace it right now.

I think there are people out there who already use Juggler and we don’t have the feature set yet which would break them which Google maybe would not allow.

maja_zf

Yes, deleting Juggler is a separate step.

ato

We did talk about this during the work week, didn’t be?

I thought the conclusion we made there was that we would go ahead and delete Juggler as soon as possible from their repo, and the reason being that they’re not planning on releasing any new versions of it.

maja_zf

Yeah, the package just needs to stay the same.

ato

Yes, I mean, as long as the package doesn’t go away.

But I wonder, you were saying earlier that you were not going to delete Juggler but you were going to change the way they do testing? Isn’t that a bit inconsistent? Shouldn’t those to things be done in separate steps?

maja_zf

That’s a good point.

I could add an extra line so that their Juggler tests are still running.

The point is that it’ll be trivial to set that up.

Going back to what you were saying earlier about vendoring and reviews, I started this PR on your fork because I want the team’s feedback—at least about the direction of the work.

It’s not about creating a system of double reviews, but I do want to use your input to polish it up.

ato

Just let us know when you want us to look at it.

maja_zf

I have… you can go ahead and look at it any time.

There are a lot of things that I’m [leading?] out of this work, for example the options passed to the call to launch are very Chrome specific right now.

I’m working around that, so say someone passes the user-data-dir option that’s fine, I’ll use that as the Firefox profile directory.

For a more polished experience of cross-browser Puppeteer, I would want to find a better solution I would want to find a better solution for managing these different browser options.

And maybe for that kind of restructuring, a design document might make sense. Or not just that, but other things of that nature.

ato

If we ever get to a point in the future where the Chrome support in Puppeteer is backed by WebDriver via this new bi-di protocol, then I think it would be worthwhile for Puppeteer to consider using WebDriver capabilites negotiation.

There are a good reasons for why they might want to do this in terms of features.

For example it’s currently not possible to run Puppeteer in the cloud. You can’t have service provider providing you with a browser and have the Puppeteer script run on your computer, because Puppeteer is fundamentally limited in that way.

WebDriver would give you the ability to allocate a computer in the cloud somewhere, upgrade into bi-di or CDP, and off you go with a remote instance of Puppeteer.

Logging

whimboo

While investigating intermittent test failures, trying to debug hangs, etc. it’s sometimes hard to locate the source of the problem.

Mostly it’s some kind of promise that doesn’t get resolved and we hang for ever. This is an example.

The way log events and observer notification on the trace log in Marionette is pretty helpful.

I wonder if there’s a way we can a bit more detail for debugging purposes, for us specifically.

ato

There’s a couple of points I’d like to make in relation to this.

Yes it is good that we have detailed logs in Marionette, but I also think it’s very specific. I wonder if we can come up with an approach that is more generic.

For example for events we emit through EventEmitter.jsm, there’s a preference you can toggle that causes the event emitter to internally log the stuff that is emits.

I wonder if that’s a better approach for logging the things that come from observer notifications or events.

Maybe we should invest some time looking into if there exists systems for this already that we don’t know about, rather than add log entries every time we await a promise or something.

whimboo

For now it’s fine because I can add local dump() statements.

The EventEmitter case wouldn’t help so much here because it only covers cases we are the origin of the event.

The case I’m talking about is when we are waiting not for our own events, but other events such as Firefox-specific observer notifications.

ato

Don’t hold me to this… but I think there is something already built in to Gecko for logging system observer notifications.

I think DOM events are harder, so I don’t know if there’s anything built in to log those.

But presumably this is an already solved problem: you can listen for [arbitrary] DOM events and have them show up in your debugger.

If we could leverage existing tooling and support I would much prefer that, than putting in lots of log statements in our code.

Logging, continued

ato

I would like to come up with an alternative way to log what gets transmitted over the wire that isn’t Log.jsm.

For example for debugging it would be useful if you could replay client input steps.

whimboo

Related to this we also have the DEBUG=1 environmental variable in the Puppeteer client, and this has at least helped me over the last few days.

Especially when trying to reverse engineer what is happening on a protocol level.

Which method/event is called, and the full payload returned.

ato

Another thing we should try improve on from Marionette is that it currently logs everything to stdout, which means if you disable stdout from Firefox you also won’t get any Marionette logs.

For geckodriver we really want to turn off the stdout from Firefox because a lot of users are getting confused by output that is outside our control, with the consequence that they file bugs on this against us.

I think we want to move to a future where the remote agent has a new log type that is specific to the server component.

Perhaps we could even consider having a log type that is specific to each browsing context?

Then we should consider streaming those back as regular CDP log entries, for example through Log.entryAdded, instead of printing them to stdout.

So instead of printing to stdout within the Firefox executable, the client can be responsible for printing them when needed: whether that is still to stdout, to stderr, a file, or whatever.

In other words, shifting the relationship so it is the client that is in control of the logs.

whimboo

Sounds interesting.

Changelog

% git log --date=iso --pretty=format:'%ad%x09%H' -- remote/ | awk '$0 >= "2019-10-23" && $0 <= "2019-11-01"' | awk -F $'\t' '{print $2}' | xargs git show -s --format='%h%x09%an%x09%s'
9079bb1e0e4e    Henrik Skupin   Bug 1591006 - [remote] Re-arrange browser-chrome tests by domains as subfolders. r=remote-protocol-reviewers,maja_zf
33299328eecb    Maja Frydrychowicz      Bug 1591216 - Minor corrections to Remote Agent documentation; r=remote-protocol-reviewers,ato
a730333a7d17    Maja Frydrychowicz      Bug 1591216 - Sync vendored puppeteer to v2.0.0; r=remote-protocol-reviewers,ato
4a42c7fcaae2    Henrik Skupin   Bug 1591341 - [remote] Always close the client connection in add_task() for browser chrome tests. r=remote-protocol-reviewers,maja_zf
b6bac2e8c2e2    Cosmin Sabou    Bug 1590930 - Temporarily skip remote/test/browser/browser_runtime_executionContext.js on windows. r=remote-protocol-reviewers,whimboo
ddd2c0453e92    Henrik Skupin   Bug 1589625 - [remote] Improve setup and teardown logic for browser chrome tests. r=remote-protocol-reviewers,ato
1066cb54296e    Henrik Skupin   Bug 1589625 - [remote] Start and stop the CDP server outside of the browser chrome tests. r=remote-protocol-reviewers,ato
19d17b52708e    Henrik Skupin   Bug 1589625 - [remote] Improve handling of tabs when closing a chrome window. r=remote-protocol-reviewers,ato
c89893d8544f    Henrik Skupin   Bug 1589625 - [remote] Add documentation for enabling logging of emitted events. r=remote-protocol-reviewers,ato,maja_zf

Work

Milestones

Development status of Puppeteer alpha

Puppeteer alpha dashboard

Bugzilla queries

All project work currently in development

Available MVP work

Completed MVP work

Bug overviews

Gutenberg dependency tree

Puppeteer examples dependency tree

Complete Puppeteer dependency tree

All ze boogs

PTO (🍂)

• ato away Thursday 31st October