pbommana@, could you pls try to repro and bisect if possible?

+anthonycase@& cchurch@, are you able to repro this bug on latest M69 Beta #69.0.3497.57 using CloudPrint?

I can't reproduce this issue. This may involve the AccountConsistencyVariations experiment.

+ ewald@, droger@ & msarda@ 

I'm able to reproduce this on the latest Beta. Basically, it seems like the Print dialogue doesn't register the fact that the user is signed in. When you're signed out and click the "Sign in" link, it opens the page to let the user sign in. But then once you're signed in, clicking the link does nothing (because you're already signed in).

I'm not sure if this is a bug with sign in/Dice code or with the print/cloud print code. Given that Dice has been on-by-default for a while (and on via Finch for even longer), I'd be surprised if Dice just broke this. Regardless: Mihai, David, could you take a quick look to see whether this looks related to Dice, or if it looks like a bug with the cloud print service? Note that cloud print relies on content area sign in state (i.e. pre-Dice it would just get whatever accounts were in the cookies for Cloud Print).

Also, does anyone know who a good POC would be on the cloud print team that we could cc onto this bug?

Woops, just realized that the bug is already assigned to thestig@, who is the right cloud print POC :) following up with him offline.

I can reproduce this on Beta and Canary now. My repro steps:
1) Start with a new profile.
2) Sign in to Chrome at the browser level.
3) Print a webpage.
4) Change destination.

The destinations drop down asks the user to sign in, but the user should already be signed in.

- The "Enable new Print Preview UI" flag doesn't make a difference here.
- Chrome 68 seems to work fine.

I tried bisecting and I suspect r570877 is the cause. I tried bisecting a second time to verify, but I'm out of time for now. I can try picking this up later, or someone else can bisect this small revision range: [570872, 570890].

+cduvall FYI.

I'm having trouble reproducing this, the steps I'm doing are:
1) Start with new profile
2) Sign in to Chrome
3) Press 3 dot menu > Print
4) Change destination

It correctly shows my profile next to "Showing destinations for" in all the versions I've tried (Beta, Canary, r570872, r570890).

Also having trouble reproducing. I tried 570872 and 570890. On Linux, it seems like signing into Chrome doesn't work (so "Sign in" shows up in the dialog), but I'm not signed in if I navigate to chrome://settings either, so I think the sign in to Chrome page is just not working. Signing in on google.com/cloudprint or via the "Sign in" link in the destinations dialog seems to work on both revisions.

I've also tried Beta and both revisions on Windows, and have not been able to reproduce.

Also, is the bug that (1) the "Sign in" link doesn't work properly (i.e. it navigates to the sign in page but then the user account isn't showing up in the dialog after signing in), (2) that the link doesn't do anything at all (user clicks it and the sign in page does not appear), or (3) that if the user is signed in to Chrome it isn't being correctly recognized in the Print Preview dialog? The original report sounds like (2), but the repro steps sound like the issue is (3).

Feel free to ping me offline if you can't reproduce it and I can try to walk you through it.

Melody has a short video of the issue here. (Googler only link) https://drive.google.com/file/d/0B-cTxjSbtzeHMTZQSEN6Y3Y5RUlmX3FOSDRZZWxBRGc1Wl80/view

The sign in link opens a new tab to try to sign in, and the new tab immediately closes to bounce the user back to the tab with Print Preview open. In a "good" build, the sign in link does not appear at all, because the user is already signed in.

For developer builds, one I believe they need API keys. https://groups.google.com/a/chromium.org/d/topic/chromium-dev/XnS6bSfqaK4/discussion

I tried reproing both on trunk and M69 (local build), and I could not.

Here are the steps I tried:
- create a new profile (no signin)
- open the print dialog
- click on "Change", click signin
- signin in the other tab
- the signin tab closes and the print tab now has my account, and no signin button

You need to create a new profile, sign in to Chrome, and then print. On M68, when you print and change destinations, Print Preview does not prompt you to sign in, because you are already signed in.

Tried to reproduce the issue on Windows 10 on the latest Beta 69.0.3497.57 and unable to reproduce the issue by following the below steps.

