|
|
Created:
3 years, 7 months ago by mmenke Modified:
3 years, 7 months ago Reviewers:
Randy Smith (Not in Mondays) CC:
chromium-reviews, cbentzel+watch_chromium.org, net-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd method to init HttpNetworkSession::Params from context bulder params
This method will most likely be temporary, and removed once IOThread's
and ProfileIOData's URLRequestContexts can be configured using a
URLRequestContextBuilder.
BUG=715697
Review-Url: https://codereview.chromium.org/2876243002
Cr-Commit-Position: refs/heads/master@{#471501}
Committed: https://chromium.googlesource.com/chromium/src/+/cfea205a4f3ebb3f443467fe96c7252ebf375089
Patch Set 1 #
Total comments: 4
Patch Set 2 : Defrag #Patch Set 3 : Oops, base on top of trunk #
Messages
Total messages: 21 (14 generated)
The CQ bit was checked by mmenke@chromium.org to run a CQ dry run
The CQ bit was checked by mmenke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mmenke@chromium.org changed reviewers: + rdsmith@chromium.org
This is pretty much pure copy-paste. Next CL will switch the IOThread over to using these params instead of an HttpNetworkSessionParams. We can either make the params (Or most of them) a Mojo configuration argument, pass them individually, or have the Mojo process grab them from field trials and command line flags (Don't like that option, though that's what the network service is doing for some things now). Regardless, we'll need a builder to be able to accept all options that IOThread and ProfileIOData use to configure a network session, and this is a stepping stone for that.
Description was changed from ========== Add method inint HttpNetworkSession::Params from context bulder's params This method will most likely be temporary, and removed once IOThread's and ProfileIOData's URLRequestContexts can be configured using a URLRequestContextBuilder. BUG=715697 ========== to ========== Add method to init HttpNetworkSession::Params from context bulder params This method will most likely be temporary, and removed once IOThread's and ProfileIOData's URLRequestContexts can be configured using a URLRequestContextBuilder. BUG=715697 ==========
LGTM. https://codereview.chromium.org/2876243002/diff/1/net/url_request/url_request... File net/url_request/url_request_context_builder.cc (right): https://codereview.chromium.org/2876243002/diff/1/net/url_request/url_request... net/url_request/url_request_context_builder.cc:204: network_session_params->host_mapping_rules = host_mapping_rules; Not relevant to this CL (no behavior change), so just a comment, but copying raw pointers like this without commentary as to expected lifetimes scares me. https://codereview.chromium.org/2876243002/diff/1/net/url_request/url_request... net/url_request/url_request_context_builder.cc:220: network_session_params->quic_user_agent_id = quic_user_agent_id; nit, suggestion: Move this to be in order with the declaration of HttpNetworkSessionParams, i.e. above quic_max_server_configs_stored_in_properties.
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by mmenke@chromium.org
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/2876243002/#ps60001 (title: "Oops, base on top of trunk")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks! https://codereview.chromium.org/2876243002/diff/1/net/url_request/url_request... File net/url_request/url_request_context_builder.cc (right): https://codereview.chromium.org/2876243002/diff/1/net/url_request/url_request... net/url_request/url_request_context_builder.cc:204: network_session_params->host_mapping_rules = host_mapping_rules; On 2017/05/12 19:43:52, Randy Smith (Not in Mondays) wrote: > Not relevant to this CL (no behavior change), so just a comment, but copying raw > pointers like this without commentary as to expected lifetimes scares me. Yea, I agree with that. I think use of this is rare enough, and we have few enough HttpNetworkSessions, that copying makes more sense. Not going to address it now, but may once I start working on the network service code. https://codereview.chromium.org/2876243002/diff/1/net/url_request/url_request... net/url_request/url_request_context_builder.cc:220: network_session_params->quic_user_agent_id = quic_user_agent_id; On 2017/05/12 19:43:52, Randy Smith (Not in Mondays) wrote: > nit, suggestion: Move this to be in order with the declaration of > HttpNetworkSessionParams, i.e. above > quic_max_server_configs_stored_in_properties. Done. Also rearranged these to match order in HttpNetworkSession::Params.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by mmenke@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494631308376680, "parent_rev": "c2c2f1e158ff023cfac05e72160fa89d6228fc3b", "commit_rev": "cfea205a4f3ebb3f443467fe96c7252ebf375089"}
Message was sent while issue was closed.
Description was changed from ========== Add method to init HttpNetworkSession::Params from context bulder params This method will most likely be temporary, and removed once IOThread's and ProfileIOData's URLRequestContexts can be configured using a URLRequestContextBuilder. BUG=715697 ========== to ========== Add method to init HttpNetworkSession::Params from context bulder params This method will most likely be temporary, and removed once IOThread's and ProfileIOData's URLRequestContexts can be configured using a URLRequestContextBuilder. BUG=715697 Review-Url: https://codereview.chromium.org/2876243002 Cr-Commit-Position: refs/heads/master@{#471501} Committed: https://chromium.googlesource.com/chromium/src/+/cfea205a4f3ebb3f443467fe96c7... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/cfea205a4f3ebb3f443467fe96c7... |