Add support in the network service for different contexts.

Add support in the network service for different contexts.

Now each StoragePartition has its own network context (which owns the net::URLRequestContext). The renderer connects to the network service through the browser instead of directly, which securely annotates the factory with the render process ID.

BUG=720374
Review-Url: https://codereview.chromium.org/2874163004
Cr-Commit-Position: refs/heads/master@{#472101}
Committed: https://chromium.googlesource.com/chromium/src/+/c190586208cdae9ddecfd8d33d5aee3fc7c4dd92

[jam, 2017-05-12 18:04:04]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 18:05:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 18:26:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 18:26:46]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jam, 2017-05-12 20:00:57]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 20:02:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 20:05:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 20:05:20]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[jam, 2017-05-12 20:42:20]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 20:43:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2017-05-12 20:47:41]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 20:48:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2017-05-12 20:50:29]

Patchset #2 (id:20001) has been deleted

[jam, 2017-05-12 21:17:07]

jam@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2017-05-12 21:56:17]

Hrm, more thread crud. I need to clone the connector to the IO thread.

[commit-bot: I haz the power, 2017-05-12 22:17:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 22:17:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2017-05-12 22:19:13]

On 2017/05/12 21:56:17, scottmg wrote:
> Hrm, more thread crud. I need to clone the connector to the IO thread.

I'll land this after your cl, so I'll adjust the code.

[scottmg, 2017-05-12 22:27:14]

Took me a while to figure out how it fits together, but lgtm.

On 2017/05/12 22:19:13, jam wrote:
> On 2017/05/12 21:56:17, scottmg wrote:
> > Hrm, more thread crud. I need to clone the connector to the IO thread.
> 
> I'll land this after your cl, so I'll adjust the code.

Shrug, yours is ready, might as well land.

[jam, 2017-05-12 23:07:23]

Description was changed from

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context. The renderer now connects
to the network service through the browser, which can add the trusted render
process ID to the factory.

BUG=720374
==========

to

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context. The renderer  connects to
the network service through the browser instead of directly, which securely
annotates the factory with the render process ID.

BUG=720374
==========

[jam, 2017-05-12 23:09:15]

jam@chromium.org changed reviewers:
+ tsepez@chromium.org

[jam, 2017-05-12 23:09:16]

+Tom for manifest/mojom security review (and overall if you have comments)

[jam, 2017-05-12 23:11:49]

Description was changed from

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context. The renderer  connects to
the network service through the browser instead of directly, which securely
annotates the factory with the render process ID.

BUG=720374
==========

to

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context. The renderer connects to
the network service through the browser instead of directly, which securely
annotates the factory with the render process ID.

BUG=720374
==========

[Tom Sepez, 2017-05-12 23:26:34]

https://codereview.chromium.org/2874163004/diff/40001/content/common/network_...
File content/common/network_service.mojom (right):

https://codereview.chromium.org/2874163004/diff/40001/content/common/network_...
content/common/network_service.mojom:14: // Points to the cookie file. If null,
an in-memory database is used.
relative to what?   Is this the complete path, or the path to a directory under
which to find several cookie files.  Can we pass 
a file handle here instead?

https://codereview.chromium.org/2874163004/diff/40001/content/common/network_...
content/common/network_service.mojom:17: mojo.common.mojom.FilePath? cache_dir;
Same question here.  Also, how is this organized given multiple profiles.  Are
they all cached in the same spot?

[jam, 2017-05-13 00:01:36]

https://codereview.chromium.org/2874163004/diff/40001/content/common/network_...
File content/common/network_service.mojom (right):

https://codereview.chromium.org/2874163004/diff/40001/content/common/network_...
content/common/network_service.mojom:14: // Points to the cookie file. If null,
an in-memory database is used.
On 2017/05/12 23:26:33, Tom Sepez wrote:
> relative to what?   Is this the complete path, or the path to a directory
under
> which to find several cookie files.  Can we pass 
> a file handle here instead?

Presumably this would be a complete path. Sending handles sounds great, but I
leave that as part of the sandboxing effort (as there would be further changes
to /net needed since those files currently expect a file path.

Note currently this isn't used, I'm using it as an example for future cls. Right
now we're using in-memory databases for cache & cookies.

https://codereview.chromium.org/2874163004/diff/40001/content/common/network_...
content/common/network_service.mojom:17: mojo.common.mojom.FilePath? cache_dir;
On 2017/05/12 23:26:33, Tom Sepez wrote:
> Same question here.  Also, how is this organized given multiple profiles.  Are
> they all cached in the same spot?

Each profile (or really, storage partition) needs to use a different directory
for the cache.

[Tom Sepez, 2017-05-15 16:06:12]

lgtm

Ok, given my followup tasks.

[jam, 2017-05-16 01:45:01]

Patchset #3 (id:60001) has been deleted

[jam, 2017-05-16 01:47:30]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[jam, 2017-05-16 01:48:44]

Scott: ptal, after syncing past your change I realized that I couldn't get the
URLLoaderFactory that's per-StoragePartition and cache it globally. So I've
created URLLoaderFactoryGetter which holds on to it per StoragePartition.

[commit-bot: I haz the power, 2017-05-16 01:48:47]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-16 01:54:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-16 01:54:21]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[jam, 2017-05-16 02:00:33]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-16 02:01:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-16 02:07:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-16 02:07:12]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[jam, 2017-05-16 03:24:51]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[jam, 2017-05-16 03:25:12]

