New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 789632 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug
Proj-Servicification

Blocking:
issue 598073
issue 840359


Show other hotlists

Hotlists containing this issue:
XXX


Sign in to add a comment

Get cookie/channel ID blocking working with the network service.

Project Member Reported by mmenke@chromium.org, Nov 29 2017

Issue description

ChromeNetworkDelegate blocks setting/getting cookies and forces pivacy mode for some requests (See OnCanSetCookie, OnCanGetCookie, and OnCanEnablePrivacyMode).  We should make all these work with the network service.  I believe these are mostly configured through extension APIs, though could be wrong about that.
 

Comment 1 by mmenke@chromium.org, Nov 29 2017

Components: Internals>Network>Cookies Internals>Network>SSL
(Using SSL label because I don't think we have a channel ID label?)
Cc: nhar...@chromium.org
They may be configured through extension APIs, but the current implementation:

1) Checks Content Settings (based on the target URL and the requesting URL)
2) Notifies the tab (for updating UI about cookies that were blocked/allowed)

Comment 4 by mmenke@chromium.org, Nov 29 2017

I filed a separate bug about the cookie updating, as I consider it a somewhat tangential issue that should be handled separately.
What's the separate bug #, for reference?

I agree that splitting it out is useful, which is why I was proposing changing those events in the URLRequest style to be notification-events only, with only a single unified policy consultation.

In any event, the policy (for channel ID and cookies) right now filters through Content Settings, which can be controlled by extensions, but set explicitly rather than as a callback/async event. If we have sufficient context in the request (to know first and third party URL), then I think the caller may be able to set these at time of request, as we already consult policy before starting the request (for Channel ID). That's why I was proposing unifying them into a single state.

The question that I don't have resolved yet, but hope to within the next day or two, is whether:
- It's sufficient to explicitly unify such blocking under a single control
- What the expected redirect behaviours are supposed to be (in the case of 3PCB, whether 3P with cookies blocked -> 1P thus with cookies -> 3P with query string should be 'valid'). If I can get a clear 'no' to that, then we can fully make it a flag at the request initialization that propogates through requests and would only require the policy consultation before the request is initialized.

Hopefully that made sense with the context of the design doc we've been discussing.

Comment 6 by mmenke@chromium.org, Nov 29 2017

The cookie logging bug is  issue 789636 .  Hrm...Wonder if we should be logging channel ID through a similar mechanism, to reveal to the user all data we're sending to a site.

I agree that it would be great if we can unify them all.
Ryan or Matt, any progress on this design doc or unified policy consultation mentioned in #5? 
Cc: jselover@chromium.org
So, Jesse's giving it a go getting up to speed on unifying the privacy mode flags, which is related to this. From the cross-team sync up we held prior to IETF101, it's also clear that we need to treat 3rd-party cookie blocking separate from any notion of a credentials flag (important for both privacy goals and performance), so we can't unify those as a single request flag.

That is, we're going to need two modes:
- Credentials flag (comparable to the CORS notion, but also self-imposed)
- 3PCB flag (unifying the two methods we have into a single method, since content settings only expresses it as a single bit)

The redirect problem is still unsolved, though, and significantly affects the shape of any API here. That is, our two options are to set a flag at the start of the request and have it propagate through redirects, or to set a callback and set it per-hop. The former is cleaner, but is a observable behaviour change in cookie-blocking behaviour, while the latter is consistent with the current behaviour, but not necessarily desirable (and with holes).

Has the network service solved how it will be handling things like CORS policies on redirects yet? Or even where the CORS policies will live? Are those being propagated back to the browser process (now that CORS is out of the renderer)? If so, going the route of maintaining the current behaviour (by dispatching callbacks during each leg of the Request) may be viable for now. Previously, I'd been given to understand it was seen as non-viable because of abstract, underspecified concerns about performance.
There's services/network/cors/, with which I am completely unfamiliar.
Blocking: 840359
FWIW, reporting seems to partly rely on cookie policy and partly do its own thing; so it may be worthwhile to consider it when designing this...
morlovich: Could you clarify?