1. Launched Chrome and signed into Chrome.
2. navigated to a webpage and clicked on Wrench menu -> Print.
3. Clicked on 'Change' button on the print preview page and selected 'Save to Google Drive'.
4. Could observe that the logged in profile is shown below the 'Save to Google Drive' destination and also next to "Showing destinations for" drop down.
Attached is the screen cast for reference.

Note: Unable to confirm the behavior using Google Cloud Print as this setup is not available at TE-end.

Thanks..

Deleted: 878051.mp4 1.9 MB

	878051.mp4 1.9 MB View Download

I can still fairly consistently repro the bug, but David cannot. David suggested looking at the network traffic:

Right click the left pane in Print Preview -> Inspect -> Devtools Network tab

Then press the "Change..." button to trigger the network requests to the Cloud Print server. Now one can see the tab-specific network traffic in Devtools.

There are some HTTP OPTIONS requests that gets 200 back, and HTTP GET requests that fail with 403. For the failing GET requests, there are no cookies being sent in the request header. M68, which does not have this bug, does send cookies in the request header.

The sender for the network requests is the print preview dialog, which has its own WebContents. It's running JS code and the requests show chrome://print as the origin.

For context, w.r.t. Cloud Print's JS code making network requests + Network Service, we also have bug 829412,  bug 829218 , and bug 829414. I tried turning network service on/off in chrome://flags, but that didn't make a difference.

As pointed by thestig, this seems to be a problem related to cookies missing on a javascript request coming from the webui.

r570877 which was pointed earlier changes how Chrome allows or blocks cookies on individual request, so that would be one more hint towards this CL.

I still can't repro unfortunately.

In the meantime, if someone can repro can you:
- send a screenshot of chrome:signin-internals
- check that you are logged in in the content area (i.e. gmail).
- check that you don't have weird cookie settings in chrome://settings/content/cookies

I am running the GCP version
cloudprint_ps_20180731_003046-RC06@209785594 pointing to production,
on chrome Version 69.0.3497.57 and I'm running incognito mode just to
make sure I'm completely signed out of chrome. Navigating to a URL (I
used starwars.com) and hitting <CTRL>+P, the option to save to drive
didn't exist in the printer list until after I logged in. From there I
was able to save the URL to Drive. So nothing out of the ordinary
here.

Just to make sure, I ran the same again but this time against Chrome
V68 - and had no issues there either. So cannot reproduce on my end.

I can still repro this on 70.0.3534.0 - here's a screenshot of signin-internals.

Deleted: signin-internals.JPG 292 KB

	signin-internals.JPG 292 KB View Download

Deleted: cookiesettings.JPG 81.9 KB

	cookiesettings.JPG 81.9 KB View Download

Does it still repro if you disable "Block third party cookies"?

Ding. "Block third party cookies" is the bit that triggers this bug. I had that set when my signed in and my settings got synced to the local profile. This definitely involves r570877 and cduvall@ can repro, so handing this over.

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

commit 150c7353da386029eac1513f22257a3717ebf991
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Aug 29 00:02:34 2018

Revert "Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic"

This reverts commit 8420d58c08a2b1dd5a0a414e44284491a5518331.

Reason for revert: This breaks the sign-in on the print dialog when third party
cookies are blocked:  http://crbug.com/878051 .
This is because the network service does not have the custom logic to whitelist
chrome:// or chrome-extensions:// schemes, which used to be here:
https://cs.chromium.org/chromium/src/components/content_settings/core/browser/cookie_settings.cc?l=98&rcl=5edbbd319ff4010b66e07ec404a3188716a9cda2

The print dialog issues requests from chrome://print, which was now blocked from
using third party cookies.

Original change's description:
> Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic
>
> This was suggested in  crbug.com/789636  so this logic can run in
> production before the launch of the network service.
>
> Had to keep the tab notification logic in each delegate, since the
> way NetworkServiceNetworkDelegate maps the request back to the
> original tab (using the URLLoader set as user data on the request)
> does not work if network service is disabled.
>
> Bug:  789636 ,  789632 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I22395f914639d8ce83457a63062927e876caeaa9
> Reviewed-on: https://chromium-review.googlesource.com/1113903
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570877}

