Applies also to 73.0.3642.0 canary

Current behavior of cookies with DevTools is very strange. It does not send cookies, but if the server returns Set-Cookie header, then that cookie is taken into use in subsequent requests, also e.g. XHR requests done by the application. 

Could you please provide more information -- having a site to reproduce that or at least screenshots of the requests would probably help.
Note that the expected behavior with cookies for cross-origin requests now is not to display raw cookies in DevTools (see  issue 793692  for details), but this does not affect whether cookies are actually sent.

Does this issue reproduce if devtools is closed?

Lax cookies not being sent is site_for_cookies being wrong, I think.

Doesn't reproduce with DevTools closed. I'll create a NodeJS/Express example in a couple of hours.

Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Attached an example. Run 'npm install && npm run serve' and navigate to 'http://localhost:3000'.

Deleted: chromium-915637.zip 8.3 KB

chromium-915637.zip 8.3 KB Download

I see, this goes through DevToolsUIBindings::LoadNetworkResource, which just does:

  auto resource_request = std::make_unique<network::ResourceRequest>();
  resource_request->url = gurl;
  resource_request->headers.AddHeadersFromString(headers);

so this is indeed expected, but I am not sure what's the proper way of pulling
out all the relevant stuff in devtools; and of course getting it right for samesite = strict is further difficulty (perhaps that should be sent only when the original would have sent it?)

In which way is this expected? The behaviour in Chrome 70 was much more predictable: when DevTools performs a call to retrieve sourcemaps, all relevant cookies were sent as if it was a regular resource.

My example will behave as predicted when you do one of the following:
- Close DevTools
- Remove the sameSite flag in serve.js (and test with or without DevTools)
- Use Chrome 70 (with or without DevTools opened)

Expected in the sense that I can see why the code is behaving like that, not in the sense that any user would want that to happen. My apologies for the poor choice of words.

Tried testing the issue on reported chrome version #71.0.3578.98 using Mac OS 10.14.1 by following below steps.

Steps:
=====
1.Launched chrome.
2.Ran "npm install && npm run serve".
3.Observed errors in terminal (attached screenshot of errors).
4.Started a local server.
5.Navigated to "localhost:8000" and opened html page.
6.Refreshed the page unable to find any error under Devtool->Network tab.

Attached screenshot and screencast for reference.
@reporter: Could you please review attached screencast and let us know if anything is being missed here. If possible could you please provide a screencast or screencast of the issue so that it would be really helpful for further triaging of the issue.
Thanks.!

Deleted: 915637.mp4 4.1 MB

	915637.mp4 4.1 MB View Download

Deleted: error.png 241 KB

	error.png 241 KB View Download

Please extract my zip-file and run the command in the "chromium-915637" folder you extracted. So:

1. unzip chromium-915637.zip -d ~
2. cd ~/chromium-915637
3. npm install && npm run serve
  (web server at port 3000 is running, you'll see the nodejs console log)
4. open http://localhost:3000 with DevTools opened
  (check node console log with instructions on html page)

If this doesn't help I'll create a screen cast later.

Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Tried testing the issue on reported chrome version #71.0.3578.98 using Mac 10.14.1 by following below steps.

Steps:
=====
1.Launched chrome and extracted the zip file "chromium-915637.zip".
2.Opened the terminal and changed the directory to "cd ~/chromium-915637".
3.Ran "npm install && npm run serve".
4.Opened "localhost:3000" with Devtools open in chrome.
5.Observed in nodejs console log only one "created session" is created with "style.css.map".
6.Refreshed the page and observed that on every refresh a new "created session" event gets created with "style.css.map". 

Attached screencast for reference.
@reporter: Could you please review attached screencast and let us know if anything is being missed here.
Thanks.!

Deleted: 915637_New.mp4 2.2 MB

	915637_New.mp4 2.2 MB View Download

You forgot to clear cookies in advance. Please have a look at attached screencast.

Deleted: Kapture 2018-12-21 at 18.18.02.mp4 1.2 MB

	Kapture 2018-12-21 at 18.18.02.mp4 1.2 MB View Download

Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I'm seeing this not just on Mac but also on Windows and also with SameSite=Strict. Disabling source maps is a workaround, otherwise the app is broken with DevTools since the session cookie is not sent and the server thus sends a new one...

Able to reproduce the issue on reported chrome version #71.0.3578.98 and latest chrome #73.0.3649.0 using Mac OS 10.14.1, Ubuntu 17.10 and Windows 10 by following steps as per comment#16 and comment#19. The following is the bisect information.

Bisect Information:
===================
Good Build: 71.0.3561.0
Bad Build: 71.0.3562.0

You are probably looking for a change made after 593894 (known good), but no later than 593895 (first known bad).
CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/fb2871b027d0e109079134df723943f1999e8f63..670a583f75224581a26338b3c4f37b936c27d57c
Reviewed-on: https://chromium-review.googlesource.com/1238464

@Mario Sanchez Prada: Please help us in reassigning the issue if it it is not related to your change.
Adding ReleaseBlock-Stable as this seems to be a recent regression. Please feel free to remove if not applicable
Thanks.!

CC'ing Dmitry Gozman (reviewer of the suspected CL) as the author Mario Sanchez Prada is not available until Jan7th.

Friendly ping to look into this issue and to provide further update on this issue as it has been marked as a stable blocker.

Thanks!

Gentle ping! Could you please provide any update on this issue as it has been marked as a stable blocker.

Thank You!

Hi! Sorry for the delay replying to this, just came back from holidays today and found it in my Inbox.

I'm not an expert on the network service, I think Matt Menke (@mmenke) would probably know better, but in any case that patch should not make a difference if the network service feature was enabled, so I assume that it was not enabled when running the tests mentioned in comment#16 and comment#19? (I didn't find any mention of that in the report).