Comment 13 by jam@chromium.org, May 14 2018

Cc: mmenke@chromium.org jam@chromium.org
 Issue 803452  has been merged into this issue.

Comment 14 by jam@chromium.org, May 14 2018

Owner: jam@chromium.org
Status: Assigned (was: Untriaged)
I can take a look at this if no one has started (if anyone has, please let me know!).

@sleevi: some of the discussion above seems to be about behavior changes (e.g. independent of getting existing functionality working with network service). If so, let's discuss this in a separate bug, as this should just cover getting network service flag to parity with existing behavior. Regarding CORS, that's still in the renderer.

Comment 16 by jam@chromium.org, May 14 2018

const base::Feature kOutOfBlinkCORS{"OutOfBlinkCORS",
                                    base::FEATURE_DISABLED_BY_DEFAULT};
Hi John,

It sounds like there's some miscommunication, so we should set up a Hangout as appropriate. I think the question from Comment #8 still stands, and the direction being moved towards in Comment #15 (to move it in) seems like it either answers or doesn't answer the question in #8. The description in Comment #8 also applies - namely, that there's opportunity to make sure that the S13N portion of this is done in a principled way, rather than simply converting a known problematic API 1:1, and it seems like doing that could save work both short and long term. That's not about (observable) behaviour changes, but about making sure that there's a sensible API contract, rather than just porting it over directly.
Issue 799935 has the design doc that's worked with the various owners for this functionality.

As mentioned in Comment #3, the API doesn't need to be a 1:1 mapping of the existing APIs, and a more targeted API such as handles those two methods would suffice.

Further, depending on the CORS side (that is, whether or not redirects are processed Service side or Browser side), it's possible to reduce that API into explicitly declarative approaches - namely, whether or not 3PCB is enabled for that domain (when making the request) and what cookies were blocked if that was set (when returning the response).

The CORS implications matter in that if redirects are handled in the Network Service, then it would need to reconsult the browser process on each redirect for the Content Settings for each destination in the redirect to check the 3PCB status, and update the flag as appropriate.

In either event, it either ends up as flag + result set in the response or callback + result set on the response, but doesn't need the parity with the current API, and would benefit from not maintaining the (buggy) API.
Project Member

Comment 19 by bugdroid1@chromium.org, May 17 2018

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

commit 22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu May 17 17:22:18 2018

Add support for third-party cookie blocking with network service.

Bug:  789632 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I651d2f1fe7211d7ef8bd13ff8434140206c4a009
Reviewed-on: https://chromium-review.googlesource.com/1058274
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559585}
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/chrome/browser/profiles/profile_impl.h
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/content/shell/browser/layout_test/blink_test_controller.h
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/content/shell/browser/layout_test/layout_test_message_filter.cc
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/content/shell/browser/layout_test/layout_test_message_filter.h
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/BUILD.gn
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/network_context.cc
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/network_context.h
[add] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/network_service_network_delegate.cc
[add] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/network_service_network_delegate.h
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/services/network/test/test_network_context.h
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[modify] https://crrev.com/22d1b1e011aa6a965aaf8b5c42fd4c673dd5f175/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 20 by dxie@chromium.org, May 22 2018

Labels: -Pri-3 Proj-Servicification-Canary OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1

Comment 21 by jam@chromium.org, Jun 7 2018

Owner: cduvall@chromium.org
Per discussion, assigning to Clark since he's looking at the other cookie callback on shutdown and this uses similar data.
Status: Started (was: Assigned)
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 14 2018

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

commit 4a5c3582b56c6b7ddd221509e2131e82abe055be
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Jun 14 16:32:28 2018

Test the different ways of reading and writing cookies with content settings.

Handle all combinations of:
-reading cookies via JavaScript or by inspecting HTTP request headers
-setting cookies via JavaScript or by HTTP response headers

