1590098 - Implement Network.getCookies

Status: RESOLVED FIXED

(Remote Protocol :: CDP, task, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

For Puppeteer support it is necessary to get the Network.getCookies domain implemented. It's specification can be found at:

https://chromedevtools.github.io/devtools-protocol/tot/Network#method-getCookies

The implementation can be similar to what we currently have in Marionette:
https://searchfox.org/mozilla-central/rev/d7537a9cd2974efa45141d974e5fc7c727f1a78f/testing/marionette/cookie.js#229

getCookiesFromHost() returns an enumerator, and entries would have to be mapped to the Cookie type as defined by CDP.

Note that the list of cookies needs to be filtered by the path property, given that only cookies from the current URL and not the host have to be returned.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Oh, and the necessary code has to be added to the Network class in:
https://searchfox.org/mozilla-central/source/remote/domains/parent/Network.jsm

And also some tests would be necessary. All of them should be added to a file called browser_getCookies.js in:
https://searchfox.org/mozilla-central/source/remote/test/browser/network

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Thomas. You showed interest in this bug about a week ago, and I'm wondering if that is still the case. If yes, please let me know how I can help. Thanks a lot.

Comment 3

[Thomas Kosmas]

Yes I am still interested, sorry I hadn't enough free time last week. I will be on irc later.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thomas, please note that whenever someone else comes up and wants to help with this bug I can no longer block this specific one for you, unless you will be able to have a patch ready soon. Thanks.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We made a bit of progress with Thomas today on IRC, and one thing which remains to be answered is how to run a specific Puppeteer test. So far I was only able to accomplish that by commenting out any test in this following list which is not needed:

https://searchfox.org/mozilla-central/rev/0678172d5b5c681061b904c776b668489e3355b0/remote/test/puppeteer/test/puppeteer.spec.js#131-176

Note that whenever you do a hg update you will have to comment it out again. Also don't forget to revert it before committing the changes.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Also for the implementation have a look at:
https://github.com/puppeteer/juggler/blob/master/testing/juggler/BrowserContextManager.js#L142-L171

This was some code from Google for a PoC that could be re-used here.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thomas, it's been 10 days already since we spoke last time. Are you still working on this bug? Otherwise one of us might pick it up soon given that it somewhat causes a lot of test failures in the puppeteer unit tests. Thanks.

Comment 8

[Thomas Kosmas]

Hey, sorry i dont have enough free time to work on this one..You are free to pick it up.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks for letting us know!

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

While waiting on reviews for other important APIs I'm going to pick-up this one now given that it would make us more shiny in the Puppeteer unit tests.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1590098 - [remote] Implement basic support for Network.getCookies. r=#remote!

Comment 13

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #5)

We made a bit of progress with Thomas today on IRC, and one thing which remains to be answered is how to run a specific Puppeteer test. So far I was only able to accomplish that by commenting out any test in this following list which is not needed:

https://searchfox.org/mozilla-central/rev/0678172d5b5c681061b904c776b668489e3355b0/remote/test/puppeteer/test/puppeteer.spec.js#131-176

Note that whenever you do a hg update you will have to comment it out again. Also don't forget to revert it before committing the changes.

I just noticed this question. You can run a specific Puppeteer unit test by replacing it function with with fit. See https://github.com/puppeteer/puppeteer/blob/master/CONTRIBUTING.md#running--writing-tests

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Great! That is good information. Running it individually let me trigger some failures. I will investigate.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f7ccbf95e92
[remote] Implement basic support for Network.getCookies. r=remote-protocol-reviewers,ato,maja_zf

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5f7ccbf95e92