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

Issue 759230 link

Starred by 2 users

Issue metadata

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

Blocked on:
issue 848510
issue 848831

Blocking:
issue 598073



Sign in to add a comment

Handle file:// URLs outside of the network service

Project Member Reported by jam@chromium.org, Aug 25 2017

Issue description

Tom had the idea that we shouldn't handle file:// URLs inside the network service. We can then only send the renderer a URLLoaderFactory for the file scheme if the frame id displaying a file.

See also bug 719594 where we do this for webui. I think if we do this then this would be one less usage of CPSP that we need.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 26 2017

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

commit d35e92ac3a05c531a92b35374208f3fd25fbcd00
Author: Reilly Grant <reillyg@chromium.org>
Date: Sat Aug 26 00:43:13 2017

Triage content_browsertest failures in sync XHRs with network service

ResourceDispatcherHostBrowserTest.SyncXMLHttpRequest_Cancelled is
expected to fail because it relies on behavior of
ResourceDispatcherHostImpl which will be removed by the Network Service
but will be similiar to a process crash.

BrowserSideNavigationBrowserDisableWebSecurityTest.ValidateBaseUrlForDataUrl
fails because file:// URLs are currently handled by the network service
and the cross-origin request is not blocked which is tracked by issue
759230.

ResourceDispatcherHostBrowserTest.SyncXMLHttpRequest now passes.

TBR=jam@chromium.org

Bug:  754827 , 759230 
Change-Id: I856542f6f2ef4d23173f0c74ff76e290cbe0d905
Reviewed-on: https://chromium-review.googlesource.com/636467
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497614}
[modify] https://crrev.com/d35e92ac3a05c531a92b35374208f3fd25fbcd00/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Comment 2 by dougt@chromium.org, Oct 16 2017

Owner: roc...@chromium.org
Status: Started (was: Untriaged)

Comment 3 by dougt@chromium.org, Oct 20 2017

Components: Internals>Network>Service
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25 2017

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

commit 4878545a4f99287f8f9fc5e76fc07e0c1575536a
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Oct 25 19:33:14 2017

Convert FrameMsg_CommitNavigation to mojom

Uses a Channel-associated interface to carry a new message mirroring
the contents of and replacing the legacy FrameMsg_CommitNavigation
IPC message.

This is a precursor to also sending a map of allowed protocol handler
URLLoaderFactoryPtrs to frames upon navigation, which would be much
more cumbersome using legacy IPC. This map will support things like
file URLs and any embedder-defined schemes (e.g. chrome-extension).

Bug:  759230 , 721414 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I9a679e58f6424c95029ce1ac63cb9fca7f7644e8
Reviewed-on: https://chromium-review.googlesource.com/736855
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511554}
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/components/subresource_filter/content/common/subresource_filter_messages.h
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/common/frame.mojom
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/common/frame_messages.h
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/common/navigation_params.h
[add] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/common/navigation_params.typemap
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/common/typemaps.gni
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/renderer/render_frame_impl.h
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/renderer/renderer_webapplicationcachehost_impl.cc
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/test/test_render_frame.cc
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/test/test_render_frame_host.cc
[modify] https://crrev.com/4878545a4f99287f8f9fc5e76fc07e0c1575536a/content/test/test_render_frame_host.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 30 2017

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

commit bba21f276796f75e0d427fe3a224088e48f375e6
Author: Ken Rockot <rockot@chromium.org>
Date: Mon Oct 30 09:25:40 2017

Always send subresource ULF on navigation

This changes the Network Service-enabled browser-side navigation path
to always send a default subresource URLLoaderFactory to the render
frame on navigation commit.

In addition to sending a default ULF, we also send one for blobs.
Ultimately the idea here is to have the browser, at navigation time,
send a frame everything it will need to request any accessible
subresources, making it easier to reason about what subresource
capabilities any given frame has.

This set of capabilities is encapsulated by a new
URLLoaderFactoryBundle type, which can be serialized over mojom and is
now passed in new-document navigation commit messages.

Follow-up usage of URLLoaderFactoryBundle will support loading things
like file:// and chrome-extension:// subresources when appropriate.

Note that this does not yet change how workers acquire ULFs for their
own resource requests.