Bug:  789632 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I9473c9b0b514ada40ac36e07e4df2dc7e44613e1
Reviewed-on: https://chromium-review.googlesource.com/1099482
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567300}
[modify] https://crrev.com/4a5c3582b56c6b7ddd221509e2131e82abe055be/chrome/browser/content_settings/content_settings_browsertest.cc
[add] https://crrev.com/4a5c3582b56c6b7ddd221509e2131e82abe055be/chrome/test/data/set_cookie_header.html
[add] https://crrev.com/4a5c3582b56c6b7ddd221509e2131e82abe055be/chrome/test/data/set_cookie_header.html.mock-http-headers
[modify] https://crrev.com/4a5c3582b56c6b7ddd221509e2131e82abe055be/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 19 2018

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

commit c57280bed803e6468633d57af7c149215e119fa5
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Jun 19 03:46:23 2018

Check cookie access when getting or setting cookies in network service

This matches the behavior in ChromeNetworkDelegate.

Third party cookie handling has been moved from NetworkContext to
CookieManager.

Note: A lot of these files changed are removing the unnecessary
CookieOptions arg from a bunch of methods.


Bug:  789636 ,  789632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I241cb396e588c433cf723c1effa8ea2c64f72266
Reviewed-on: https://chromium-review.googlesource.com/1100105
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568325}
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/android_webview/browser/aw_content_browser_client.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/android_webview/browser/aw_cookie_access_policy.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/android_webview/browser/aw_cookie_access_policy.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/content_settings/content_settings_browsertest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/content_settings/tab_specific_content_settings.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/content_settings/tab_specific_content_settings.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/content_settings/tab_specific_content_settings_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/components/content_settings/core/browser/cookie_settings.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/components/content_settings/core/browser/cookie_settings.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/components/content_settings/core/common/cookie_settings_base.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/components/content_settings/core/common/cookie_settings_base.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/components/content_settings/core/common/cookie_settings_base_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/content/browser/frame_host/render_frame_message_filter.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/content/browser/network_service_client.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/content/browser/network_service_client.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/content/public/browser/content_browser_client.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/headless/lib/browser/headless_content_browser_client.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/headless/lib/browser/headless_content_browser_client.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/cookie_manager.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/cookie_manager.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/cookie_manager_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/cookie_settings.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/cookie_settings.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/cookie_settings_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/network_context.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/network_context.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/network_service_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/public/mojom/cookie_manager.mojom
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/test/test_network_context.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/url_loader.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/url_loader.h
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/services/network/url_loader_unittest.cc
[modify] https://crrev.com/c57280bed803e6468633d57af7c149215e119fa5/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 25 by bugdroid1@chromium.org, Jun 19 2018

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

commit c0b6c25b06e2bc26634f294e511ab5bea1e008ea
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Jun 19 17:32:05 2018

Network Service: Sync cookie settings to incognito profiles

Bug:  789632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Idbea61afbedd446993f01f5701a8f4ef484c5926
Reviewed-on: https://chromium-review.googlesource.com/1103188
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568511}
[modify] https://crrev.com/c0b6c25b06e2bc26634f294e511ab5bea1e008ea/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/c0b6c25b06e2bc26634f294e511ab5bea1e008ea/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 26 by bugdroid1@chromium.org, Jun 19 2018

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

commit 936b660c4c5ba36e21c9ac293846a67fe1cf4481
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Jun 19 21:21:23 2018

Network Service: Implement OnCanEnablePrivacyMode in network delegate

This matches the implementation in ChromeNetworkDelegate.

Bug:  789632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I68c00d635fd1eb527aa2e80851acf8a3fd7325af
Reviewed-on: https://chromium-review.googlesource.com/1104937
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568606}
[modify] https://crrev.com/936b660c4c5ba36e21c9ac293846a67fe1cf4481/services/network/BUILD.gn
[modify] https://crrev.com/936b660c4c5ba36e21c9ac293846a67fe1cf4481/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/936b660c4c5ba36e21c9ac293846a67fe1cf4481/services/network/network_service_network_delegate.h
[add] https://crrev.com/936b660c4c5ba36e21c9ac293846a67fe1cf4481/services/network/network_service_network_delegate_unittest.cc

[cduvall]:  Do we need to keep the legacy path in ChromeNetworkDelegate, or could we move the new implementations into NetworkContext::ContextNetworkDelegate (Which wraps the real network delegate).  That way, we can have the new code working in production and remove the old code.

