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

Issue 792546 link

Starred by 3 users

Issue metadata

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


Sign in to add a comment

Network Service: Cross-site document blocking for non-exploited renderers

Project Member Reported by nick@chromium.org, Dec 6 2017

Issue description

In  issue 786505 , creis@ implemented an initial cross-site document blocking policy that is based on a ResourceHandler.

The same policy needs to be implemented by the network service.

Implementing this should fix the following browser_tests when run with the network service enabled:

 -SiteIsolationStatsGathererBrowserTest/SiteIsolationStatsGathererBrowserTest.CrossSiteDocumentBlockingForMimeType/0
 -SiteIsolationStatsGathererBrowserTest/SiteIsolationStatsGathererBrowserTest.CrossSiteDocumentBlockingForMimeType/1
 -CrossSiteDocumentBlockingTest.BlockDocuments
 -CrossSiteDocumentBlockingIsolatedOriginTest.BlockDocumentsFromIsolatedOrigin
 -CrossSiteDocumentBlockingTest.RangeRequest
 

Comment 1 by nick@chromium.org, Dec 6 2017

Labels: -Type-Bug Type-Feature

Comment 2 by creis@chromium.org, Dec 6 2017

Cc: rdsmith@chromium.org csharrison@chromium.org mmenke@chromium.org
Yes, we've started discussing possible approaches for this.  Thanks for filing!
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6 2017

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

commit ccc58b7abd1cbbd8a79faaae9f941ae3a1bc3baa
Author: Nick Carter <nick@chromium.org>
Date: Wed Dec 06 20:32:07 2017

Network Service: clarify XSDB comment in network_content_browsertests.filter

Bug:  792546 ,  786505 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Id8ff7999115ca602c929b8c8e691121894ed94b7
Reviewed-on: https://chromium-review.googlesource.com/811744
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522179}
[modify] https://crrev.com/ccc58b7abd1cbbd8a79faaae9f941ae3a1bc3baa/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Comment 4 by creis@chromium.org, Dec 13 2017

Blocking: -786505

Comment 5 by creis@chromium.org, Dec 13 2017

Blocking: 268640

Comment 6 by jam@chromium.org, Jan 10 2018

Blocking: 598073
Some quick&dirty notes from a meeting between jam@, nick@ and me:

- Most of XSDB code will have to be refactored to enable sharing between 1) the current ResourceHandler-based implementation and 2) the new implementation for the NetworkService.  jam@ points out that other similarly shared pieces of code have been moved into //services/network (FWIW, there is a precedent for //content DEPS directly into non-public pieces of //services/network;  OTOH, maybe we should put the common code under //services/network/public/cpp).

- URLLoader::DidRead in //services/network/url_loader.cc looks like a good place for hooking up the new XSDB implementation.  Response is not sent to the renderer until URLLoader::SendResponseToClient.  XSDB can probably postpone this as needed (currently this is gated only on generic MIME sniffing) and when blocking 1) strip the headers and 2) replace |consumer_handle_| with a new, empty body (from a new, empty mojo::DataPipe?).

- Keeping XSDB turned off for non-isolated sites, can be done by having a |enable_xsdb_| flag in NetworkContext in services/network/public/mojom/network_service.mojom (that flag would be somehow passed or exposed to URLLoaderFactory and URLLoader).  The same flag can be used to avoid XSDB for requests not initiated by the renderer process.

- Things that today are computed at runtime, might have to be computed when constructing the NetworkContext and/or URLLoaderFactoryHow (e.g. which origins XSDB should block from content scripts or link doctor - see ShouldBypassDocumentBlocking)

- jam@ would ideally want to see the Network Service reimplementation of XSDB done by the end of Q1

- nick@ wonders if we need extra code to prevent downloads from being sent via renderer?

Comment 8 by mmenke@chromium.org, Feb 14 2018

My feeling is that if we expect consumers to use code after the network
service has shipped and we've removed the legacy path, we should put code
in services/network/public/cpp.  If it's destined to become private network
service-only code, it should be in services/network, though I'm not sure
this view is universally held.

I wonder if we could merge it with the mime sniffing URLLoader already
does, though that may make using the same logic in the legacy and network
service paths more complicated.
Cc: -rdsmith@chromium.org
Owner: lukasza@chromium.org
Status: Started (was: Available)
FWIW, I've started working on a series of CLs to reimplement XSDB/CORB in the network service.  I am making good progress, although I don't yet have implementation that works an end-to-end.

RE: #c8