Bug:  721414 , 759230 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I47b0897c7c012292633d5f08c874a18cc0fb1dcb
Reviewed-on: https://chromium-review.googlesource.com/740904
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512459}
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/BUILD.gn
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/frame.mojom
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/typemaps.gni
[add] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/url_loader_factory_bundle.cc
[add] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/url_loader_factory_bundle.h
[add] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/url_loader_factory_bundle.mojom
[add] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/url_loader_factory_bundle.typemap
[add] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/url_loader_factory_bundle_struct_traits.cc
[add] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/renderer/render_frame_impl.h
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/test/test_render_frame.cc
[modify] https://crrev.com/bba21f276796f75e0d427fe3a224088e48f375e6/content/test/test_render_frame_host.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 30 2017

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

commit 02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Mon Oct 30 15:05:11 2017

Revert "Always send subresource ULF on navigation"

This reverts commit bba21f276796f75e0d427fe3a224088e48f375e6.

Suspected to cause lots of test failures on Mojo Linux bot.
https://build.chromium.org/p/chromium.fyi/builders/Mojo%20Linux/builds/6676

>> Always send subresource ULF on navigation
>>
>> This changes the Network Service-enabled browser-side navigation path
>> to always send a default subresource URLLoaderFactory to the render
>> frame on navigation commit.
>>
>> In addition to sending a default ULF, we also send one for blobs.
>> Ultimately the idea here is to have the browser, at navigation time,
>> send a frame everything it will need to request any accessible
>> subresources, making it easier to reason about what subresource
>> capabilities any given frame has.
>>
>> This set of capabilities is encapsulated by a new
>> URLLoaderFactoryBundle type, which can be serialized over mojom and is
>> now passed in new-document navigation commit messages.
>>
>> Follow-up usage of URLLoaderFactoryBundle will support loading things
>> like file:// and chrome-extension:// subresources when appropriate.
>>
>> Note that this does not yet change how workers acquire ULFs for their
>> own resource requests.
>>
>> Bug:  721414 , 759230 
>> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
>> Change-Id: I47b0897c7c012292633d5f08c874a18cc0fb1dcb
>> Reviewed-on: https://chromium-review.googlesource.com/740904

TBR=jam@chromium.org, rockot@chromium.org, dcheng@chromium.org

Bug:  721414 ,  759230 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I7e7a0d5f57033369a8061cf2c189ef7b43b0ceb1
Reviewed-on: https://chromium-review.googlesource.com/743881
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512495}
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/common/BUILD.gn
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/common/frame.mojom
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/common/typemaps.gni
[delete] https://crrev.com/dd64a192d8b8a180fb40f389f4ecf8680467e968/content/common/url_loader_factory_bundle.cc
[delete] https://crrev.com/dd64a192d8b8a180fb40f389f4ecf8680467e968/content/common/url_loader_factory_bundle.h
[delete] https://crrev.com/dd64a192d8b8a180fb40f389f4ecf8680467e968/content/common/url_loader_factory_bundle.mojom
[delete] https://crrev.com/dd64a192d8b8a180fb40f389f4ecf8680467e968/content/common/url_loader_factory_bundle.typemap
[delete] https://crrev.com/dd64a192d8b8a180fb40f389f4ecf8680467e968/content/common/url_loader_factory_bundle_struct_traits.cc
[delete] https://crrev.com/dd64a192d8b8a180fb40f389f4ecf8680467e968/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/renderer/render_frame_impl.h
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/test/test_render_frame.cc
[modify] https://crrev.com/02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9/content/test/test_render_frame_host.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 31 2017

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

commit e60ebbd1ae1e543a73b177fc097cadbf568d2c43
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Oct 31 01:50:59 2017

Reland "Always send subresource ULF on navigation"