The CQ bit was unchecked by jam@chromium.org

[jam, 2017-05-16 03:25:15]

The CQ bit was checked by jam@chromium.org

[jam, 2017-05-16 03:25:15]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2874163004/#ps80001
(title: "update after r471804")

[jam, 2017-05-16 03:25:16]

The CQ bit was unchecked by jam@chromium.org

[commit-bot: I haz the power, 2017-05-16 03:25:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2017-05-16 03:26:02]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-16 03:26:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-16 03:36:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-16 03:36:31]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[scottmg, 2017-05-16 03:53:42]

scottmg@chromium.org changed reviewers:
+ ananta@chromium.org

[scottmg, 2017-05-16 03:53:43]

lgtm 

(I was excited it was going to be url_loader_factory_factory, but factory_getter
is more appropriate. ;)

+ananta fyi, you'll need to rebase the hookup code again.

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
File content/browser/url_loader_factory_getter.cc (right):

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
content/browser/url_loader_factory_getter.cc:24:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
DCHECK_CURRENTLY_ON(BrowserThread::IO);

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
content/browser/url_loader_factory_getter.cc:33: BrowserThread::PostTask(
I was worried at first about this racing with the subsequent call to the user of
the test factory, but I guess since we necessarily navigate on UI, and call
GetNetworkFactory() which DCHECKs that it's on IO, it's pretty future-safe.

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
File content/browser/url_loader_factory_getter.h (right):

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
content/browser/url_loader_factory_getter.h:16: // Holds on to URLLoaderFactory
for a given StoragePartition allows code running
"for a given StoragePartition allows code" missing "and" in there.

[jam, 2017-05-16 04:02:55]

Patchset #3 (id:80001) has been deleted

[jam, 2017-05-16 04:08:40]

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
File content/browser/url_loader_factory_getter.cc (right):

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
content/browser/url_loader_factory_getter.cc:24:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
On 2017/05/16 03:53:43, scottmg wrote:
> DCHECK_CURRENTLY_ON(BrowserThread::IO);

Done.

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
content/browser/url_loader_factory_getter.cc:33: BrowserThread::PostTask(
On 2017/05/16 03:53:43, scottmg wrote:
> I was worried at first about this racing with the subsequent call to the user
of
> the test factory, but I guess since we necessarily navigate on UI, and call
> GetNetworkFactory() which DCHECKs that it's on IO, it's pretty future-safe.

Yep exactly :)

[jam, 2017-05-16 04:08:50]

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
File content/browser/url_loader_factory_getter.h (right):

https://codereview.chromium.org/2874163004/diff/100001/content/browser/url_lo...
content/browser/url_loader_factory_getter.h:16: // Holds on to URLLoaderFactory
for a given StoragePartition allows code running
On 2017/05/16 03:53:43, scottmg wrote:
> "for a given StoragePartition allows code" missing "and" in there.

Done.

[jam, 2017-05-16 04:09:15]

The CQ bit was checked by jam@chromium.org

[jam, 2017-05-16 04:09:15]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2874163004/#ps120001
(title: "review comment")

[commit-bot: I haz the power, 2017-05-16 04:09:51]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2017-05-16 04:09:55]

Description was changed from

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context. The renderer connects to
the network service through the browser instead of directly, which securely
annotates the factory with the render process ID.

BUG=720374
==========

to

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context (which owns the
net::URLRequestContext). The renderer connects to the network service through
the browser instead of directly, which securely annotates the factory with the
render process ID.

BUG=720374
==========

[commit-bot: I haz the power, 2017-05-16 04:24:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-16 04:24:21]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2017-05-16 05:52:01]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[jam, 2017-05-16 05:52:03]

Patchset #4 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-05-16 05:52:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-16 08:16:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-16 08:16:06]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jam, 2017-05-16 13:46:54]

The CQ bit was checked by jam@chromium.org

[jam, 2017-05-16 13:46:55]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2874163004/#ps140001
(title: "review comment and clang fixes")

[commit-bot: I haz the power, 2017-05-16 13:47:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-16 14:45:33]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1494942414700650,
"parent_rev": "0fe78b97dbbd03e9e08d676085f80a991d767d1f", "commit_rev":
"c190586208cdae9ddecfd8d33d5aee3fc7c4dd92"}

[commit-bot: I haz the power, 2017-05-16 14:45:43]

Description was changed from

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context (which owns the
net::URLRequestContext). The renderer connects to the network service through
the browser instead of directly, which securely annotates the factory with the
render process ID.

BUG=720374
==========

to

==========
Add support in the network service for different contexts.

Now each StoragePartition has its own network context (which owns the
net::URLRequestContext). The renderer connects to the network service through
the browser instead of directly, which securely annotates the factory with the
render process ID.

BUG=720374

Review-Url: https://codereview.chromium.org/2874163004
Cr-Commit-Position: refs/heads/master@{#472101}
Committed:
https://chromium.googlesource.com/chromium/src/+/c190586208cdae9ddecfd8d33d5a...
==========

[commit-bot: I haz the power, 2017-05-16 14:45:44]

Committed patchset #4 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/c190586208cdae9ddecfd8d33d5a...