Headless: Use persistent cookie storage [chromium/src : master]
Joel Einbinder (Gerrit)
ptal. I don't fully understand all the code, but it works. Maybe someone more knowledgeable can clean it up.
To view, visit change 719612. To unsubscribe, or for help writing mail filters, visit settings.
Andrey Lushnikov (Gerrit)
Context: there's a pptr issue about cookies not being saved https://github.com/GoogleChrome/puppeteer/issues/921
David, do you think this patch is reasonable? If yes, could you please recommend where to put a test for this?
David Vallet (Gerrit)
+Sami in case I missed something.
Thanks for implementing this! I've seen it being mentioned before as a feature request, but I can't find the bug# right now.
I'd probably add the tests to https://cs.chromium.org/chromium/src/headless/lib/headless_browser_browsertest.cc?rcl=6c0a0529076feaec9f5415f89c01ae5dab08989e&l=454
1 comment:
File headless/lib/browser/headless_url_request_context_getter.cc:
content::CookieStoreConfig cookie_config(
headless_browser_context_->GetPath().Append(
FILE_PATH_LITERAL("Cookies"))If no --user-data-dir is set, this defaults to EXE_DIR (see https://cs.chromium.org/chromium/src/headless/lib/browser/headless_browser_context_impl.cc?rcl=60dbde98da0052691c263cf66859eb14514aae28&l=206)
which could be problematic.I think for now it would be better to enable it only if --user-data-dir is set and perhaps add a TODO to enable it by default once saving/restoring sessions is implemented (https://crbug.com/617931)
To view, visit change 719612. To unsubscribe, or for help writing mail filters, visit settings.
Sami Kyöstilä (Gerrit)
Thanks for the fix!
1 comment:
content::CookieStoreConfig cookie_config(
headless_browser_context_->GetPath().Append(
FILE_PATH_LITERAL("Cookies"))
If no --user-data-dir is set, this defaults to EXE_DIR (see https://cs.chromium. […]
Good points. We already hit a problem where the crash reporter was trying to write into the app folder on Mac and failed.
Also let's avoid doing this if we're in incognito mode:
To view, visit change 719612. To unsubscribe, or for help writing mail filters, visit settings.
Joel Einbinder (Gerrit)
Joel Einbinder uploaded patch set #2 to this change.
Headless: Use persistent cookie storage
Bug: 775261
Change-Id: Ia0af1a184a5780c214be7607fdbd40a5a914d295
---
M headless/lib/browser/headless_url_request_context_getter.cc
1 file changed, 19 insertions(+), 0 deletions(-)
To view, visit change 719612. To unsubscribe, or for help writing mail filters, visit settings.
Joel Einbinder (Gerrit)
Still working on the test.
1 comment:
net::URLRequestContextBuilder builder;
// Don't store cookies in incognito mGood points. […]
Done.
To view, visit change 719612. To unsubscribe, or for help writing mail filters, visit settings.
Joel Einbinder (Gerrit)
I am having trouble figuring out how to add a test for this. I added one to Puppeteer. https://github.com/GoogleChrome/puppeteer/pull/1055
David Vallet (Gerrit)
Patch set 4:Code-Review +1
David Vallet (Gerrit)
Thanks Joel, I created https://crbug.com/775375 to track adding tests for this.
Joel Einbinder (Gerrit)
Patch set 4:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot merged this change.
Headless: Use persistent cookie storage
Bug: 775261
Change-Id: Ia0af1a184a5780c214be7607fdbd40a5a914d295
Reviewed-on: https://chromium-review.googlesource.com/719612
Reviewed-by: David Vallet <dva...@chromium.org>
Commit-Queue: Joel Einbinder <einb...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509347}
---
M headless/lib/browser/headless_url_request_context_getter.cc
1 file changed, 27 insertions(+), 0 deletions(-)