Either way, this should work since the whole patch was based on the fact that, AFAIU, SimpleURLLoader should work fine regardless of the network service being enabled or not, and if that's not the case it might mean a bug in there (again, @mmenke probably knows more).

Last, I remember a comment already mentioning that it was confusing that the "legacy" path that my patch removed was still there (see [1]), maybe this was why it was there after all. @caseq was the author of the patch that migrated shell_devbinding_tools.cc to the network service [2], so maybe he might know more about this too.

Sorry for the breakage and also in case this comment doesn't provide the right answers yet, but at least I hope the extra info helps clarify the issue (and/or report a new bug if there's one).

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=840410#c6 mentioning that 
[2] https://chromium-review.googlesource.com/c/chromium/src/+/973811

SimpleURLLoader works fine either way, yes, but stuff that needs to work with cookies w/SameSite needs to fill in a bunch of extra stuff in ResourceRequest.

As for the network service feature: do you mean recording network requests? This is enabled by default and I didn't disable it (https://developers.google.com/web/tools/chrome-devtools/network-performance/reference). So yes, in that case. Maybe you can clarify how to check this setting if I didn't understand correctly.

The consumer is responsible for filling out fields of the request required to actually make the request - the network service cannot deduce whether a request is samesite or not without the consumer telling it what site actually sent the request.

 Issue 915739  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0a9c2ed1e36b2d3cebe2845806a355ea470ec50

commit a0a9c2ed1e36b2d3cebe2845806a355ea470ec50
Author: Andrey Kosyakov <caseq@chromium.org>
Date: Tue Jan 08 03:50:59 2019

DevTools: fix SameSite cookies not sent after migration to SimpleURLLoader

Bug:  915637 , 915739 
Change-Id: I15eecfd7d3e8c992d0907b6499c57764ca538f09
Reviewed-on: https://chromium-review.googlesource.com/c/1399515
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620611}
[modify] https://crrev.com/a0a9c2ed1e36b2d3cebe2845806a355ea470ec50/chrome/browser/devtools/devtools_ui_bindings.cc
[modify] https://crrev.com/a0a9c2ed1e36b2d3cebe2845806a355ea470ec50/content/shell/browser/shell_devtools_bindings.cc
[modify] https://crrev.com/a0a9c2ed1e36b2d3cebe2845806a355ea470ec50/third_party/blink/renderer/devtools/front_end/Tests.js

How safe is this merge? Have you tested and verified this in canary?

CL listed at #33 is not in canary yet, pls update bug with canary result tomorrow and comment on merge safety. Thank you.

This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The NextAction date has arrived: 2019-01-09

Johan, thanks a lot for your repro case, this really helped to isolate the problem. This script works as intended for me now, but just to double check, would someone of the original reporters be willing to verify this has been fixed in Canary?

Able to reproduce the issue on reported chrome version# 71.0.3578.98 using Ubuntu 17.10 by following steps as per  comment#16 and comment#19.

Verified the fix on Mac 10.14.1, Ubuntu 17.10 and Windows 10 as per  comment#16 and comment#19 on latest chrome version #73.0.3666.0.
Attaching screencast for reference.
Observed that in node-js console log only one "created session" is created.
Hence, the fix is working as expected.
Adding the verified labels.

Thanks...!!

Deleted: 915637_M73.mp4 2.0 MB

	915637_M73.mp4 2.0 MB View Download

Tested on latest canary (73.0.3667.0) on MacOS. Fix is working as expected.

Swarnasree & Johan, thanks a lot!
Re merge safety -- I'm rather confident that this is safe to merge, considering it's a one-liner that sets a field in a parameter passed to a well-tested method and is only utilized when DevTools are fetching source maps and source code.

Approving merge to M72 branch 3626 based on comments #40, #41 & #42. Please merge ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cacdf46e76e875a484d8dbd6b3525f9fdd6f017

commit 0cacdf46e76e875a484d8dbd6b3525f9fdd6f017
Author: Andrey Kosyakov <caseq@chromium.org>
Date: Thu Jan 10 18:57:54 2019

DevTools: fix SameSite cookies not sent after migration to SimpleURLLoader

Bug:  915637 , 915739 
Change-Id: I15eecfd7d3e8c992d0907b6499c57764ca538f09
Reviewed-on: https://chromium-review.googlesource.com/c/1399515
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620611}(cherry picked from commit a0a9c2ed1e36b2d3cebe2845806a355ea470ec50)
Reviewed-on: https://chromium-review.googlesource.com/c/1405538
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#633}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/0cacdf46e76e875a484d8dbd6b3525f9fdd6f017/chrome/browser/devtools/devtools_ui_bindings.cc
[modify] https://crrev.com/0cacdf46e76e875a484d8dbd6b3525f9fdd6f017/content/shell/browser/shell_devtools_bindings.cc
[modify] https://crrev.com/0cacdf46e76e875a484d8dbd6b3525f9fdd6f017/third_party/blink/renderer/devtools/front_end/Tests.js

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/0cacdf46e76e875a484d8dbd6b3525f9fdd6f017

Commit: 0cacdf46e76e875a484d8dbd6b3525f9fdd6f017
Author: caseq@chromium.org
Commiter: caseq@chromium.org
Date: 2019-01-10 18:57:54 +0000 UTC

DevTools: fix SameSite cookies not sent after migration to SimpleURLLoader

Bug:  915637 , 915739 
Change-Id: I15eecfd7d3e8c992d0907b6499c57764ca538f09
Reviewed-on: https://chromium-review.googlesource.com/c/1399515
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620611}(cherry picked from commit a0a9c2ed1e36b2d3cebe2845806a355ea470ec50)
Reviewed-on: https://chromium-review.googlesource.com/c/1405538
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#633}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}