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

Issue 712913 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-Servicification


Sign in to add a comment

Switch URLLoaderFactory to be per frame instead of per-process

Project Member Reported by jam@chromium.org, Apr 19 2017

Issue description

Currently the renderer has one URLLoaderFactory. A lot of the tasks we've been discussing for network service effort depend on having it be-frame instead. That way if a page is using service worker or appcache, we can give just that frame and children a different URLLoaderFactory.
 

Comment 1 by jam@chromium.org, Apr 19 2017

Owner: yhirano@chromium.org
Yutaka: assigning to you as initial owner, thanks.
Components: Blink>Loader
Status: Assigned (was: Unconfirmed)

Comment 3 by kinuko@chromium.org, Apr 19 2017

Cc: toyoshim@chromium.org
+cc toyoshim@-san (And thanks Yutaka for taking this one!)
As the first step I added CHECKs checking route ID in web_url_loader_impl.cc. It looks route ID is not set in the service worker shadow page. kinuko@, can you take a look?

https://codereview.chromium.org/2831513002/#ps40001
https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/433518

Comment 5 by kinuko@chromium.org, Apr 19 2017

Yep I don't think we set that... let me talk to you offline about that.
Blockedon: 715640

Comment 7 by kinuko@chromium.org, Apr 27 2017

Blockedon: -715640

Comment 8 by kinuko@chromium.org, Apr 27 2017

Blocking: 715640

Comment 10 by jam@chromium.org, May 2 2017

Blocking: 715632 715673 717714
Project Member

Comment 14 by bugdroid1@chromium.org, May 11 2017

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

commit 6eae3c8d8753e442119f88ff2c5464cddcbe196f
Author: yhirano <yhirano@chromium.org>
Date: Thu May 11 09:28:52 2017

Have PingLoader use FetchContext::CreateURLLoader

...instead of using Platform::CreateURLLoader directly

BUG= 712913 