XSDB/CORB's core logic/implementation is destined to become private network service-only code - I'll be putting it into services/network.  In the long-term the only thing sticking out of services/network/public/cpp will be a knob to turn XSDB/CORB on and off.  In the short-term we might also need to make a XSDB/CORB exception for chrome-extension:// initiators (until content scripts have a separate URLLoaderFactory that either ignores XSDB/CORB or only disables it for origins declared in extension manifest v3 as XHR-able from content scripts).


Project Member

Comment 11 by bugdroid1@chromium.org, Feb 26 2018

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

commit 702e84f69e4d6741867d4ab4465c49a67fde601f
Author: Nick Carter <nick@chromium.org>
Date: Mon Feb 26 23:08:33 2018

Remove LinkDoctorBaseURL special case for cross origin read blocking.

This special case would have been hard to port to the network service.
It's easier to just force the net_error_helper to issue a CORS-enabled
request, which https://www.googleapis.com/rpc readily understands.

A browsertest is added to explicitly test that CORB is applied to
subresources loaded by error pages. Currently this test fails; we'll
fix that in a separate CL.

Manual testing: in an official/branded Chrome build, navigate to
"http://blog.thestranger.com" and on the resulting DNS error page,
observe a suggested correction link (of "http://thestranger.com/blog").
Repeat these steps with a chrome://net-internals trace running, and
observe a request to "https://www.googleapis.com/rpc" that has the
"Origin: null" request header, and which includes an
"Access-Control-Allow-Origin: *" header in the response.

BUG= 814913 , 792546 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib7434bd52a27909dd67c5e9a867db1dab7090d59
Reviewed-on: https://chromium-review.googlesource.com/927561
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539309}
[modify] https://crrev.com/702e84f69e4d6741867d4ab4465c49a67fde601f/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/702e84f69e4d6741867d4ab4465c49a67fde601f/chrome/renderer/net/net_error_helper.cc
[modify] https://crrev.com/702e84f69e4d6741867d4ab4465c49a67fde601f/chrome/test/data/mock-link-doctor.json.mock-http-headers
[modify] https://crrev.com/702e84f69e4d6741867d4ab4465c49a67fde601f/content/browser/loader/cross_site_document_blocking_browsertest.cc
[modify] https://crrev.com/702e84f69e4d6741867d4ab4465c49a67fde601f/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Labels: -Pri-1 Pri-2
I'll continue working on this, but I am not treating this as P1.  Please shout if you think I should.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 8 2018

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

commit a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Thu Mar 08 00:59:55 2018

Move core CORB logic into //services/network.

This CL moves the core CORB logic (figuring out if a given response
should be CORB-protected based on 1) response metadata like initiator,
MIME type, CORS headers and 2) sniffing the first bytes of the response)
from //content/common into //services/network.

As part of the move, this CL makes some extra, opportunistic changes:
- Renames the moved files (and the moved class) to match the name agreed
  upon in the explainer
  (s/CrossSiteDocumentClassifier/CrossOriginReadBlocking)
- Also moves the explainer doc to //services/network
- Tweaks the old CrossSiteDocumentMimeType enum so that
  - It is nested in CrossOriginReadBlocking::MimeType
  - The enum values have shorter names

In the long-term, the core CORB logic should only be needed within
services/network.  In the short-term, this logic has the following
consumers:

- content/browser/loader/cross_site_document_resource_handler.cc
  (integrates CORB into the old, pre-network-service,
  ResourceHandler-based world;  this can go away after the network
  service ships)

- content/renderer/loader/site_isolation_stats_gatherer.cc
  (gathers UMA that help decide what responses can be blocked
  by CORB with acceptably low risk of breaking existing websites;
  this can go away after CORB ships)

- WIP( https://crbug.com/792546 ): services/network/url_loader.cc
  (this will integrate CORB into the new, network-service world).

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib8fe412b1f807d23e41700b7f7090cd4b77c1783
Reviewed-on: https://chromium-review.googlesource.com/929446
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541661}
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/browser/loader/cross_origin_read_blocking_explainer.md
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/browser/loader/cross_site_document_resource_handler_unittest.cc
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/common/BUILD.gn
[delete] https://crrev.com/8cfa58712d2207ba3926778f5045dab1b95ce1b2/content/common/cross_site_document_classifier_unittest.cc
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/renderer/loader/site_isolation_stats_gatherer.cc
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/renderer/loader/site_isolation_stats_gatherer.h
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/content/test/BUILD.gn
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/services/network/BUILD.gn
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/services/network/OWNERS
[rename] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/services/network/cross_origin_read_blocking.cc
[rename] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/services/network/cross_origin_read_blocking.h
[add] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/services/network/cross_origin_read_blocking_explainer.md
[add] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/services/network/cross_origin_read_blocking_unittest.cc
[modify] https://crrev.com/a9a7e8833ab48fc90dd91c3e0cfaec68abc56b05/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/README.md

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 12 2018

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