Thought I asked this earlier today on an issue, but I can't find my earlier comment, so maybe got distracted before I posted.
Project Member

Comment 28 by bugdroid1@chromium.org, Jun 20 2018

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

commit a7501a27f3ca3ccfca8afe34ed9b12d6b7763137
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Jun 20 17:34:02 2018

Network Service: Handle deleting session only channel IDs

SessionCleanupChannelIDStore and tests are mostly copied from
QuotaPolicyChannelIDStore.


Bug:  789644 ,  789632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I1178b774273ac39ae86b1aff3be94be1e1b00058
Reviewed-on: https://chromium-review.googlesource.com/1107132
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568901}
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/chrome/browser/net/DEPS
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/chrome/browser/net/quota_policy_channel_id_store.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/chrome/browser/net/quota_policy_channel_id_store.h
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/components/content_settings/core/browser/cookie_settings.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/components/content_settings/core/browser/cookie_settings.h
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/components/content_settings/core/common/cookie_settings_base.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/components/content_settings/core/common/cookie_settings_base.h
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/components/content_settings/core/common/cookie_settings_base_unittest.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/BUILD.gn
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/cookie_manager.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/cookie_manager.h
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/cookie_manager_unittest.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/network_context.cc
[modify] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/network_context.h
[add] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/session_cleanup_channel_id_store.cc
[add] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/session_cleanup_channel_id_store.h
[add] https://crrev.com/a7501a27f3ca3ccfca8afe34ed9b12d6b7763137/services/network/session_cleanup_channel_id_store_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Jun 27 2018

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

commit 8420d58c08a2b1dd5a0a414e44284491a5518331
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Jun 27 20:05:01 2018

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}
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/android_webview/browser/net/aw_network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/android_webview/browser/net/aw_network_delegate.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/chrome/browser/android/download/intercept_download_resource_throttle.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/chrome/browser/net/chrome_network_delegate_unittest.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/content/shell/browser/shell_network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/content/shell/browser/shell_network_delegate.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/ios/chrome/browser/net/ios_chrome_network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/ios/chrome/browser/net/ios_chrome_network_delegate.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/layered_network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/layered_network_delegate.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/layered_network_delegate_unittest.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/network_delegate.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/network_delegate_impl.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/base/network_delegate_impl.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/proxy_resolution/network_delegate_error_observer_unittest.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/proxy_resolution/pac_file_fetcher_impl_unittest.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/url_request/url_request.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/url_request/url_request_test_util.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/net/url_request/url_request_test_util.h
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/services/network/BUILD.gn
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/services/network/network_context.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/services/network/network_context_unittest.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/services/network/network_service_network_delegate.cc
[modify] https://crrev.com/8420d58c08a2b1dd5a0a414e44284491a5518331/services/network/network_service_network_delegate.h
[delete] https://crrev.com/d665ffc963e72121d16059a3b48aa99397fb0031/services/network/network_service_network_delegate_unittest.cc

Status: Fixed (was: Started)
Cookie/channel ID blocking now work through the network service, and can be configured with content settings.
Project Member

Comment 31 by bugdroid1@chromium.org, Jul 4

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

commit 87135a31e034842e4e2030b309276e8851008cc4
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Jul 04 01:49:54 2018

Network Service: Enable encrypted cookies

Cookies will now be stored encrypted by default in the network service.
Tested that  crbug.com/848361  is fixed with this change.

Bug:  789632 ,  848361 ,  789644 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ic627501ddf1c5030bbf2a203f005f5ebca92dfd8
Reviewed-on: https://chromium-review.googlesource.com/1104791
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572444}
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/chrome/browser/chrome_browser_main_linux.cc
[add] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/chrome/browser/chrome_network_service_browsertest.cc
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/chrome/test/BUILD.gn
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/components/safe_browsing/browser/safe_browsing_network_context.cc
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/BUILD.gn
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/DEPS
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/network_context.cc
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/network_service.cc
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/network_service.h
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/network_service_unittest.cc
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/public/mojom/BUILD.gn
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/87135a31e034842e4e2030b309276e8851008cc4/services/network/public/mojom/network_service.mojom

