Initialize data reduction proxy bypass stats on IO thread

Initialize data reduction proxy (DRP) bypass stats on IO thread, and
register as a network change notifier on the IO thread. This ensures
that network change notifier does not notify DRP bypass stats on IO
thread after DRP bypass stats has been destroyed. This prevents the
use after null crash.

Also, add thread checker to some of the other DRP classes.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=669929

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

components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc

@@ -85,6 +85,7 @@ bool DataReductionProxyBypassProtocol::MaybeBypassProxyAndPrepareToRetry(
     net::URLRequest* request,
     DataReductionProxyBypassType* proxy_bypass_type,
     DataReductionProxyInfo* data_reduction_proxy_info) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(request);
   const net::HttpResponseHeaders* response_headers =
       request->response_info().headers.get();

components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_BYPASS_PROTOCOL_H_
 
 #include "base/macros.h"
+#include "base/threading/thread_checker.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
 
 namespace net {
@@ -53,6 +54,8 @@ class DataReductionProxyBypassProtocol {
   // Must outlive |this|.
   DataReductionProxyConfig* config_;
 
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyBypassProtocol);
 };
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats.cc

@@ -134,17 +134,24 @@ DataReductionProxyBypassStats::DataReductionProxyBypassStats(
       proxy_net_errors_count_(0),
       unavailable_(false) {
   DCHECK(config);
-  NetworkChangeNotifier::AddNetworkChangeObserver(this);
+  // Constructed on the UI thread, but should be checked on the IO thread.
+  thread_checker_.DetachFromThread();
 }
 
 DataReductionProxyBypassStats::~DataReductionProxyBypassStats() {
   NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
 }
 
+void DataReductionProxyBypassStats::InitializeOnIOThread() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  NetworkChangeNotifier::AddNetworkChangeObserver(this);
+}
+
 void DataReductionProxyBypassStats::OnUrlRequestCompleted(
     const net::URLRequest* request,
     bool started,
     int net_error) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DataReductionProxyTypeInfo proxy_info;
   // Ignore requests that did not use the data reduction proxy. The check for
   // LOAD_BYPASS_PROXY is necessary because the proxy_server() in the |request|
@@ -184,12 +191,14 @@ void DataReductionProxyBypassStats::OnUrlRequestCompleted(
 
 void DataReductionProxyBypassStats::SetBypassType(
     DataReductionProxyBypassType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   last_bypass_type_ = type;
   triggering_request_ = true;
 }
 
 DataReductionProxyBypassType
 DataReductionProxyBypassStats::GetBypassType() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   return last_bypass_type_;
 }
 
@@ -197,6 +206,7 @@ void DataReductionProxyBypassStats::RecordBytesHistograms(
     const net::URLRequest& request,
     bool data_reduction_proxy_enabled,
     const net::ProxyConfig& data_reduction_proxy_config) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   RecordBypassedBytesHistograms(request, data_reduction_proxy_enabled,
                                 data_reduction_proxy_config);
   RecordMissingViaHeaderBytes(request);
