New issue
Advanced search Search tips

Issue 847555 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 875202



Sign in to add a comment

Make app request contexts work with the NetworkService

Project Member Reported by mmenke@chromium.org, May 29 2018

Issue description

They currently only use URLRequestContexts.  This will be simplest to do if we can wait until everything works with the network service, so we don't need significantly forked in/out of process configuration paths.

In particular waiting until cookie/channel IDs, extensions, HTTP auth, cert verification, about URLs, and pretty much everything we require in-process only code to configure no longer requires that code would save a lot of effort and duplicated code.  Should also wait until we no longer need a media URLRequestContext.
 

Comment 1 by dxie@chromium.org, Jun 8 2018

Labels: OS-Chrome OS-Windows OS-Mac OS-Linux
Status: Available (was: Untriaged)
mmenke@, is there anything actionable here?
I'm waiting for secure storage and channel ID to be hooked up to the network service, to minimize the amount of hybrid setup code needed (We'll still need some hybrid setup, for the NetworkDelegate and protocol handlers).
Owner: mmenke@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10

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

commit 34080e1ba2dac9c67d340657c665dd625db84af9
Author: Matt Menke <mmenke@chromium.org>
Date: Fri Aug 10 16:04:42 2018

ChromeURLRequestContextGetter:  Initialize on construction.

Previously, they were initialized on first use, however, now that
initialization sets up NetworkContexts when the network service is
disabled, relying on the first use of the class through the
ChromeURLRequestContextGetter API before setting up the NetworkContext
could cause problems.

Bug:  847555 
Change-Id: I2aaac7e717e64fb41ba2fb9141506bb7512b73fa
Reviewed-on: https://chromium-review.googlesource.com/1169319
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582188}
[modify] https://crrev.com/34080e1ba2dac9c67d340657c665dd625db84af9/chrome/browser/net/chrome_url_request_context_getter.cc
[modify] https://crrev.com/34080e1ba2dac9c67d340657c665dd625db84af9/chrome/browser/net/chrome_url_request_context_getter.h
[modify] https://crrev.com/34080e1ba2dac9c67d340657c665dd625db84af9/chrome/browser/profiles/off_the_record_profile_io_data.cc
[modify] https://crrev.com/34080e1ba2dac9c67d340657c665dd625db84af9/chrome/browser/profiles/profile_impl_io_data.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 14

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

commit d38efd91f35c3439e8e5081f47634b46396ecad1
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Aug 14 20:39:45 2018

NetworkService: Migrate isolated StoragePartition URLRequestContexts

Their configuration is now done through a NetworkContextParams object,
other than injecting ProtocolHandlers / URLRequestJobInterceptors, and
cert verification.

This does change behavior in a few ways: Previously, CookieSettings
were only partially applied to isolated URLRequestContexts - settings
applied through the NetworkDelegate, like blocking cookies, were
applied, but settings applied through the QuotaPolicyCookieStore,
like making cookies for some sites session-only, were not. This CL
makes both types of settings apply to isolated partitions to avoid
having separate NetworkContext APIs to configure each subset of
cookie behaviors, which are normally keyed off the same set of
settings.

Also, isolated URLRequestContexts are more independent of
the main profile URLRequestContext than they used to be.  They only
share a cert verifier, and most of the NetworkDelegate, in addition
to global objects (HostResolver, AuthHandlerFactory, NQE, NCN, and
NetLog), as opposed to sharing everything but NetworkContext,
URLRequestJobFactory, CookieStore, DiskCache, and HttpNetworkSession.

BUG= 847555 

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Id462de6d71029210a7af7a7ac3fa8f3de896d9d8
Reviewed-on: https://chromium-review.googlesource.com/1153630
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583018}
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/net/chrome_url_request_context_getter.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/net/chrome_url_request_context_getter.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/net/profile_network_context_service.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/net/profile_network_context_service.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/off_the_record_profile_io_data.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/off_the_record_profile_io_data.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile_browsertest.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile_impl_io_data.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/chrome/test/base/testing_profile.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/net/base/layered_network_delegate.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/net/base/layered_network_delegate.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/net/url_request/url_request_context_builder.h
[modify] https://crrev.com/d38efd91f35c3439e8e5081f47634b46396ecad1/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 15

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

commit 8f68663c63057cbbd0ff89443d034e0fce578d7f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Aug 15 07:29:02 2018

Sheriff: Disable NetworkContextConfigurationBrowserTest.CookiesEnabled on Mac OSX

This test (OnDiskApp/NetworkContextConfigurationBrowserTest.CookiesEnabled/1) is
now failing on the Mac10.11 bot:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/28580

Noth that the Findit suspects the following CL is failing the test with 86%
confidence:
https://chromium-review.googlesource.com/c/chromium/src/+/1153630
https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/28580

At first, I tried to revert it but failed because of merge conflict:
https://chromium-review.googlesource.com/c/chromium/src/+/1175601

Bug:  847555 
Change-Id: Idf91c9f8595139dd1d9c667adc304e08d0e6083d
Tbr: mmenke@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1175585
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583189}
[modify] https://crrev.com/8f68663c63057cbbd0ff89443d034e0fce578d7f/chrome/browser/net/network_context_configuration_browsertest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 16

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

commit dea0b13bfb57aa3c07c3d2a82dd6f3a1ae5e7027
Author: Matt Menke <mmenke@chromium.org>
Date: Thu Aug 16 22:12:24 2018

OSX: Try to fix NetworkContextConfigurationBrowserTest.CookiesEnabled.

Looks the CookieStore isn't flushed on browser exit, by default.

BUG= 847555 

Change-Id: I2596f8ea3d62a5d5e71e0514ba8e7caa6049605d
Reviewed-on: https://chromium-review.googlesource.com/1178803
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583846}
[modify] https://crrev.com/dea0b13bfb57aa3c07c3d2a82dd6f3a1ae5e7027/chrome/browser/net/network_context_configuration_browsertest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 17

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

commit 7bc01f1ffb9f4affc67d10ed5c3c3691fe519078
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Fri Aug 17 11:24:45 2018

Revert "OSX: Try to fix NetworkContextConfigurationBrowserTest.CookiesEnabled."

This reverts commit dea0b13bfb57aa3c07c3d2a82dd6f3a1ae5e7027.

Reason for revert: As described in  https://crbug.com/875202 , this rework broke the PRE_CookiesEnabled test. I haven't seen CookiesEnabled fail (but there has been a lot of nose, so please check that as well before you reland).

Original change's description:
> OSX: Try to fix NetworkContextConfigurationBrowserTest.CookiesEnabled.
> 
> Looks the CookieStore isn't flushed on browser exit, by default.
> 
> BUG= 847555 
> 
> Change-Id: I2596f8ea3d62a5d5e71e0514ba8e7caa6049605d
> Reviewed-on: https://chromium-review.googlesource.com/1178803
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Maks Orlovich <morlovich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583846}

TBR=mmenke@chromium.org,morlovich@chromium.org

Change-Id: I7e2fd7c874026fccdc200b0fd78af2faa4614095
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847555 
Reviewed-on: https://chromium-review.googlesource.com/1179761
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584020}
[modify] https://crrev.com/7bc01f1ffb9f4affc67d10ed5c3c3691fe519078/chrome/browser/net/network_context_configuration_browsertest.cc

Blockedon: 875202
Cc: mmenke@chromium.org morlovich@chromium.org
 Issue 875202  has been merged into this issue.
Status: Fixed (was: Started)

Sign in to add a comment