Move more SystemURLRequestContext members into its ContextStorage

Move more SystemURLRequestContext members into its ContextStorage

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
Review-Url: https://codereview.chromium.org/2910643003
Cr-Commit-Position: refs/heads/master@{#477782}
Committed: https://chromium.googlesource.com/chromium/src/+/c2af5956cc783ccf67f93c3028f892ce489a2f28

[mmenke, 2017-05-26 18:21:07]

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

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

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-26 18:36:19]

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

[commit-bot: I haz the power, 2017-05-26 18:36:19]

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

[mmenke, 2017-05-26 18:42:55]

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

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

Dry run: CQ is trying da patch.

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

[mmenke, 2017-05-26 19:02:26]

Description was changed from

==========
Move IOThread's cert and auth stuff into the system context's storage.

This is needed before switching over to URLRequestContextBuilder.  We
may need to move this stuff back out before switching ProfileIOData's
URLRequestContext over to using a URLRequestContextBuilder, but may
not.  First pass, this is simpler than giving URLRequestContextBuilder
some APIs that don't take overship of these objects.

BUG=715695
==========

to

==========
Move IOThread's cert and auth stuff into the system context's storage.

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
==========

[mmenke, 2017-05-26 19:28:02]

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

[commit-bot: I haz the power, 2017-05-26 19:28:30]

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-26 19:46:38]

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

[commit-bot: I haz the power, 2017-05-26 19:46:39]

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

[mmenke, 2017-05-26 20:00:31]

Description was changed from

==========
Move IOThread's cert and auth stuff into the system context's storage.

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
==========

to

==========
Move more SystemURLRequestContext members in its ContextStorage

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
==========

[mmenke, 2017-05-26 20:02:49]

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

[commit-bot: I haz the power, 2017-05-26 20:03:10]

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-26 20:38:49]

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

[commit-bot: I haz the power, 2017-05-26 20:38:50]

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

[mmenke, 2017-05-26 20:39:51]

Description was changed from

==========
Move more SystemURLRequestContext members in its ContextStorage

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
==========

to

==========
Move more SystemURLRequestContext members into its ContextStorage

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
==========

[mmenke, 2017-05-26 20:41:16]

mmenke@chromium.org changed reviewers:
+ rdsmith@chromium.org

[mmenke, 2017-05-26 20:41:17]

I think I'll need to do some more Mojo/V8 proxy logic refactoring, but this is
probably the last CL I'll end you before switching over to
URLRequestContextBuilder.

[mmenke, 2017-05-26 20:48:21]

On 2017/05/26 20:41:17, mmenke wrote:
> I think I'll need to do some more Mojo/V8 proxy logic refactoring, but this is
> probably the last CL I'll end you before switching over to
> URLRequestContextBuilder.

Oops, this one looks to have real test failures.  Don't both reviewing this one
until I have them resolved (I'll tell you when that happens).

[mmenke, 2017-05-30 18:06:12]

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

[commit-bot: I haz the power, 2017-05-30 18:06:45]

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-30 18:20:11]

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

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

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[mmenke, 2017-05-30 21:21:07]

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

[commit-bot: I haz the power, 2017-05-30 21:21:39]

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-30 21:26:24]

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

[commit-bot: I haz the power, 2017-05-30 21:26:25]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[mmenke, 2017-06-02 16:35:28]

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

[commit-bot: I haz the power, 2017-06-02 16:35:43]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-02 16:54:26]

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

[commit-bot: I haz the power, 2017-06-02 16:54:38]

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-06-02 17:04:18]

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

[commit-bot: I haz the power, 2017-06-02 17:04:20]

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

[mmenke, 2017-06-02 19:13:14]

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

[commit-bot: I haz the power, 2017-06-02 19:13:55]

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-06-02 19:25:54]

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

[commit-bot: I haz the power, 2017-06-02 19:25:54]

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

[mmenke, 2017-06-02 19:29:07]

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

[commit-bot: I haz the power, 2017-06-02 19:29:43]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-02 19:44:56]

Patchset #8 (id:140001) has been deleted

[mmenke, 2017-06-02 20:12:51]

rdsmith:  PTAL (No rush on this - fine if you don't get to it until next
Tuesday).

[commit-bot: I haz the power, 2017-06-02 21:02:34]

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

[commit-bot: I haz the power, 2017-06-02 21:02:36]

Dry run: This issue passed the CQ dry run.

[mmenke, 2017-06-07 17:56:14]

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

[commit-bot: I haz the power, 2017-06-07 17:56:45]

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-06-07 18:59:27]

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

[commit-bot: I haz the power, 2017-06-07 18:59:31]

Dry run: This issue passed the CQ dry run.

[Randy Smith (Not in Mondays), 2017-06-07 19:28:17]