@@ -205,6 +215,7 @@ void DataReductionProxyBypassStats::RecordBytesHistograms(
 void DataReductionProxyBypassStats::OnProxyFallback(
     const net::ProxyServer& bypassed_proxy,
     int net_error) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DataReductionProxyTypeInfo data_reduction_proxy_info;
   if (bypassed_proxy.is_valid() && !bypassed_proxy.is_direct() &&
       data_reduction_proxy_config_->IsDataReductionProxy(
@@ -237,11 +248,13 @@ void DataReductionProxyBypassStats::OnProxyFallback(
 }
 
 void DataReductionProxyBypassStats::ClearRequestCounts() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   successful_requests_through_proxy_count_ = 0;
   proxy_net_errors_count_ = 0;
 }
 
 void DataReductionProxyBypassStats::NotifyUnavailabilityIfChanged() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   bool prev_unavailable = unavailable_;
   unavailable_ =
       (proxy_net_errors_count_ >= kMinFailedRequestsWhenUnavailable &&
@@ -255,6 +268,7 @@ void DataReductionProxyBypassStats::RecordBypassedBytesHistograms(
     const net::URLRequest& request,
     bool data_reduction_proxy_enabled,
     const net::ProxyConfig& data_reduction_proxy_config) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   int64_t content_length = request.received_response_content_length();
 
   // Only record histograms when the data reduction proxy is enabled.
@@ -370,6 +384,7 @@ void DataReductionProxyBypassStats::RecordBypassedBytesHistograms(
 
 void DataReductionProxyBypassStats::RecordMissingViaHeaderBytes(
     const URLRequest& request) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Responses that were served from cache should have been filtered out
   // already.
   DCHECK(!request.was_cached());
@@ -395,6 +410,7 @@ void DataReductionProxyBypassStats::RecordMissingViaHeaderBytes(
 
 void DataReductionProxyBypassStats::OnNetworkChanged(
     NetworkChangeNotifier::ConnectionType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   ClearRequestCounts();
 }
 
@@ -402,6 +418,7 @@ void DataReductionProxyBypassStats::RecordBypassedBytes(
     DataReductionProxyBypassType bypass_type,
     DataReductionProxyBypassStats::BypassedBytesType bypassed_bytes_type,
     int64_t content_length) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Individual histograms are needed to count the bypassed bytes for each
   // bypass type so that we can see the size of requests. This helps us
   // remove outliers that would skew the sum of bypassed bytes for each type.

components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/threading/thread_checker.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/network_change_notifier.h"
@@ -55,6 +56,9 @@ class DataReductionProxyBypassStats
 
   ~DataReductionProxyBypassStats() override;
 
+  // Performs initialization on the IO thread.
+  void InitializeOnIOThread();
+
   // Callback intended to be called from |DataReductionProxyNetworkDelegate|
   // when a request completes. This method is used to gather bypass stats.
   void OnUrlRequestCompleted(const net::URLRequest* request,
@@ -154,6 +158,8 @@ class DataReductionProxyBypassStats
   // Whether or not the data reduction proxy is unavailable.
   bool unavailable_;
 
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyBypassStats);
 };
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.cc

@@ -43,6 +43,7 @@ void DataReductionProxyConfigurator::Enable(
 net::ProxyConfig DataReductionProxyConfigurator::CreateProxyConfig(
     bool secure_transport_restricted,
     const std::vector<net::ProxyServer>& proxies_for_http) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   net::ProxyConfig config;
   config.proxy_rules().type =
       net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME;

components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.cc

@@ -38,6 +38,8 @@ DataReductionProxyInterceptor::~DataReductionProxyInterceptor() {
 net::URLRequestJob* DataReductionProxyInterceptor::MaybeInterceptRequest(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   return nullptr;
 }
 
@@ -45,12 +47,16 @@ net::URLRequestJob* DataReductionProxyInterceptor::MaybeInterceptRedirect(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
     const GURL& location) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   return MaybeInterceptResponseOrRedirect(request, network_delegate);
 }
 
 net::URLRequestJob* DataReductionProxyInterceptor::MaybeInterceptResponse(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   return MaybeInterceptResponseOrRedirect(request, network_delegate);
 }
 
@@ -58,6 +64,8 @@ net::URLRequestJob*
 DataReductionProxyInterceptor::MaybeInterceptResponseOrRedirect(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   DCHECK(request);
   if (request->response_info().was_cached)
     return nullptr;
@@ -107,6 +115,8 @@ void DataReductionProxyInterceptor::MaybeAddBypassEvent(
     const DataReductionProxyInfo& data_reduction_proxy_info,
     DataReductionProxyBypassType bypass_type,
     bool should_retry) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   if (data_reduction_proxy_info.bypass_action != BYPASS_ACTION_TYPE_NONE) {
     event_creator_->AddBypassActionEvent(
         request->net_log(), data_reduction_proxy_info.bypass_action,

components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/threading/thread_checker.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
 #include "net/url_request/url_request_interceptor.h"
 
@@ -85,6 +86,8 @@ class DataReductionProxyInterceptor : public net::URLRequestInterceptor {
   // these cases.
   std::unique_ptr<DataReductionProxyBypassProtocol> bypass_protocol_;
 
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyInterceptor);
 };
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc

@@ -167,6 +167,9 @@ DataReductionProxyIOData::DataReductionProxyIOData(
 }
 
 DataReductionProxyIOData::~DataReductionProxyIOData() {
+  // Guaranteed to be destroyed on IO thread if the IO thread is still
+  // available at the time of destruction. If the IO thread is unavailable,
+  // then the destruction will happen on the UI thread.
 }
 
 void DataReductionProxyIOData::ShutdownOnUIThread() {
@@ -194,6 +197,7 @@ void DataReductionProxyIOData::SetDataReductionProxyService(
 void DataReductionProxyIOData::InitializeOnIOThread() {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
   config_->InitializeOnIOThread(basic_url_request_context_getter_.get());
+  bypass_stats_->InitializeOnIOThread();
   proxy_delegate_->InitializeOnIOThread();
   if (config_client_.get())
     config_client_->InitializeOnIOThread(url_request_context_getter_);

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

@@ -193,6 +193,7 @@ DataReductionProxyNetworkDelegate::~DataReductionProxyNetworkDelegate() {
 void DataReductionProxyNetworkDelegate::InitIODataAndUMA(
     DataReductionProxyIOData* io_data,
     DataReductionProxyBypassStats* bypass_stats) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(bypass_stats);
   data_reduction_proxy_io_data_ = io_data;
   data_reduction_proxy_bypass_stats_ = bypass_stats;
@@ -200,6 +201,7 @@ void DataReductionProxyNetworkDelegate::InitIODataAndUMA(
 
 std::unique_ptr<base::Value>
 DataReductionProxyNetworkDelegate::SessionNetworkStatsInfoToValue() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   auto dict = base::MakeUnique<base::DictionaryValue>();
   // Use strings to avoid overflow. base::Value only supports 32-bit integers.
   dict->SetString("session_received_content_length",
@@ -213,6 +215,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeURLRequestInternal(
     net::URLRequest* request,
     const net::CompletionCallback& callback,
     GURL* new_url) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (data_use_group_provider_) {
     // Creates and initializes a |DataUseGroup| for the |request| if it does not
     // exist. Even though we do not use the |DataUseGroup| here, we want to
@@ -234,6 +237,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeStartTransactionInternal(
     net::URLRequest* request,
     const net::CompletionCallback& callback,
     net::HttpRequestHeaders* headers) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!data_reduction_proxy_io_data_)
     return;
   if (!data_reduction_proxy_io_data_->lofi_decider())
@@ -249,6 +253,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
     const net::ProxyInfo& proxy_info,
     const net::ProxyRetryInfoMap& proxy_retry_info,
     net::HttpRequestHeaders* headers) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(data_reduction_proxy_config_);
   DCHECK(request);
 
@@ -330,6 +335,7 @@ void DataReductionProxyNetworkDelegate::OnBeforeSendHeadersInternal(
 void DataReductionProxyNetworkDelegate::OnCompletedInternal(
     net::URLRequest* request,
     bool started) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(request);
   // TODO(maksims): remove this once OnCompletedInternal() has net_error in
   // arguments.
@@ -383,6 +389,7 @@ void DataReductionProxyNetworkDelegate::CalculateAndRecordDataUsage(
     DataReductionProxyRequestType request_type,
     int64_t original_content_length,
     int net_error) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_LE(-1, original_content_length);
   int64_t data_used = request.GetTotalReceivedBytes();
 
@@ -411,6 +418,7 @@ void DataReductionProxyNetworkDelegate::AccumulateDataUsage(
     DataReductionProxyRequestType request_type,
     const scoped_refptr<DataUseGroup>& data_use_group,
     const std::string& mime_type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_GE(data_used, 0);
   DCHECK_GE(original_size, 0);
   if (data_reduction_proxy_io_data_) {
@@ -426,6 +434,7 @@ void DataReductionProxyNetworkDelegate::RecordContentLength(
     const net::URLRequest& request,
     DataReductionProxyRequestType request_type,
     int64_t original_content_length) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!request.response_headers() || request.was_cached() ||
       request.received_response_content_length() == 0) {
     return;
@@ -457,6 +466,7 @@ void DataReductionProxyNetworkDelegate::RecordContentLength(
 
 void DataReductionProxyNetworkDelegate::RecordLitePageTransformationType(
     LitePageTransformationType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.LoFi.TransformationType", type,
                             LITE_PAGE_TRANSFORMATION_TYPES_INDEX_BOUNDARY);
 }
@@ -465,6 +475,7 @@ bool DataReductionProxyNetworkDelegate::WasEligibleWithoutHoldback(
     const net::URLRequest& request,
     const net::ProxyInfo& proxy_info,
     const net::ProxyRetryInfoMap& proxy_retry_info) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(proxy_info.is_empty() || proxy_info.is_direct() ||
          !data_reduction_proxy_config_->IsDataReductionProxy(
              proxy_info.proxy_server(), nullptr));
@@ -482,6 +493,7 @@ bool DataReductionProxyNetworkDelegate::WasEligibleWithoutHoldback(
 
 void DataReductionProxyNetworkDelegate::SetDataUseGroupProvider(
     std::unique_ptr<DataUseGroupProvider> data_use_group_provider) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   data_use_group_provider_ = std::move(data_use_group_provider);
 }
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h

@@ -13,6 +13,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h"
 #include "net/base/completion_callback.h"
 #include "net/base/layered_network_delegate.h"
@@ -171,6 +172,8 @@ class DataReductionProxyNetworkDelegate : public net::LayeredNetworkDelegate {
 
   std::unique_ptr<DataUseGroupProvider> data_use_group_provider_;
 
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyNetworkDelegate);
 };
 }  // namespace data_reduction_proxy