This is a reland of bba21f276796f75e0d427fe3a224088e48f375e6
Original change's description:
> Always send subresource ULF on navigation
> 
> This changes the Network Service-enabled browser-side navigation path
> to always send a default subresource URLLoaderFactory to the render
> frame on navigation commit.
> 
> In addition to sending a default ULF, we also send one for blobs.
> Ultimately the idea here is to have the browser, at navigation time,
> send a frame everything it will need to request any accessible
> subresources, making it easier to reason about what subresource
> capabilities any given frame has.
> 
> This set of capabilities is encapsulated by a new
> URLLoaderFactoryBundle type, which can be serialized over mojom and is
> now passed in new-document navigation commit messages.
> 
> Follow-up usage of URLLoaderFactoryBundle will support loading things
> like file:// and chrome-extension:// subresources when appropriate.
> 
> Note that this does not yet change how workers acquire ULFs for their
> own resource requests.
> 
> Bug:  721414 , 759230 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: I47b0897c7c012292633d5f08c874a18cc0fb1dcb
> Reviewed-on: https://chromium-review.googlesource.com/740904
> Commit-Queue: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#512459}

Bug:  721414 ,  759230 
Change-Id: Ie13bcd97b7a5af8dbe4263a70239ba124205da15
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation

TBR=dcheng@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ie13bcd97b7a5af8dbe4263a70239ba124205da15
Reviewed-on: https://chromium-review.googlesource.com/744222
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512706}
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/BUILD.gn
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/frame.mojom
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/typemaps.gni
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/url_loader_factory_bundle.cc
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/url_loader_factory_bundle.h
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/url_loader_factory_bundle.mojom
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/url_loader_factory_bundle.typemap
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/url_loader_factory_bundle_struct_traits.cc
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/renderer/render_frame_impl.h
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/test/test_render_frame.cc
[modify] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/content/test/test_render_frame_host.cc
[add] https://crrev.com/e60ebbd1ae1e543a73b177fc097cadbf568d2c43/third_party/WebKit/LayoutTests/fast/dom/Window/window-open-subresource-loading.html

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 31 2017

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

commit f409847329e8c94d3912d3c54e0121a06e778d0f
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Oct 31 08:27:28 2017

Revert "Reland "Always send subresource ULF on navigation""

This reverts commit e60ebbd1ae1e543a73b177fc097cadbf568d2c43.

Reason for revert: Caused ~60 browser_tests and
content_browsertests to fail with the network service enabled.

Original change's description:
> Reland "Always send subresource ULF on navigation"
> 
> This is a reland of bba21f276796f75e0d427fe3a224088e48f375e6
> Original change's description:
> > Always send subresource ULF on navigation
> > 
> > This changes the Network Service-enabled browser-side navigation path
> > to always send a default subresource URLLoaderFactory to the render
> > frame on navigation commit.
> > 
> > In addition to sending a default ULF, we also send one for blobs.
> > Ultimately the idea here is to have the browser, at navigation time,
> > send a frame everything it will need to request any accessible
> > subresources, making it easier to reason about what subresource
> > capabilities any given frame has.
> > 
> > This set of capabilities is encapsulated by a new
> > URLLoaderFactoryBundle type, which can be serialized over mojom and is
> > now passed in new-document navigation commit messages.
> > 
> > Follow-up usage of URLLoaderFactoryBundle will support loading things
> > like file:// and chrome-extension:// subresources when appropriate.
> > 
> > Note that this does not yet change how workers acquire ULFs for their
> > own resource requests.
> > 
> > Bug:  721414 , 759230 
> > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> > Change-Id: I47b0897c7c012292633d5f08c874a18cc0fb1dcb
> > Reviewed-on: https://chromium-review.googlesource.com/740904
> > Commit-Queue: Ken Rockot <rockot@chromium.org>
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#512459}
> 
> Bug:  721414 ,  759230 
> Change-Id: Ie13bcd97b7a5af8dbe4263a70239ba124205da15
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> 
> TBR=dcheng@chromium.org
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: Ie13bcd97b7a5af8dbe4263a70239ba124205da15
> Reviewed-on: https://chromium-review.googlesource.com/744222
> Commit-Queue: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#512706}

TBR=dcheng@chromium.org,jam@chromium.org,rockot@chromium.org

Change-Id: Iee7d5a63f105ac4c15d8320668a33d66972820b1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  721414 ,  759230 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/746302
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512779}
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/common/BUILD.gn
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/common/frame.mojom
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/common/typemaps.gni
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/content/common/url_loader_factory_bundle.cc
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/content/common/url_loader_factory_bundle.h
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/content/common/url_loader_factory_bundle.mojom
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/content/common/url_loader_factory_bundle.typemap
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/content/common/url_loader_factory_bundle_struct_traits.cc
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/renderer/render_frame_impl.h
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/test/test_render_frame.cc
[modify] https://crrev.com/f409847329e8c94d3912d3c54e0121a06e778d0f/content/test/test_render_frame_host.cc
[delete] https://crrev.com/d548ffaf774d421879ceaf5fee3d72772385de33/third_party/WebKit/LayoutTests/fast/dom/Window/window-open-subresource-loading.html

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 2 2017

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