LGTM--all comments below I'm happy to go with your judgement if it disagrees
with the comment.

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:587: RegisterSTHObserver(ct_tree_tracker_.get());
Why not move this with the ct_verifier code move?  It looks to be a direct data
dependency; why not keep it clustered with that code?

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:851: CreateDefaultAuthHandlerFactory());
Would you be willing to either put in a comment that this call must occur after
the creation of the global host resolver above, or make it implicit by giving
the host resolver as an argument to the function?

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.h (right):

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.h:238:
std::unique_ptr<net::HttpAuthHandlerRegistryFactory>
Why change to the subclass type instead of just sticking with
HttpAuthHandlerFactory?  (Don't care, just curious.)

[mmenke, 2017-06-07 20:11:11]

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

[commit-bot: I haz the power, 2017-06-07 20:11:32]

Dry run: CQ is trying da patch.

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

[mmenke, 2017-06-07 21:06:03]

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:587: RegisterSTHObserver(ct_tree_tracker_.get());
On 2017/06/07 19:28:17, Randy Smith (Not in Mondays) wrote:
> Why not move this with the ct_verifier code move?  It looks to be a direct
data
> dependency; why not keep it clustered with that code?

Because these aren't owned by (or pointed to by) the URLRequestContext, and I
want to share URLRequestContextBuilder code between this and ProfileIOData.  If
these remain shared, I'll either need special case code there for the first
URLRequestContext built, or I'll need to make all shared objects in a separate
function during set up (While the system URLRequestContext is created first,
once we switch to using a network service, that could change, if we construct
them on first use when using the NetworkService).  If I use a separate function,
it's more likely I'll move shared objects back here, and make this the separate
function.

That does perhaps argue I should be doing a design document up front to decide
some of this stuff, but there's just so much cruft that I've been reluctant to
do it, and the path is grungy enough that I'mt not sure how well I'd be able to
keep to the plan, anyways.

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.cc:851: CreateDefaultAuthHandlerFactory());
On 2017/06/07 19:28:17, Randy Smith (Not in Mondays) wrote:
> Would you be willing to either put in a comment that this call must occur
after
> the creation of the global host resolver above, or make it implicit by giving
> the host resolver as an argument to the function?

Did the latter (Was planning to do it in the next CL, anyway, when we won't even
have a URLRequestContext to steal things from)

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
File chrome/browser/io_thread.h (right):

https://codereview.chromium.org/2910643003/diff/160001/chrome/browser/io_thre...
chrome/browser/io_thread.h:238:
std::unique_ptr<net::HttpAuthHandlerRegistryFactory>
On 2017/06/07 19:28:17, Randy Smith (Not in Mondays) wrote:
> Why change to the subclass type instead of just sticking with
> HttpAuthHandlerFactory?  (Don't care, just curious.)

Just because an "HttpAuthHandlerRegistryFactory" is not clearly an
"HttpAuthHandlerFactory" (It sounds like it creates HttpAuthHandlerRegistries),
and I thought this made it a little clearer it was, though it's certainly
debatable.  Anyhow, don't have strong opinion here, so I changed it.

[mmenke, 2017-06-07 21:06:18]

The CQ bit was unchecked by mmenke@chromium.org

[mmenke, 2017-06-07 21:06:23]

The CQ bit was checked by mmenke@chromium.org

[mmenke, 2017-06-07 21:06:23]

The patchset sent to the CQ was uploaded after l-g-t-m from rdsmith@chromium.org
Link to the patchset: https://codereview.chromium.org/2910643003/#ps200001
(title: "Response")

[commit-bot: I haz the power, 2017-06-07 21:06:59]

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-06-07 21:07:42]

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

[commit-bot: I haz the power, 2017-06-07 21:07:44]

CQ cannot get SignCLA result. Please try later.

[mmenke, 2017-06-07 21:33:35]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2017-06-07 21:34:19]

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-06-07 23:26:50]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1496871215038820,
"parent_rev": "466d40a9545e8124e2ce2561146d46aceec23420", "commit_rev":
"9667358ddfbcc627fa35bad406bddf3b36404c48"}

[commit-bot: I haz the power, 2017-06-07 23:27:03]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1496871215038820,
"parent_rev": "7576773367d39acbaa5e8011722803605354d047", "commit_rev":
"c2af5956cc783ccf67f93c3028f892ce489a2f28"}

[commit-bot: I haz the power, 2017-06-07 23:27:14]

Description was changed from

==========
Move more SystemURLRequestContext members into its ContextStorage

This is needed before switching over to URLRequestContextBuilder.

BUG=715695
==========

to

==========
Move more SystemURLRequestContext members into its ContextStorage

This is needed before switching over to URLRequestContextBuilder.

BUG=715695

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

[commit-bot: I haz the power, 2017-06-07 23:27:15]

Committed patchset #10 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/c2af5956cc783ccf67f93c3028f8...