New issue
Advanced search Search tips

Issue 600450 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

enable_brotli should not be a member of HttpNetworkSession::Params.

Project Member Reported by mmenke@chromium.org, Apr 4 2016

Issue description

This is the only dependency of the URLRequest[Job] layer on HttpNetworkSession, and the dependency should be removed.  Simplest solution seems to be to make it a member of URLRequestContext.

The way the network stack is currently architected, the HttpNetworkSession is only used by lower level classes:  HttpNetworkTransaction and below.
 
Summary: enable_brotli should not be a member of HttpNetworkSession::Params. (was: enable_brotli is currently a member of HttpNetworkSession::Params.)
I will take a look into that.

Comment 4 by mmenke@chromium.org, Apr 22 2016

You could also set it on the SystemURLRequestContext - make the bool a member of IOThread::Globals, and set it on the SystemURLRequestContext, and the profile's main request context in ProfileIOData::Init.

Then make URLRequestContext::CopyFrom copy it, and it should propogate to the other URLRequestContext that piggy back off of the main one.  Don't think we need to worry about anything else.
Project Member

Comment 6 by bugdroid1@chromium.org, May 2 2016

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

commit 3d40c8112303a985fab79c74e92e9a79fc4ac9f9
Author: maksim.sisov <maksim.sisov@intel.com>
Date: Mon May 02 13:27:16 2016

Move bool enable_brotli from HttpNetworkSession::Params

Moving bool enable_brotli from HttpNetoworkSession::Params
to URLRequestContext.

Brotli is set on thread initialization and profileiodata initialization.
Then it is propagated to other contexts through
URLRequestContext::CopyFrom method.

BUG= 600450 

Review-Url: https://codereview.chromium.org/1922403002
Cr-Commit-Position: refs/heads/master@{#390930}

[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/chrome/browser/io_thread.cc
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/chrome/browser/io_thread.h
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/net/http/http_network_session.cc
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/net/http/http_network_session.h
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/net/url_request/url_request_context.cc
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/net/url_request/url_request_context.h
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/3d40c8112303a985fab79c74e92e9a79fc4ac9f9/net/url_request/url_request_http_job_unittest.cc

Status: Fixed (was: Available)

Sign in to add a comment