Review-Url: https://codereview.chromium.org/2868773002
Cr-Commit-Position: refs/heads/master@{#470890}

[modify] https://crrev.com/6eae3c8d8753e442119f88ff2c5464cddcbe196f/third_party/WebKit/Source/core/loader/PingLoader.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, May 11 2017

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

commit cecb6d2ec467966fcd43f0664f9820f526039765
Author: yhirano <yhirano@chromium.org>
Date: Thu May 11 11:59:03 2017

Stop having two URLLoaderFactories in RendererBlinkPlatformImpl

RendererBlinkPlatformImpl has one InterfacePtr<URLLoaderFactory> and one
AssociatedInterfacePtr<URLLoaderFactory> but only one instance is needed
at one time: They are used in the same way, but held differently because
of the type difference. This CL introduces a container type to hold them.

BUG= 712913 
R=kinuko@chromium.org, jam@chromium.org

Review-Url: https://codereview.chromium.org/2858503002
Cr-Commit-Position: refs/heads/master@{#470926}

[modify] https://crrev.com/cecb6d2ec467966fcd43f0664f9820f526039765/content/renderer/BUILD.gn
[add] https://crrev.com/cecb6d2ec467966fcd43f0664f9820f526039765/content/renderer/possibly_associated_interface_ptr.h
[modify] https://crrev.com/cecb6d2ec467966fcd43f0664f9820f526039765/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/cecb6d2ec467966fcd43f0664f9820f526039765/content/renderer/renderer_blink_platform_impl.h

Project Member

Comment 16 by bugdroid1@chromium.org, May 12 2017

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

commit 75bad4f7e1b204142c189666bf8d268a3a36d6aa
Author: yhirano <yhirano@chromium.org>
Date: Fri May 12 03:57:52 2017

Have ResourceLoader use FetchContext::CreateURLLoader

...instead of using Platform::CreateURLLoader directly

BUG= 712913 

Review-Url: https://codereview.chromium.org/2872403003
Cr-Commit-Position: refs/heads/master@{#471214}

[modify] https://crrev.com/75bad4f7e1b204142c189666bf8d268a3a36d6aa/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp
[modify] https://crrev.com/75bad4f7e1b204142c189666bf8d268a3a36d6aa/third_party/WebKit/Source/platform/loader/testing/MockFetchContext.h

Project Member

Comment 17 by bugdroid1@chromium.org, May 18 2017

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

commit 9f1c33a3a158a560bbddf899bfc4b09f8d4ef009
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Thu May 18 08:41:55 2017

Introduce WebFrameClient::CreateURLLoader

This CL introduces WebFrameClient::CreateURLLoader. Currently many
WebFrameClient implementations use blink::Platform::CreateURLLoader, but
they will be replaced with more appropriate code in the future.

Bug:  712913 
Change-Id: I1df0da19af04d49666356ddb72fb454e07cca116
Reviewed-on: https://chromium-review.googlesource.com/504567
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472743}
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/components/plugins/renderer/webview_plugin.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/components/printing/renderer/print_web_view_helper.cc
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/content/renderer/render_frame_impl.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/web/LocalFrameClientImpl.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/web/WebSharedWorkerImpl.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/Source/web/tests/FrameTestHelpers.h
[modify] https://crrev.com/9f1c33a3a158a560bbddf899bfc4b09f8d4ef009/third_party/WebKit/public/web/WebFrameClient.h

Project Member

Comment 18 by bugdroid1@chromium.org, Jun 29 2017

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

commit b29773986fb56b3561cab32da7a10e108d1bb4e9
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Thu Jun 29 00:26:49 2017

Pass request and task runner to WebURLLoader construction function

This CL adds WebURLRequest and SingleThreadTaskRunner params to
Platfrom::CreateURLLoader to enable further refactoring.

Bug:  712913 ,  730338 
Change-Id: I263905dadd5505135eb171c3920d92d1b335b578
Reviewed-on: https://chromium-review.googlesource.com/552417
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483220}
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/components/plugins/renderer/webview_plugin.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/components/printing/renderer/print_web_view_helper.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/ppapi_plugin/ppapi_blink_platform_impl.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/ppapi_plugin/ppapi_blink_platform_impl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/renderer/fetchers/resource_fetcher_impl.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/renderer/render_frame_impl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/renderer/renderer_blink_platform_impl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/test/test_blink_web_unit_test_support.cc
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/content/test/test_blink_web_unit_test_support.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/frame/FrameTestHelpers.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/platform/loader/testing/FetchTestingPlatformSupport.cpp
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/platform/loader/testing/FetchTestingPlatformSupport.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/platform/loader/testing/MockFetchContext.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.cpp
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/web/LocalFrameClientImpl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/public/platform/Platform.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/public/web/WebFrameClient.h
[modify] https://crrev.com/b29773986fb56b3561cab32da7a10e108d1bb4e9/third_party/WebKit/public/web/WebLocalFrame.h

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 29 2017

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

commit cf3713102fb05816d3e8385a122b6ce46737992b
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Thu Jun 29 06:49:19 2017

Remove WebURLLoader::SetLoadingTaskRunner

This CL adds a constructor parameter instead.

Bug:  712913 
Change-Id: I9f8b8c5ab7e35301652f51fd81a03a0879851cbf
Reviewed-on: https://chromium-review.googlesource.com/554198
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483311}
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/child/web_url_loader_impl.h
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/child/web_url_loader_impl_unittest.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/renderer/service_worker/service_worker_fetch_context_impl.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/renderer/service_worker/service_worker_fetch_context_impl.h
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/renderer/service_worker/worker_fetch_context_impl.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/renderer/service_worker/worker_fetch_context_impl.h
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/content/test/test_blink_web_unit_test_support.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/Source/platform/BUILD.gn
[delete] https://crrev.com/a082cb81730396976290b610df68537ab979bbca/third_party/WebKit/Source/platform/WebURLLoader.cpp
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/Source/platform/loader/testing/MockFetchContext.h
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/Source/platform/testing/weburl_loader_mock.cc
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/Source/platform/testing/weburl_loader_mock.h
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/public/platform/WebURLLoader.h
[modify] https://crrev.com/cf3713102fb05816d3e8385a122b6ce46737992b/third_party/WebKit/public/platform/WebWorkerFetchContext.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 5 2017

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

commit 652fc568ad930f51c62da7f06ba187ea3e37dd00
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Wed Jul 05 08:55:07 2017

Use NetworkService's URLLoaderFactory if per-frame custom one is not given

As the custom one is not always given.

This fixes bunch of network_service_content_browsertests failures that have started to fail after
https://chromium-review.googlesource.com/c/554399/:
https://build.chromium.org/p/chromium.fyi/builders/Mojo%20Linux/builds/2596

Bug:  712913 
Change-Id: Ic09b7de05456366f81b35b70f525e875c3af90b2
Reviewed-on: https://chromium-review.googlesource.com/559289
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484221}
[modify] https://crrev.com/652fc568ad930f51c62da7f06ba187ea3e37dd00/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/652fc568ad930f51c62da7f06ba187ea3e37dd00/content/renderer/render_frame_impl.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 7 2017

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

commit 98be3272d43c5dbde31ef3ef139a952a0d40f4ae
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Mon Aug 07 17:55:39 2017

Turn OutOfBlinkCORS blink runtime enabled flag into chrome feature