commit 76916d1b9f68238bcb2b418c1fffd3f66df09278
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon Mar 12 22:58:44 2018

Refactoring: Move mime type calculations closer to the point of 1st use.

This CL moves the call to
  canonical_mime_type_ =
      network::CrossOriginReadBlocking::GetCanonicalMimeType(mime_type);
closer to the point where the value of the |canonical_mime_type_| field
is needed for the 1st time.

The move helps with the following:
- Makes unit tests more robust against shuffling of chucks inside
  ShouldBlockBasedOnHeaders (some shuffling will be needed when
  moving some checks into //services/network).
- Makes the code of ShouldBlockBasedOnHeaders closer to the promise to
  perform less expensive checks first (the GetCanonicalMimeType has
  medium cost - it has to compare the mime type against multiple
  hardcoded strings).

The move necessitates some small follow-up tweaks in unit tests.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib21add69443dd0748aafd57da89a3cc16ffcdec1
Reviewed-on: https://chromium-review.googlesource.com/957804
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542643}
[modify] https://crrev.com/76916d1b9f68238bcb2b418c1fffd3f66df09278/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/76916d1b9f68238bcb2b418c1fffd3f66df09278/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/76916d1b9f68238bcb2b418c1fffd3f66df09278/content/browser/loader/cross_site_document_resource_handler_unittest.cc
[modify] https://crrev.com/76916d1b9f68238bcb2b418c1fffd3f66df09278/services/network/cross_origin_read_blocking.h

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 13 2018

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

commit 05f356ec8439626a8abf66b926cb2e191c2ebe20
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Mar 13 20:26:11 2018

Refactor ContentBrowserClient API exempting content scripts from CORB.

//services/network won't be able to directly use ContentBrowserClient,
so this CL refactors CORB-related parts of ContentBrowserClient in such
a way that the data they return in a browser-process can be easily
passed to the network service process.

Specifically, this CL changes the following ContentBrowserClient API:

  virtual bool ShouldBypassDocumentBlocking(const url::Origin& initiator,
                                            const GURL& url,
                                            ResourceType resource_type);

into:

  virtual const char* GetInitatorSchemeBypassingDocumentBlocking();

Bug:  792546 
Change-Id: I39d88a97ff24d776319b2687837838fa594054c5
Tbr: rdevlin.cronin@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/958041
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542897}
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/chrome/browser/extensions/chrome_content_browser_client_extensions_part.h
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/05f356ec8439626a8abf66b926cb2e191c2ebe20/content/public/browser/content_browser_client.h

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 30 2018

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

commit 42f5edc8364f151c80c185137f65601b829f4f28
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Mar 30 17:14:01 2018

Renaming CrossOriginReadBlocking::Result enum to SniffingResult.

The rename will help accomodate a separate BlockingDecision enum that
I plan to introduce in a follow-up CL.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib40573a8f2247223cfda4503c5a9bf126034bbff
Reviewed-on: https://chromium-review.googlesource.com/986933
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547194}
[modify] https://crrev.com/42f5edc8364f151c80c185137f65601b829f4f28/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/42f5edc8364f151c80c185137f65601b829f4f28/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/42f5edc8364f151c80c185137f65601b829f4f28/services/network/cross_origin_read_blocking.h
[modify] https://crrev.com/42f5edc8364f151c80c185137f65601b829f4f28/services/network/cross_origin_read_blocking_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 18 2018

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

commit 2c53e865303f5395de577a16523f9a8b10a9ac0c
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Apr 18 20:51:58 2018

CORB: Move code for processing response headers into //services/network.

This CL just moves some Cross-Origin Read-Blocking (CORB) code into
//service/network.  No intended behavior change.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I8d6ccc6a2bd6c673edf6b127868b2d0868bed4c4
Reviewed-on: https://chromium-review.googlesource.com/1012746
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551817}
[modify] https://crrev.com/2c53e865303f5395de577a16523f9a8b10a9ac0c/content/browser/loader/cross_site_document_blocking_browsertest.cc
[modify] https://crrev.com/2c53e865303f5395de577a16523f9a8b10a9ac0c/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/2c53e865303f5395de577a16523f9a8b10a9ac0c/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/2c53e865303f5395de577a16523f9a8b10a9ac0c/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/2c53e865303f5395de577a16523f9a8b10a9ac0c/services/network/cross_origin_read_blocking.h

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 20 2018

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