commit be87ab32c0b113d05c061b355abbe1e257f5474c
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Nov 02 19:40:23 2017

Reland "Always send subresource ULF on navigation"

This is a reland of bba21f276796f75e0d427fe3a224088e48f375e6

Reland fixes a few things:

 - JS navs are special-cased renderer-side: even if such navigations
   result in a new document, we reuse the existing
   URLLoaderFactoryBundle since the document is from the same origin.
 - Some test code now passes a valid empty URLLoaderFactoryBundle
   instead of nullopt to avoid violating checks that are not
   relevant to these tests.
 - https://chromium-review.googlesource.com/c/chromium/src/+/747808
   properly configures and disables some failing SSL tests when the
   Network Service is enabled.

The only remaining failure seen from the original landing is
SafeBrowsingTriggeredInterceptingBrowserTest.AbusiveMetadata, which
appears to be failing both with and without Network Service enabled,
and both with and without this CL applied.

Original change's description:
> Always send subresource ULF on navigation
>
> This changes the Network Service-enabled browser-side navigation path
> to always send a default subresource URLLoaderFactory to the render
> frame on navigation commit.
>
> In addition to sending a default ULF, we also send one for blobs.
> Ultimately the idea here is to have the browser, at navigation time,
> send a frame everything it will need to request any accessible
> subresources, making it easier to reason about what subresource
> capabilities any given frame has.
>
> This set of capabilities is encapsulated by a new
> URLLoaderFactoryBundle type, which can be serialized over mojom and is
> now passed in new-document navigation commit messages.
>
> Follow-up usage of URLLoaderFactoryBundle will support loading things
> like file:// and chrome-extension:// subresources when appropriate.
>
> Note that this does not yet change how workers acquire ULFs for their
> own resource requests.
>
> Bug:  721414 , 759230 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: I47b0897c7c012292633d5f08c874a18cc0fb1dcb
> Reviewed-on: https://chromium-review.googlesource.com/740904
> Commit-Queue: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#512459}

Bug:  721414 ,  759230 
Change-Id: I1964aa51f837171d2fd6defedb629fc75012c35b
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_mojo

TBR=dcheng@chromium.org
TBR=jam@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_mojo

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I1964aa51f837171d2fd6defedb629fc75012c35b
Reviewed-on: https://chromium-review.googlesource.com/748266
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513584}
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/BUILD.gn
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/frame.mojom
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/typemaps.gni
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/url_loader_factory_bundle.cc
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/url_loader_factory_bundle.h
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/url_loader_factory_bundle.mojom
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/url_loader_factory_bundle.typemap
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/url_loader_factory_bundle_struct_traits.cc
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/common/url_loader_factory_bundle_struct_traits.h
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/renderer/render_frame_impl.h
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/test/test_render_frame.cc
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/content/test/test_render_frame_host.cc
[modify] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[add] https://crrev.com/be87ab32c0b113d05c061b355abbe1e257f5474c/third_party/WebKit/LayoutTests/fast/dom/Window/window-open-subresource-loading.html

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 5 2017

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

commit 314714c736ac5966f73716a66c69321bc5f7fa81
Author: Ken Rockot <rockot@chromium.org>
Date: Sun Nov 05 23:36:24 2017

Support file:// URLs with Network Service enabled

Enables file:// URLs to work for both navigation and (where
appropriate) subresource requests when the Network Service is enabled,
without any dependency on the legacy net URL request path.

This is done by implementing a new URLLoaderFactory to handle file
requests and hooking it into both NavigationURLLoaderNetworkService
and an entry in any URLLoaderFactoryBundle sent to a frame being
navigated to a file:// resource.