Project Member

Comment 32 by bugdroid1@chromium.org, Jul 4

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

commit ea0521dd24669f34f0d64927eef0fa847b677eb0
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Jul 04 06:04:00 2018

Revert "Network Service: Enable encrypted cookies"

This reverts commit 87135a31e034842e4e2030b309276e8851008cc4.

Reason for revert: ChromeNetworkServiceBrowserTest.EncryptedCookies
keeps failing on Mac10.11 bot.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/27835

Original change's description:
> Network Service: Enable encrypted cookies
> 
> Cookies will now be stored encrypted by default in the network service.
> Tested that  crbug.com/848361  is fixed with this change.
> 
> Bug:  789632 ,  848361 ,  789644 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: Ic627501ddf1c5030bbf2a203f005f5ebca92dfd8
> Reviewed-on: https://chromium-review.googlesource.com/1104791
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Maks Orlovich <morlovich@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572444}

TBR=dcheng@chromium.org,jam@chromium.org,mmenke@chromium.org,cfroussios@chromium.org,morlovich@chromium.org,cduvall@chromium.org

Change-Id: I192a10c6d28a102dcf5ae247cdbda4c602872eb9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  789632 ,  848361 ,  789644 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1125460
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572490}
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/chrome/browser/chrome_browser_main_linux.cc
[delete] https://crrev.com/2de92717271b3a0bd9eaf260bed975b6e9c0e76a/chrome/browser/chrome_network_service_browsertest.cc
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/chrome/test/BUILD.gn
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/components/safe_browsing/browser/safe_browsing_network_context.cc
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/BUILD.gn
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/DEPS
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/network_context.cc
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/network_service.cc
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/network_service.h
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/network_service_unittest.cc
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/public/mojom/BUILD.gn
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/ea0521dd24669f34f0d64927eef0fa847b677eb0/services/network/public/mojom/network_service.mojom

Project Member

Comment 33 by bugdroid1@chromium.org, Jul 6

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

commit 1c6f0e1c732f129237c96e7e01622da7d54bdd7a
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Jul 06 17:31:34 2018

Reland "Network Service: Enable encrypted cookies"

This is a reland of 87135a31e034842e4e2030b309276e8851008cc4

Original change's description:
> Network Service: Enable encrypted cookies
>
> Cookies will now be stored encrypted by default in the network service.
> Tested that  crbug.com/848361  is fixed with this change.
>
> Bug:  789632 ,  848361 ,  789644 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: Ic627501ddf1c5030bbf2a203f005f5ebca92dfd8
> Reviewed-on: https://chromium-review.googlesource.com/1104791
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Maks Orlovich <morlovich@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572444}

TBR=jam@chromium.org,dcheng@chromium.org,mmenke@chromium.org,cfroussios@chromium.org,morlovich@chromium.org

Bug:  789632 ,  848361 ,  789644 
Change-Id: I68ec0249a0f8fba8563c4a24abb6482da33d4ebc
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1127960
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573003}
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/chrome/browser/chrome_browser_main_linux.cc
[add] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/chrome/browser/chrome_network_service_browsertest.cc
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/chrome/test/BUILD.gn
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/components/safe_browsing/browser/safe_browsing_network_context.cc
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/BUILD.gn
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/DEPS
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/network_context.cc
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/network_service.cc
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/network_service.h
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/network_service_unittest.cc
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/public/mojom/BUILD.gn
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/1c6f0e1c732f129237c96e7e01622da7d54bdd7a/services/network/public/mojom/network_service.mojom

Project Member

Comment 34 by bugdroid1@chromium.org, Aug 29

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

Project Member

Comment 35 by bugdroid1@chromium.org, Aug 29

Labels: merge-merged-3535
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

Project Member

Comment 36 by bugdroid1@chromium.org, Aug 29

Labels: merge-merged-3497
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

Project Member

Comment 37 by bugdroid1@chromium.org, Aug 31

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

Sign in to add a comment