URLLoaderFactory creation in RendererBlinkPlatformImpl::CreateURLLoader
should not use request information.

Bug:  712913 
Change-Id: I84a5607899a8b34fedd7974156a870ff85c8c886
Reviewed-on: https://chromium-review.googlesource.com/597479
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492355}
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/chrome/browser/about_flags.cc
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/content/child/runtime_features.cc
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/content/public/common/content_features.cc
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/content/public/common/content_features.h
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/third_party/WebKit/LayoutTests/VirtualTestSuites
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/third_party/WebKit/Source/platform/exported/WebRuntimeFeatures.cpp
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/third_party/WebKit/Source/platform/exported/WebURLRequest.cpp
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/third_party/WebKit/public/platform/WebRuntimeFeatures.h
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/third_party/WebKit/public/platform/WebURLRequest.h
[modify] https://crrev.com/98be3272d43c5dbde31ef3ef139a952a0d40f4ae/tools/metrics/histograms/enums.xml

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 8 2017

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

commit 09f45112645cf6624c17142a37b19f67bc20eba4
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Tue Aug 08 05:07:00 2017

Use per-frame URLLoaderFactory whenever possible

With this CL, RenderFrameImpl stores its own URLLoaderFactory whenever
possible, rather than using one in RenderBlinkPlatformImpl.

Bug:  712913 
Change-Id: I785ff30f80a3f57ce52d5a9e40a40f6ab81410ff
Reviewed-on: https://chromium-review.googlesource.com/597480
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492537}
[modify] https://crrev.com/09f45112645cf6624c17142a37b19f67bc20eba4/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/09f45112645cf6624c17142a37b19f67bc20eba4/content/renderer/render_frame_impl.h
[modify] https://crrev.com/09f45112645cf6624c17142a37b19f67bc20eba4/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/09f45112645cf6624c17142a37b19f67bc20eba4/content/renderer/renderer_blink_platform_impl.h

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 9 2017

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

commit c3f47658ce8681b740cd8b74e5219cbb6b6a7863
Author: Daniel Hintze <hintzed@google.com>
Date: Wed Aug 09 11:55:12 2017

Fixing bug in CORSURLLoader scaffolding

This CL fixes what I believe to be a small bug introduced in
https://chromium-review.googlesource.com/c/597480.

I think the bug went undetected because of another bug in the virtual
test suite for out-of-blink-cors, which I think did not actually test
out-of-blink-cors. (https://chromium-review.googlesource.com/c/567610).

Bug:  712913 
Change-Id: I3be0c4a8c2e2d4b0c5c0a48d8439952ef4ec59a6
Reviewed-on: https://chromium-review.googlesource.com/607007
Commit-Queue: Daniel Hintze <hintzed@google.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492945}
[modify] https://crrev.com/c3f47658ce8681b740cd8b74e5219cbb6b6a7863/content/child/loader/cors_url_loader.cc
[modify] https://crrev.com/c3f47658ce8681b740cd8b74e5219cbb6b6a7863/content/child/loader/cors_url_loader_factory.cc
[modify] https://crrev.com/c3f47658ce8681b740cd8b74e5219cbb6b6a7863/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/c3f47658ce8681b740cd8b74e5219cbb6b6a7863/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/c3f47658ce8681b740cd8b74e5219cbb6b6a7863/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/c3f47658ce8681b740cd8b74e5219cbb6b6a7863/third_party/WebKit/LayoutTests/virtual/outofblink-cors/external/wpt/fetch/README.txt
[delete] https://crrev.com/7f7424773cd566fe1b2204d41afc882bc09714fa/third_party/WebKit/LayoutTests/virtual/outofblink-cors/webexposed/README.txt

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 10 2017

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

commit c0099d75e2e718db3b34cebf7a6b925fd3193e80
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Thu Aug 10 11:57:23 2017

Add RenderFrameImpl::GetDefaultURLLoaderFactoryContainer

It is sometimes convenient if we can access multiple default URLLoaderFactory's
at once in some custom URLLoader context. (E.g. in ServiceWorker we
need both default network URLLoader and blob URLLoader)

Bug:  715640 ,  712913 
Change-Id: I1d791f6a2c213cd1d23e7985c924714f00e136f5
Reviewed-on: https://chromium-review.googlesource.com/608987
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493360}
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/child/BUILD.gn
[add] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/child/url_loader_factory_container.cc
[add] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/child/url_loader_factory_container.h
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/renderer/render_frame_impl.h
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/renderer/render_thread_impl.h
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/c0099d75e2e718db3b34cebf7a6b925fd3193e80/content/renderer/renderer_blink_platform_impl.h