Bug:  878051 ,  789636 ,  789632 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
Change-Id: Ib023acff943f992c8b6ec8b15bbdbc1524ed3720
Reviewed-on: https://chromium-review.googlesource.com/1194329
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586944}
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/android_webview/browser/net/aw_network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/android_webview/browser/net/aw_network_delegate.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/chrome/browser/android/download/intercept_download_resource_throttle.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/chrome/browser/net/chrome_network_delegate_unittest.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/content/shell/browser/shell_network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/content/shell/browser/shell_network_delegate.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/ios/chrome/browser/net/ios_chrome_network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/ios/chrome/browser/net/ios_chrome_network_delegate.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/layered_network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/layered_network_delegate.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/layered_network_delegate_unittest.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/network_delegate.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/network_delegate_impl.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/base/network_delegate_impl.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/proxy_resolution/network_delegate_error_observer_unittest.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/url_request/url_request.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/net/url_request/url_request_test_util.h
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/BUILD.gn
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/network_context.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/network_context_unittest.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/network_service_network_delegate.h
[add] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/network_service_network_delegate_unittest.cc
[modify] https://crrev.com/150c7353da386029eac1513f22257a3717ebf991/services/network/network_service_unittest.cc

As per comment #22, as bugdroid comment is available, removing 'Needs-Bisect' label.
Please feel free to add if needed.

Thanks..

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

commit 05c6076c45368bc3557e639b20dd4a1dc143314e
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Aug 29 06:40:06 2018

Revert "Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic"

This reverts commit 8420d58c08a2b1dd5a0a414e44284491a5518331.

Reason for revert: This breaks the sign-in on the print dialog when third party
cookies are blocked:  http://crbug.com/878051 .
This is because the network service does not have the custom logic to whitelist
chrome:// or chrome-extensions:// schemes, which used to be here:
https://cs.chromium.org/chromium/src/components/content_settings/core/browser/cookie_settings.cc?l=98&rcl=5edbbd319ff4010b66e07ec404a3188716a9cda2

The print dialog issues requests from chrome://print, which was now blocked from
using third party cookies.

Original change's description:
> Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic
>
> This was suggested in  crbug.com/789636  so this logic can run in
> production before the launch of the network service.
>
> Had to keep the tab notification logic in each delegate, since the
> way NetworkServiceNetworkDelegate maps the request back to the
> original tab (using the URLLoader set as user data on the request)
> does not work if network service is disabled.
>
> Bug:  789636 ,  789632 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I22395f914639d8ce83457a63062927e876caeaa9
> Reviewed-on: https://chromium-review.googlesource.com/1113903
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570877}

