Thanks for bringing this up and sorry for the trouble.

I didn't consider possible extra security contexts while adding the API. Will verify if that indeed applies, and make sure it works when the network service is disabled.

Thanks, chongz!

Sorry, that last sentence should be "Otherwise, we should rename the method (If any consumer needs what it currently does), and make a new GetURLLoaderFactoryForBrowserProcessIOThread() method that works in all cases."

Regarding security: the URLLoaderFactory used for frames just specifies that the process id is 0, but other than that there are no differences.

somewhat parallel: curious if the speech code can move to the UI thread?

The speech code does not seem heavily maintained, and I'm not interested in doing that myself.

Re #c5: Thanks for the comments! Just to clarify:

* |StoragePartitionImpl::URLLoaderFactoryForBrowserProcess()| and |URLLoaderFactoryGetter| uses the same |partition_->GetNetworkContext()|.
* They both call |CreateURLLoaderFactory()| with process id 0.

So it's safe to use |URLLoaderFactoryGetter| as the backend of |StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessIOThread()|, and to assume it to be the IO thread-equivalent of |StoragePartitionImpl::URLLoaderFactoryForBrowserProcess()|.

Or do you actually opposite to adding the IO thread-equivalent API?

I had also been thinking we'd be using a per-render-frame URLLoaderFactories, which would also mean the shared factory would work, but I've given up on that, and decided I'll just assume render frame IDs / request IDs are basically here to stay.  We seem to be adding more and more dependencies on them, and I've also faced pushback on the only path forward on removing them, so seems a losing battle.

@Matt: makes sense regarding not changing the speech code. I just looked and it's not trivial. I had converted some other code to run on the UI, but that was very self contained.

@Chong: yes that's correct.

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

commit ace7816eca05aff544532af9a43cccfdd388f46c
Author: Chong Zhang <chongz@chromium.org>
Date: Fri May 04 04:39:36 2018

Make URLLoaderFactoryGetter available w/o Network Service

This CL makes |StoragePartitionImpl::url_loader_factory_getter_|
available even when Network Service is disable, such that
|StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessIOThread()|
can be used in all cases.

Note that |URLLoaderFactoryGetter::GetBlobFactory()| is only available
when the network service or servicified service worker is enabled.

This CL also affects some functional unrelated tests, which are tracked
as TODOs and will be addressed in followup CLs.

Bug:  826869 
Change-Id: If408dc20f5e4564a5865c93a4c66316d5682d9e2
Reviewed-on: https://chromium-review.googlesource.com/1033728
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555984}
[modify] https://crrev.com/ace7816eca05aff544532af9a43cccfdd388f46c/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/ace7816eca05aff544532af9a43cccfdd388f46c/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/ace7816eca05aff544532af9a43cccfdd388f46c/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/ace7816eca05aff544532af9a43cccfdd388f46c/content/browser/storage_partition_impl_map.cc
[modify] https://crrev.com/ace7816eca05aff544532af9a43cccfdd388f46c/content/browser/url_loader_factory_getter.cc
[modify] https://crrev.com/ace7816eca05aff544532af9a43cccfdd388f46c/content/browser/url_loader_factory_getter.h

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

commit 36193c585601c36e704c19a2cbcf1252a09dcc49
Author: Chong Zhang <chongz@chromium.org>
Date: Fri May 04 18:31:58 2018

Fix 2 unittests that don't have appropriate URLRequestContext setup

This CL:
1. Fixes |AudioOutputAuthorizationHandlerTest| by using
   |net::TestURLRequestContextGetter| (The original code has an
   use-after-release issue, see bug for logs).
2. Fixes |RenderProcessHostUnitTest| by returning a valid pointer in
   |TestBrowserContext::CreateRequestContextForStoragePartition()|.

This patch doesn't have behavior change.

Bug:  826869 
Change-Id: I05982ce49a9d42029cc3278fa248587820790d74
Reviewed-on: https://chromium-review.googlesource.com/1026986
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556127}
[modify] https://crrev.com/36193c585601c36e704c19a2cbcf1252a09dcc49/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/36193c585601c36e704c19a2cbcf1252a09dcc49/content/browser/storage_partition_impl_map.cc
[modify] https://crrev.com/36193c585601c36e704c19a2cbcf1252a09dcc49/content/public/test/test_browser_context.cc

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

commit 83d0e4f0ee8557205ee4fb315d8ef655765d9c08
Author: Chong Zhang <chongz@chromium.org>
Date: Fri May 04 18:55:09 2018

Move tests for StoragePartitition and fix potential threading issue

The original tests pass non-thread-safe objects across threads which
could easily break.

This CL:
1. Adds 'storage_partition_test_utils.h/cc'.
2. Adds |IOThreadSharedURLLoaderFactoryOwner| which lives on UI thread
   and owns a |SharedURLLoaderFactory| on IO thread.
3. Moves related tests into 'storage_partition_impl_browsertest.cc' to
   make sure they run w/ and w/o Network Service.

This patch doesn't have behavior change.

Bug:  826869 
Change-Id: I23d636b079d72627f8944dd61892d72f175e0c7d
Reviewed-on: https://chromium-review.googlesource.com/1033806
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556136}
[modify] https://crrev.com/83d0e4f0ee8557205ee4fb315d8ef655765d9c08/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/83d0e4f0ee8557205ee4fb315d8ef655765d9c08/content/browser/storage_partition_impl_browsertest.cc
[modify] https://crrev.com/83d0e4f0ee8557205ee4fb315d8ef655765d9c08/content/test/BUILD.gn
[add] https://crrev.com/83d0e4f0ee8557205ee4fb315d8ef655765d9c08/content/test/storage_partition_test_utils.cc
[add] https://crrev.com/83d0e4f0ee8557205ee4fb315d8ef655765d9c08/content/test/storage_partition_test_utils.h

|GetURLLoaderFactoryForBrowserProcessIOThread()| is an IO thread-equivalent of |GetURLLoaderFactoryForBrowserProcess()| now.