I was looking into the code around (partly because I wanted to get rid of Platform::CreateURLLoader for S*Workers) and see what possible paths we can take to deprecate Platform::CreateURLLoader() and make URLLoader purely per-frame/per-context.  One of the ideas that looked pragmatic is following (I've very briefly talked about this with Yutaka and toyoshim@), there could be some gotchas I'm overlooking but let me dump it here too:

* Deprecate CreateURLLoader everywhere except for FetchContext’s one
* Instead WebFrameClient defines CreateURLLoaderFactory, which returns something like https://chromium-review.googlesource.com/c/chromium/src/+/551403
* WebURLLoaderFactory should be attached to and stored in each loading context, e.g. in LocalFrame for frames, or in a worker fetch context for workers. WebURLLoaderFactory internally hides the details about URLLoaderFactory selection.
* FrameFetchContext::CreateURLLoader() will be implemented like: Frame()>GetURLLoaderFactory()>CreateURLLoader()
* It feels MockURLLoader can be probably hidden / baked in this abstraction too, e.g. we can have MockWebURLLoaderFactory (ugh naming is confusing) that just returns mock’ed URLLoader when its CreateURLLoader is called, and TestWebFrameClient / MockFetchContext can internally create it and use it.
* To make the transition easier we can also have Platform::CreateDefaultURLLoaderFactory() for the time being, and replace the existing Platform::CreateURLLoader() with it. (The slight difference is that in this way we can still start using per-frame / per-context URLLoaderFactory even for such code paths)
Project Member

Comment 27 by bugdroid1@chromium.org, Oct 24 2017

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

commit 017b79e68acccf53f361d80168cb85daa9c4a8b4
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Tue Oct 24 17:46:10 2017

Always use per-frame / per-context URLLoaderFactory via WebURLLoaderFactory

Replacing CreateURLLoader() in following places with CreateURLLoaderFactory:
- WebFrameClient (e.g. RenderFrameImpl)
- WebWorkerFetchContext
- Platform

And always try to use the created URLLoaderFactory for the context (e.g.
Frame, Worker context or FetchContext).

Remaining TODOs:
- We should remove the WebURLLoaderFactoryWithMock impl in //content and all
  test harness should create its own blink::WebURLLoaderFactoryWithMock
  w/o going though Platform layer (crbug.com/751425)
- Once we finalize the MojoLoading migration WebURLLoaderFactory should be
  replaced with mojo::URLLoaderFactory
- (More code cleanup around throttles etc)

Bug:  712913 , 751425
Change-Id: I4249fd9cf5b056802dc57138e9555d5d9c00122a
Reviewed-on: https://chromium-review.googlesource.com/708074
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511202}
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/components/plugins/renderer/webview_plugin.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/components/printing/renderer/print_render_frame_helper.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/ppapi_plugin/ppapi_blink_platform_impl.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/ppapi_plugin/ppapi_blink_platform_impl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/loader/web_url_loader_impl.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/loader/web_url_loader_impl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/render_frame_impl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/renderer_blink_platform_impl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/service_worker/service_worker_fetch_context_impl.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/service_worker/service_worker_fetch_context_impl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/service_worker/worker_fetch_context_impl.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/renderer/service_worker/worker_fetch_context_impl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/test/test_blink_web_unit_test_support.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/test/test_blink_web_unit_test_support.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/test/test_render_frame.cc
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/content/test/test_render_frame.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/exported/WorkerShadowPage.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/exported/WorkerShadowPage.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/FrameTestHelpers.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/FrameTestHelpers.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/LocalFrameClient.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/core/loader/WorkerFetchContext.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/loader/BUILD.gn
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/loader/testing/FetchTestingPlatformSupport.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/loader/testing/FetchTestingPlatformSupport.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/loader/testing/MockFetchContext.h
[add] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/loader/testing/WebURLLoaderFactoryWithMock.cpp
[add] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/loader/testing/WebURLLoaderFactoryWithMock.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.cpp
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/Source/platform/testing/URLTestHelpers.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/public/platform/Platform.h
[add] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/public/platform/WebURLLoaderFactory.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/public/platform/WebWorkerFetchContext.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/public/web/WebFrameClient.h
[modify] https://crrev.com/017b79e68acccf53f361d80168cb85daa9c4a8b4/third_party/WebKit/public/web/WebLocalFrame.h

Status: Fixed (was: Assigned)
With the change (https://chromium-review.googlesource.com/708074) now basically all the resource requests should go through per-frame or per-worker URLLoaderFactory, so let me close this. (There could be related remaining work like removing routing_id)
Components: Internals>Network>Service
Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Components: Internals>Services>Storage
Setting Internals>Services>Storage to all children of issue 611938

Sign in to add a comment