Replace URLRequestContext::CopyFrom with a similar method on URLRequestContextBuilder |
||||
Issue descriptionThe URLRequestContext::CopyFrom method is easy to misuse: One can create a copy of an URLRequestContext and change some of its fields (e.g. the ChannelIDService) without realizing that that has no effect unless a new HttpNetworkSession is created as well. URLRequestContextBuilder appears much better suited for creating a new URLRequestContext. Would it make sense to add a method to URLRequestContextBuilder to copy the params from an existing URLRequestContext so that the builder can be used to copy and modify a context? This could be used to replace URLRequestContext::CopyFrom. Here's how I'm thinking it would be used: const URLRequestContext* orig_context = (this is the context we want to copy from and then change some things) URLRequestContextBuilder builder; builder.CopyFrom(orig_context); // set things on the builder, for example: builder.SetCookieAndChannelIDStores(new_cookie_store, new_channel_id_service); // use the builder to make the new context: std::unique_ptr<URLRequestContext> new_context = builder.Build()
,
Jul 14 2016
There's issue 146593. I'd rather we just isolate URLRequestContexts entirely rather than make a "better" URLRequestContext::CopyFrom.
,
Jul 14 2016
That makes sense. I know that URLRequestContext::CopyFrom is a mess, and https://codereview.chromium.org/2145103003/ is using it. I provided some guidance there on how more needs to be done than just call URLRequestContext::set_channel_id_service, and was thinking URLRequestContextBuilder would be something better to use (even better would be not needing any CopyFrom-like method).
,
Jul 14 2016
Changing this to available so that it doesn't filter into the network bug triage work list.
,
Jul 14 2016
I'd like a beefed a builder which allows sharing maybe proxy logic and NetLog, and nothing else (Well, maybe HostResolver, too?), and have consumers who really want their own URLRequestContext/CookieStore/whatever just use that. And I'd like to make it impossible for anything but a few classes to create/modify URLRequestContexts, to prevent people from creating yet more broken URLRequestContexts. We could probably make them completely isolated other than NetLog without too much concern, just worried about overhead of them having their own proxy URLRequestContext.
,
Jul 17 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 17 2017
I don't think we want to do this - builders aren't really copy-able. We should work towards getting rid of them method instead, which will hopefully be made easier once we finish the network stack servicification stuff.
,
Jul 17 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by rdsmith@chromium.org
, Jul 14 2016