Bug:  759230 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.linux:linux_mojo
Change-Id: I74052a320ee3db493fc2892481ae6a4b3ebcbb63
Reviewed-on: https://chromium-review.googlesource.com/742652
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514071}
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/chrome/browser/net/chrome_network_delegate.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/BUILD.gn
[add] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/file_url_loader_factory.cc
[add] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/file_url_loader_factory.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/browser/loader/navigation_url_loader_network_service.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/public/browser/content_browser_client.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/public/common/url_utils.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/content/public/common/url_utils.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/mojo/public/cpp/system/file_data_pipe_producer.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/mojo/public/cpp/system/file_data_pipe_producer.h
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/mojo/public/cpp/system/tests/file_data_pipe_producer_unittest.cc
[modify] https://crrev.com/314714c736ac5966f73716a66c69321bc5f7fa81/net/base/directory_lister.cc

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Status: Fixed (was: Started)

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

Owner: jcivelli@chromium.org
Status: Assigned (was: Fixed)
Assigning to Jay until we can remove the enabling of file URLs in NetworkContexts, i.e.
https://cs.chromium.org/search/?q="enable_file_url_support+%3D+true"&type=cs

This is causing
WebContentsImplBrowserTest.DownloadImage_Deny_FileImage
to fail

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

also looks like this is blocking BrowserSideNavigationBrowserDisableWebSecurityTest.ValidateBaseUrlForDataUrl
Status: Started (was: Assigned)
I have a CL in progress.
https://chromium-review.googlesource.com/c/chromium/src/+/1045313
Disabling file access in the network service broke some extension, download and worker tests. I fixed the extension and download cases, still need work for the worker case.

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

Labels: -Pri-2 Proj-Servicification-Canary OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Project Member

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

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

commit 3c1035553514e770022274c9c3b3bd207f3d515e
Author: Jay Civelli <jcivelli@google.com>
Date: Wed May 23 20:55:00 2018

Change worker browser tests to use HTTP instead of file URLs

Changing the worker browser tests to use HTTP URLs instead of file URLs
in preparation for disabling file access in the network service which
will cause these tests to fail.

Bug:  759230 
Change-Id: I0d7cccfe19698f8c273a04ee44d1b508aaff1149
Reviewed-on: https://chromium-review.googlesource.com/1069743
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561250}
[modify] https://crrev.com/3c1035553514e770022274c9c3b3bd207f3d515e/content/browser/shared_worker/worker_browsertest.cc

Blockedon: 848510
Blockedon: 848831
Project Member

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

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

commit 2578ebea37645d57d4f90f1f38aba89687b35742
Author: Jay Civelli <jcivelli@google.com>
Date: Tue Jun 05 18:44:31 2018

Disable default support for file URLs in the network service

Disabling default support for file URLs in the network service. It was
required for PAC script file retrieval wich was deprecated and disabled
when the network service is running in a previous CL.

This exposed some other cases where file URL support is required:
- extension background pages require file access when such permission is
  required
- downloads require file access (so downloading a local page works)

This also fixes the content browser test
WebContentsImplBrowserTest.DownloadImage_Deny_FileImage when the network
service is enabled.

Bug:  759230 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Id314bb74d77b06934af1b456f3beb0650b1f5e76
Reviewed-on: https://chromium-review.googlesource.com/1045313
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564596}
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/chrome/browser/net/profile_network_context_service.cc
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/content/browser/BUILD.gn
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/content/browser/download/download_manager_impl.cc
[add] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/content/browser/download/file_download_url_loader_factory_getter.cc
[add] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/content/browser/download/file_download_url_loader_factory_getter.h
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/2578ebea37645d57d4f90f1f38aba89687b35742/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

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

Status: Fixed (was: Started)
Thanks Jay!
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 2

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

commit 7f6604cb299d5f896ca43ba1bd55148f4e0bb41f
Author: Eric Roman <eroman@chromium.org>
Date: Tue Oct 02 22:50:02 2018

Account for byte range restriction in URLLoaderCompletionStatus sizes for file://.

Bug:  759230 
Change-Id: Ic94211a04bc97afd01097b099a42ad7d365f09cc
Reviewed-on: https://chromium-review.googlesource.com/c/1244297
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596021}
[modify] https://crrev.com/7f6604cb299d5f896ca43ba1bd55148f4e0bb41f/content/browser/file_url_loader_factory.cc

Sign in to add a comment