commit 5550e13d4aa324ee0337a1872a90435c9cb28a9c
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Apr 20 18:38:58 2018

CORB: Move ShouldBlockBasedOnHeaders into //services/network.

This CL moves ShouldBlockBasedOnHeaders logic for Cross-Origin Read
Blocking (CORB) into //services/network.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I37ddaa72af7c2f9e80370f5d8dd49bd907c3d831
Reviewed-on: https://chromium-review.googlesource.com/1013119
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552406}
[modify] https://crrev.com/5550e13d4aa324ee0337a1872a90435c9cb28a9c/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/5550e13d4aa324ee0337a1872a90435c9cb28a9c/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/5550e13d4aa324ee0337a1872a90435c9cb28a9c/content/browser/loader/cross_site_document_resource_handler_unittest.cc
[modify] https://crrev.com/5550e13d4aa324ee0337a1872a90435c9cb28a9c/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/5550e13d4aa324ee0337a1872a90435c9cb28a9c/services/network/cross_origin_read_blocking.h

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 27 2018

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

commit 6749ebad82d79d05a43d77bacf6b8491d4782560
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Apr 27 00:09:55 2018

CORB: Move sniffer selection logic into //services/network.

This CL moves into //services/network the CORB logic that selects one or more
sniffers: HTML, XML, JSON and/or JSON security prefix sniffer.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I38e12af18e6a937b4c238efae2688d85426b885c
Reviewed-on: https://chromium-review.googlesource.com/1022956
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554205}
[modify] https://crrev.com/6749ebad82d79d05a43d77bacf6b8491d4782560/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/6749ebad82d79d05a43d77bacf6b8491d4782560/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/6749ebad82d79d05a43d77bacf6b8491d4782560/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/6749ebad82d79d05a43d77bacf6b8491d4782560/services/network/cross_origin_read_blocking.h

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 27 2018

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

commit 67e6fceed93e33161aa6c63af183424594ae688f
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Apr 27 04:57:31 2018

CORB: Some methods can be encapsulated and only exposed to unit tests.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib8712b436e3099ad3e2379ce1835648f19fb380e
Reviewed-on: https://chromium-review.googlesource.com/1022975
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554309}
[modify] https://crrev.com/67e6fceed93e33161aa6c63af183424594ae688f/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/67e6fceed93e33161aa6c63af183424594ae688f/services/network/cross_origin_read_blocking.h

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 27 2018

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

commit 48dc32f3ce15eb65a3f2cecffe6e4df1dae460fa
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Apr 27 13:17:32 2018

Move part of CORB-related UMA code into //services/network.

This CL moves part of CORB-related UMA code into //services/network.
Only the metrics related to content::ResourceType have not been moved
and remain only logged from //content/browser/loader code.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ic649b587aafa679a0fef98eea5e5d6eb297bc698
Reviewed-on: https://chromium-review.googlesource.com/1028841
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554368}
[modify] https://crrev.com/48dc32f3ce15eb65a3f2cecffe6e4df1dae460fa/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/48dc32f3ce15eb65a3f2cecffe6e4df1dae460fa/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/48dc32f3ce15eb65a3f2cecffe6e4df1dae460fa/content/browser/loader/cross_site_document_resource_handler_unittest.cc
[modify] https://crrev.com/48dc32f3ce15eb65a3f2cecffe6e4df1dae460fa/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/48dc32f3ce15eb65a3f2cecffe6e4df1dae460fa/services/network/cross_origin_read_blocking.h

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 27 2018

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

commit 1cc6de04d86c67235243fb793f5ba74f439eb334
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Apr 27 18:07:39 2018

Mark newly enabled (r554192) CORB layout tests as failing with network service.

TBR=lukasza@chromium.org

Bug:  792546 
Change-Id: I5142b81d26f7221b8fe03d70ed3b14ffeaa461d4
Reviewed-on: https://chromium-review.googlesource.com/1032881
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554420}
[modify] https://crrev.com/1cc6de04d86c67235243fb793f5ba74f439eb334/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 23 by dxie@chromium.org, May 17 2018

Labels: -Pri-2 Proj-Servicification-Canary OS-All Pri-1

Comment 24 by dxie@chromium.org, May 18 2018

