Created new URLRequestContext for secure proxy check

components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
index 947f4514a86d17fe364419789e05fe43f949c5d4..5eeac8d9abe91888ea45178858371a75c3e8f6f8 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
@@ -7,7 +7,13 @@
 #include "base/sequenced_task_runner.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h"
 #include "net/base/load_flags.h"
+#include "net/http/http_network_layer.h"
+#include "net/http/http_network_session.h"
+#include "net/proxy/proxy_config_service_fixed.h"
 #include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_builder.h"

[sclittle, 2015/03/18 22:52:15]

Some of these includes aren't used.

[tbansal1, 2015/03/19 22:43:57]

Done.

+#include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 
 namespace data_reduction_proxy {
@@ -59,11 +65,39 @@ void DataReductionProxyService::OnURLFetchComplete(
 
 net::URLFetcher* DataReductionProxyService::GetURLFetcherForSecureProxyCheck(
     const GURL& secure_proxy_check_url) {
+  DCHECK(url_request_context_getter_);
+
+  net::URLRequestContext* url_request_context(

[bengr, 2015/03/16 23:21:06]

Who owns this memory? I don't see anyone deleting it.

[tbansal1, 2015/03/19 22:43:57]

Done.

+      new net::URLRequestContext());
+  url_request_context->CopyFrom(
+      url_request_context_getter_->GetURLRequestContext());
+
+  net::HttpNetworkSession::Params* params_original =
+      const_cast<net::HttpNetworkSession::Params*>(
+          url_request_context->GetNetworkSessionParams());
+  DCHECK(params_original);
+
+  net::HttpNetworkSession::Params params_modified = *params_original;

[bengr, 2015/03/16 23:21:06]

Why do you need to const_cast? Why can't you just dereference the return from
params_original and assign to params_modified?

[tbansal1, 2015/03/19 22:43:57]

Done.

+  params_modified.enable_quic = false;
+  params_modified.enable_quic_for_proxies = false;
+  params_modified.use_alternate_protocols = false;

[sclittle, 2015/03/18 22:52:15]

I think you just need to clear params_modified.next_protos and set enable_quic =
false, see:

https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...

[tbansal1, 2015/03/19 22:43:57]

Done.

+
+  scoped_refptr<net::HttpNetworkSession> network_session(
+      new net::HttpNetworkSession(params_modified));
+
+  url_request_context->set_http_transaction_factory(
+      new net::HttpNetworkLayer(network_session.get()));

[sclittle, 2015/03/18 22:52:15]

I don't think the URLRequestContext actually takes ownership of the
HttpTransactionFactory; you'll have to manage the ownership yourself.

[tbansal1, 2015/03/19 22:43:57]

Done.

[tbansal1, 2015/03/19 22:43:57]

Done.

+
+  // URLRequestContextGetter to be used for the secure proxy check.
+  scoped_refptr<net::URLRequestContextGetter> getter_secure_proxy_check(

[sclittle, 2015/03/18 22:52:15]

Ideally, this URLRequestContextGetter would own the URLRequestContext somehow,
so that when the URLFetcher is destroyed, the URLRequestContext is also
destroyed.

Since all this stuff lives on the IO thread, maybe you could create your own
SecureProxyCheckURLRequestContextGetter or something that owns the
URLRequestContext; you could probably give it ownership of the
HttpTransactionFactory as well.

[tbansal1, 2015/03/19 22:43:57]

Managing the memory in a similar way as it was done for fetcher_.
Creating new class might be overkill?

+      new net::TrivialURLRequestContextGetter(
+          url_request_context,
+          url_request_context_getter_->GetNetworkTaskRunner()));
+
   net::URLFetcher* fetcher = net::URLFetcher::Create(
       secure_proxy_check_url, net::URLFetcher::GET, this);
   fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE | net::LOAD_BYPASS_PROXY);
-  DCHECK(url_request_context_getter_);
-  fetcher->SetRequestContext(url_request_context_getter_);
+  fetcher->SetRequestContext(getter_secure_proxy_check.get());
   // Configure max retries to be at most kMaxRetries times for 5xx errors.
   static const int kMaxRetries = 5;
   fetcher->SetMaxRetriesOn5xx(kMaxRetries);
@@ -78,8 +112,21 @@ net::URLFetcher* DataReductionProxyService::GetURLFetcherForSecureProxyCheck(
 
 void DataReductionProxyService::SecureProxyCheck(
     const GURL& secure_proxy_check_url,
-    FetcherResponseCallback fetcher_callback) {
+    FetcherResponseCallback fetcher_callback,

[bengr, 2015/03/16 23:21:06]

const& ?

[tbansal1, 2015/03/19 22:43:57]

Done.

[tbansal1, 2015/03/19 22:43:57]

Done.

+    scoped_refptr<base::SequencedTaskRunner> io_task_runner) {
   DCHECK(CalledOnValidThread());
+
+  io_task_runner->PostTask(
+          FROM_HERE, base::Bind(
+              &DataReductionProxyService::SecureProxyCheckOnIOThread,
+              base::Unretained(this),
+              secure_proxy_check_url,
+              fetcher_callback));
+}
+
+void DataReductionProxyService::SecureProxyCheckOnIOThread(
+    const GURL& secure_proxy_check_url,
+    FetcherResponseCallback fetcher_callback) {
   net::URLFetcher* fetcher =
       GetURLFetcherForSecureProxyCheck(secure_proxy_check_url);
   if (!fetcher)