Bug:  878051 ,  789636 ,  789632 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
Change-Id: Ib023acff943f992c8b6ec8b15bbdbc1524ed3720
Reviewed-on: https://chromium-review.googlesource.com/1194329
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#586944}(cherry picked from commit 150c7353da386029eac1513f22257a3717ebf991)
Reviewed-on: https://chromium-review.googlesource.com/1195153
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3535@{#5}
Cr-Branched-From: 4d18295073a6bf8f2f4d26e4e059767e091eaaf3-refs/heads/master@{#586474}
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/android_webview/browser/net/aw_network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/android_webview/browser/net/aw_network_delegate.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/chrome/browser/android/download/intercept_download_resource_throttle.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/chrome/browser/net/chrome_network_delegate_unittest.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/content/shell/browser/shell_network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/content/shell/browser/shell_network_delegate.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/ios/chrome/browser/net/ios_chrome_network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/ios/chrome/browser/net/ios_chrome_network_delegate.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/layered_network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/layered_network_delegate.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/layered_network_delegate_unittest.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/network_delegate.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/network_delegate_impl.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/base/network_delegate_impl.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/proxy_resolution/network_delegate_error_observer_unittest.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/url_request/url_request.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/net/url_request/url_request_test_util.h
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/BUILD.gn
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/network_context.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/network_context_unittest.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/network_service_network_delegate.h
[add] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/network_service_network_delegate_unittest.cc
[modify] https://crrev.com/05c6076c45368bc3557e639b20dd4a1dc143314e/services/network/network_service_unittest.cc

Today's 8:00 PM PT canary failed to trigger : https://bugs.chromium.org/p/chromium/issues/detail?id=876964#c31.

So I merged revert listed at #22 to current canary branch at #24 and triggering new canary from same branch.

Canary #70.0.3535.4 in progress with this merge in. 

Pls update bug with canary result tomorrow. Thank you.

The NextAction date has arrived: 2018-08-29

As per comment #14, Tried to reproduce the issue on Windows 10 on the latest Beta 69.0.3497.57 and unable to reproduce the issue.
As per comment #21, tried to reproduce the issue by enabling and disabling the "Block third party cookies" option, but the same behavior is observed on the build without fix and on the latest M-70.

Google CloudPrint setup is not available at TE end to test and verify this issue.

cduvall@ Request you to check and help us in verifying the fix on the M-70 build.

Thanks..

I am not able to reproduce the issue on 70.0.3535.4, looks like the revert worked.

thestig@, could you also try reproducing on the latest canary just to confirm the fix?

Yep, Canary works for me. Can we do a clean merge to M69?

Thank you cduvall@ and thestig@. 

Pls request a merge to M69 if cl is now safe to merge. 

This has been verified on Canary, requesting merge.

This bug requires manual review: We are only 5 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

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

Approving merge to M69 branch 3497 based on comment #32.

Pls mark bug as fixed after the merge if nothing else is pending after the merge. Thank you.

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

commit 36f832483da0f645784a0e9b25cda3860e510ca2
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Aug 29 18:58:29 2018

Revert "Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic"

This reverts commit 8420d58c08a2b1dd5a0a414e44284491a5518331.

Reason for revert: This breaks the sign-in on the print dialog when third party
cookies are blocked:  http://crbug.com/878051 .
This is because the network service does not have the custom logic to whitelist
chrome:// or chrome-extensions:// schemes, which used to be here:
https://cs.chromium.org/chromium/src/components/content_settings/core/browser/cookie_settings.cc?l=98&rcl=5edbbd319ff4010b66e07ec404a3188716a9cda2

The print dialog issues requests from chrome://print, which was now blocked from
using third party cookies.

Original change's description:
> Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic
>
> This was suggested in  crbug.com/789636  so this logic can run in
> production before the launch of the network service.
>
> Had to keep the tab notification logic in each delegate, since the
> way NetworkServiceNetworkDelegate maps the request back to the
> original tab (using the URLLoader set as user data on the request)
> does not work if network service is disabled.
>
> Bug:  789636 ,  789632 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I22395f914639d8ce83457a63062927e876caeaa9
> Reviewed-on: https://chromium-review.googlesource.com/1113903
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570877}

TBR=cduvall@chromium.org

(cherry picked from commit 150c7353da386029eac1513f22257a3717ebf991)

Bug:  878051 ,  789636 ,  789632 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib023acff943f992c8b6ec8b15bbdbc1524ed3720
Reviewed-on: https://chromium-review.googlesource.com/1194329
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#586944}
Reviewed-on: https://chromium-review.googlesource.com/1195751
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#841}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/android_webview/browser/net/aw_network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/android_webview/browser/net/aw_network_delegate.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/chrome/browser/android/download/intercept_download_resource_throttle.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/chrome/browser/net/chrome_network_delegate_unittest.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/content/shell/browser/shell_network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/content/shell/browser/shell_network_delegate.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/ios/chrome/browser/net/ios_chrome_network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/ios/chrome/browser/net/ios_chrome_network_delegate.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/layered_network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/layered_network_delegate.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/layered_network_delegate_unittest.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/network_delegate.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/network_delegate_impl.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/base/network_delegate_impl.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/proxy_resolution/network_delegate_error_observer_unittest.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/url_request/url_request.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/net/url_request/url_request_test_util.h
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/services/network/BUILD.gn
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/services/network/network_context.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/services/network/network_context_unittest.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/services/network/network_service_network_delegate.h
[add] https://crrev.com/36f832483da0f645784a0e9b25cda3860e510ca2/services/network/network_service_network_delegate_unittest.cc

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

commit e873764be569975947a71da6323c690f1a25e50e
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Aug 31 17:26:34 2018

Reland "Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic"

This reverts commit 150c7353da386029eac1513f22257a3717ebf991.

Added some more tests, and support for the special casing of chrome:// URLs.
The chrome-extension:// case is only needed when network service is disabled,
since chrome-extension:// requests only use the URLRequestContext in that case.

See diff after Patchset 1 for changes since the original.

Original change's description:
> Revert "Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic"
>
> This reverts commit 8420d58c08a2b1dd5a0a414e44284491a5518331.
>
> Reason for revert: This breaks the sign-in on the print dialog when third party
> cookies are blocked:  http://crbug.com/878051 .
> This is because the network service does not have the custom logic to whitelist
> chrome:// or chrome-extensions:// schemes, which used to be here:
> https://cs.chromium.org/chromium/src/components/content_settings/core/browser/cookie_settings.cc?l=98&rcl=5edbbd319ff4010b66e07ec404a3188716a9cda2
>
> The print dialog issues requests from chrome://print, which was now blocked from
> using third party cookies.
>
> Original change's description:
> > Unify ChromeNetworkDelegate and NetworkServiceNetworkDelegate cookie logic
> >
> > This was suggested in  crbug.com/789636  so this logic can run in
> > production before the launch of the network service.
> >
> > Had to keep the tab notification logic in each delegate, since the
> > way NetworkServiceNetworkDelegate maps the request back to the
> > original tab (using the URLLoader set as user data on the request)
> > does not work if network service is disabled.
> >
> > Bug:  789636 ,  789632 
> > Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> > Change-Id: I22395f914639d8ce83457a63062927e876caeaa9
> > Reviewed-on: https://chromium-review.googlesource.com/1113903
> > Reviewed-by: Matt Menke <mmenke@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#570877}
>
> Bug:  878051 ,  789636 ,  789632 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
> Change-Id: Ib023acff943f992c8b6ec8b15bbdbc1524ed3720
> Reviewed-on: https://chromium-review.googlesource.com/1194329
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#586944}

Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ia2b596baeacd1d25d99104db88403846e251f806
Bug:  878051 ,  789636 ,  789632 
Reviewed-on: https://chromium-review.googlesource.com/1195147
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588072}
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/android_webview/browser/net/aw_network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/android_webview/browser/net/aw_network_delegate.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/chrome/browser/android/download/intercept_download_resource_throttle.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/chrome/browser/net/chrome_network_delegate_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/chrome/browser/net/profile_network_context_service.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/content/shell/browser/shell_network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/content/shell/browser/shell_network_delegate.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/ios/chrome/browser/net/ios_chrome_network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/ios/chrome/browser/net/ios_chrome_network_delegate.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/layered_network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/layered_network_delegate.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/layered_network_delegate_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/network_delegate.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/network_delegate_impl.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/base/network_delegate_impl.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/proxy_resolution/network_delegate_error_observer_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/url_request/url_request.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/net/url_request/url_request_test_util.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/BUILD.gn
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/cookie_manager.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/cookie_settings.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/cookie_settings.h
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/cookie_settings_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/network_context.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/network_context_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/network_service_network_delegate.h
[delete] https://crrev.com/24565e4134dafe85af45f9e02a26afdb5e8f847d/services/network/network_service_network_delegate_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/network_service_unittest.cc
[modify] https://crrev.com/e873764be569975947a71da6323c690f1a25e50e/services/network/public/mojom/cookie_manager.mojom

As per comment #14, #28, as this issue is not reproducible at TE end on the reported version 69.0.3497.42, as Google CloudPrint setup is not available.

cduvall@ Request you to check and help us in verifying the fix on the latest M-71 build.

Thanks..

cduvall@ verified this change M69 stable RC on #69.0.3497.81. Thank you.

We likely need a M70 merge here.

This bug requires manual review: We are only 2 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

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

We actually don't need the reland in comment 37 for M70, the revert from comment 24 fixes the bug and is in M70.

You are right, this doesn't need a merge. (It's actually comment 22 that originally landed on trunk as r586944.)