Labels: -OS-All OS-Android
Project Member

Comment 25 by bugdroid1@chromium.org, May 18 2018

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

commit 0b05f082f326fc8c5c628b60c9f21e501a5c2aa4
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri May 18 23:04:00 2018

Introduce URLLoaderFactoryParams.

This CL introduces URLLoaderFactoryParams which encapsulates all
input parameters of NetworkContext::CreateURLLoaderFactory.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5ba5348aa49c2de958ecef8ab34a0103ec98aa4f
Reviewed-on: https://chromium-review.googlesource.com/1064881
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560087}
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/android_webview/browser/aw_safe_browsing_ui_manager.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/chrome/browser/safe_browsing/download_protection/download_feedback_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/chrome/browser/safe_browsing/download_protection/two_phase_uploader_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/chrome/browser/safe_browsing/safe_browsing_service.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/chromecast/browser/network_context_manager.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/components/net_log/net_export_file_writer_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/components/safe_browsing/browser/safe_browsing_network_context.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/browser/loader/navigation_url_loader_impl_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/browser/storage_partition_impl_browsertest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/browser/url_loader_factory_getter.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/ios/web/browser_state.mm
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/cors/preflight_controller_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/network_context.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/network_context.h
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/network_context_cert_transparency_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/network_context_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/network_service_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/public/cpp/simple_url_loader_unittest.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/test/test_network_context.h
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/url_loader.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/url_loader.h
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/url_loader_factory.cc
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/url_loader_factory.h
[modify] https://crrev.com/0b05f082f326fc8c5c628b60c9f21e501a5c2aa4/services/network/url_loader_unittest.cc

Blockedon: 846334
Blockedon: 846339
Blockedon: 846341
Blockedon: 846346
Blockedon: 846352
Project Member

Comment 31 by bugdroid1@chromium.org, May 25 2018

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

commit a155a25c322bfbd136312607528b68a1f8326a74
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri May 25 16:30:56 2018

Use CrossOriginReadBlocking from URLLoader.

Bug:  792546 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib2ed466f5b99ac4c6000eb2c4012e7feef186a31
Reviewed-on: https://chromium-review.googlesource.com/1033535
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561905}
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/browser/loader/cross_site_document_blocking_browsertest.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/public/browser/site_isolation_policy.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/public/browser/site_isolation_policy.h
[add] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/content/test/data/site_isolation/html-prefix.txt
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/services/network/cross_origin_read_blocking.h
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/services/network/url_loader.cc
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/services/network/url_loader.h
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
[modify] https://crrev.com/a155a25c322bfbd136312607528b68a1f8326a74/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Blockedon: -846339

Comment 33 by creis@chromium.org, Jun 26 2018

Blockedon: 853239
Blockedon: 846339

Comment 35 by dxie@google.com, Jun 27 2018

Cc: -falken@chromium.org
nasko, I need your help on this bug. We are trying to ship network service to canary in M69 and we need this bug addressed. Can you assign someone to look at this or alternatively do you think we can ship to canary without this?

Comment 36 by nasko@chromium.org, Jun 27 2018

creis@ and jam@ have been working more closely with lukasza@ on CORB with Network Service, so I defer to them on what the status of this is.

Comment 37 by dxie@chromium.org, Jun 27 2018

Cc: -csharrison@chromium.org falken@chromium.org
Labels: -Proj-Servicification-Canary Proj-Servicification
i spoke to creis@ and seems reasonable to ship without this to canary. Hopefully lukasza@ will take care of it when he's back.

Comment 38 by dxie@google.com, Jun 27 2018

Issue 852191 has been merged into this issue.

Comment 39 by creis@chromium.org, Jun 27 2018

To be clear, we will need help from network service folks on questions about how to fix issue 853239 (how to track check CORS mode on a plugin fetch) and issue 846346 (separate URLLoaderFactory for extension content scripts).

I'm not opposed to starting Canary trials without fixes for these, but they should be blockers for Beta trials.

(Also, sounds like comment 38 was a typo.  Issue 852191 was moved to be a dupe of  issue 789670  instead.)

Comment 40 by jam@chromium.org, Jun 27 2018

Status: Fixed (was: Started)
I have a patch out for bug 853239, and bug 846346 isn't specific to network service (I left comment there).

I'll mark this bug closed in the meantime. Thanks a lot Lukasz!

Comment 41 by jam@chromium.org, Jun 27 2018

Labels: -Proj-Servicification Proj-Servicification-Canary

Sign in to add a comment