Trigger data reduction proxy unreachable message via on proxy fall back.

Use the ChromeNetworkDelegate::OnProxyFallback() callback to keep track of number of times the data reduction proxies fall back because of network errors. Display a warning to users if there are network errors when connecting to the proxy and no successful requests through the proxy.

BUG=401244

Review URL: https://codereview.chromium.org/568893002

Cr-Commit-Position: refs/heads/master@{#296244}

chrome/browser/net/chrome_network_delegate.cc

@@ -447,7 +447,7 @@ void ChromeNetworkDelegate::OnResolveProxy(
 void ChromeNetworkDelegate::OnProxyFallback(const net::ProxyServer& bad_proxy,
                                             int net_error) {
   if (data_reduction_proxy_usage_stats_) {
-    data_reduction_proxy_usage_stats_->RecordBypassEventHistograms(
+    data_reduction_proxy_usage_stats_->OnProxyFallback(
         bad_proxy, net_error);
   }
 }

components/data_reduction_proxy/browser/data_reduction_proxy_params.cc

@@ -361,23 +361,6 @@ bool DataReductionProxyParams::IsDataReductionProxy(
   return false;
 }
 
-// TODO(kundaji): Check that the request will actually be sent through the
-// proxy.
-bool DataReductionProxyParams::IsDataReductionProxyEligible(
-    const net::URLRequest* request) {
-  DCHECK(request);
-  DCHECK(request->context());
-  DCHECK(request->context()->proxy_service());
-  net::ProxyInfo result;
-  request->context()->proxy_service()->config().proxy_rules().Apply(
-      request->url(), &result);
-  if (!result.proxy_server().is_valid())
-    return false;
-  if (result.proxy_server().is_direct())
-    return false;
-  return IsDataReductionProxy(result.proxy_server().host_port_pair(), NULL);
-}
-
 bool DataReductionProxyParams::IsBypassedByDataReductionProxyLocalRules(
     const net::URLRequest& request,
     const net::ProxyConfig& data_reduction_proxy_config) const {

components/data_reduction_proxy/browser/data_reduction_proxy_params.h

@@ -130,11 +130,6 @@ class DataReductionProxyParams {
       const net::HostPortPair& host_port_pair,
       DataReductionProxyTypeInfo* proxy_info) const;
 
-  // Returns true if this request will be sent through the data request proxy
-  // based on applying the param rules to the URL. We do not check bad proxy
-  // list.
-  virtual bool IsDataReductionProxyEligible(const net::URLRequest* request);
-
   // Returns true if this request would be bypassed by the data request proxy
   // based on applying the |data_reduction_proxy_config| param rules to the
   // request URL.

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc

@@ -28,6 +28,10 @@ namespace data_reduction_proxy {
 
 namespace {
 
+const int kMinFailedRequestsWhenUnavailable = 1;
+const int kMaxSuccessfulRequestsWhenUnavailable = 0;
+const int kMaxFailedRequestsBeforeReset = 3;
+
 // Records a net error code that resulted in bypassing the data reduction
 // proxy (|is_primary| is true) or the data reduction proxy fallback.
 void RecordDataReductionProxyBypassOnNetworkError(
@@ -79,9 +83,11 @@ DataReductionProxyUsageStats::DataReductionProxyUsageStats(
       last_bypass_type_(BYPASS_EVENT_TYPE_MAX),
       triggering_request_(true),
       ui_thread_proxy_(ui_thread_proxy),
-      eligible_num_requests_through_proxy_(0),
-      actual_num_requests_through_proxy_(0),
+      successful_requests_through_proxy_count_(0),
+      proxy_net_errors_count_(0),
       unavailable_(false) {
+  DCHECK(params);
+
   NetworkChangeNotifier::AddNetworkChangeObserver(this);
 };
 
@@ -93,13 +99,10 @@ void DataReductionProxyUsageStats::OnUrlRequestCompleted(
     const net::URLRequest* request, bool started) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (request->status().status() == net::URLRequestStatus::SUCCESS) {
-    if (data_reduction_proxy_params_->IsDataReductionProxyEligible(request)) {
-      bool was_received_via_proxy =
-          data_reduction_proxy_params_->WasDataReductionProxyUsed(
-              request, NULL);
-      IncrementRequestCounts(was_received_via_proxy);
-    }
+  if (request->status().status() == net::URLRequestStatus::SUCCESS &&
+      data_reduction_proxy_params_->WasDataReductionProxyUsed(request, NULL)) {
+    successful_requests_through_proxy_count_++;
+    NotifyUnavailabilityIfChanged();
   }
 }
 
@@ -109,40 +112,23 @@ void DataReductionProxyUsageStats::OnNetworkChanged(
   ClearRequestCounts();
 }
 
-void DataReductionProxyUsageStats::IncrementRequestCounts(
-    bool was_received_via_proxy) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (was_received_via_proxy) {
-    actual_num_requests_through_proxy_++;
-  }
-  eligible_num_requests_through_proxy_++;
-
-  // To account for the case when the proxy works for a little while and then
-  // gets blocked, we reset the counts occasionally.
-  if (eligible_num_requests_through_proxy_ > 50
-      && actual_num_requests_through_proxy_ > 0) {
-    ClearRequestCounts();
-  } else {
-    MaybeNotifyUnavailability();
-  }
-}
-
 void DataReductionProxyUsageStats::ClearRequestCounts() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  eligible_num_requests_through_proxy_ = 0;
-  actual_num_requests_through_proxy_ = 0;
-  MaybeNotifyUnavailability();
+  successful_requests_through_proxy_count_ = 0;
+  proxy_net_errors_count_ = 0;
 }
 
-void DataReductionProxyUsageStats::MaybeNotifyUnavailability() {
+void DataReductionProxyUsageStats::NotifyUnavailabilityIfChanged() {
   bool prev_unavailable = unavailable_;
-  unavailable_ = (eligible_num_requests_through_proxy_ > 0 &&
-      actual_num_requests_through_proxy_ == 0);
+  unavailable_ =
+      (proxy_net_errors_count_ >= kMinFailedRequestsWhenUnavailable &&
+          successful_requests_through_proxy_count_ <=
+              kMaxSuccessfulRequestsWhenUnavailable);
   if (prev_unavailable != unavailable_) {
-     ui_thread_proxy_->PostTask(FROM_HERE, base::Bind(
-          &DataReductionProxyUsageStats::NotifyUnavailabilityOnUIThread,
-          base::Unretained(this),
-          unavailable_));
+    ui_thread_proxy_->PostTask(FROM_HERE, base::Bind(
+        &DataReductionProxyUsageStats::NotifyUnavailabilityOnUIThread,
+        base::Unretained(this),
+        unavailable_));
   }
 }
 
@@ -240,15 +226,29 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
   }
 }
 
-void DataReductionProxyUsageStats::RecordBypassEventHistograms(
+void DataReductionProxyUsageStats::OnProxyFallback(
     const net::ProxyServer& bypassed_proxy,
-    int net_error) const {
+    int net_error) {
   DataReductionProxyTypeInfo data_reduction_proxy_info;
   if (bypassed_proxy.is_valid() && !bypassed_proxy.is_direct() &&
       data_reduction_proxy_params_->IsDataReductionProxy(
       bypassed_proxy.host_port_pair(), &data_reduction_proxy_info)) {
     if (data_reduction_proxy_info.is_ssl)
       return;
+
+    proxy_net_errors_count_++;
+
+    // To account for the case when the proxy is reachable for sometime, and
+    // then gets blocked, we reset counts when number of errors exceed
+    // the threshold.
+    if (proxy_net_errors_count_ >= kMaxFailedRequestsBeforeReset &&
+        successful_requests_through_proxy_count_ >
+            kMaxSuccessfulRequestsWhenUnavailable) {
+      ClearRequestCounts();
+    } else {
+      NotifyUnavailabilityIfChanged();
+    }
+
     if (!data_reduction_proxy_info.is_fallback) {
       RecordDataReductionProxyBypassInfo(
           true, false, bypassed_proxy, BYPASS_EVENT_TYPE_NETWORK_ERROR);

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h

@@ -65,8 +65,10 @@ class DataReductionProxyUsageStats
       const BooleanPrefMember& data_reduction_proxy_enabled,
       const net::ProxyConfig& data_reduction_proxy_config);
 
-  void RecordBypassEventHistograms(const net::ProxyServer& bypassed_proxy,
-                                   int net_error) const;
+  // Called by |ChromeNetworkDelegate| when a proxy is put into the bad proxy
+  // list. Used to track when the data reduction proxy falls back.
+  void OnProxyFallback(const net::ProxyServer& bypassed_proxy,
+                       int net_error);
 
  private:
   enum BypassedBytesType {
@@ -84,9 +86,11 @@ class DataReductionProxyUsageStats
   virtual void OnNetworkChanged(
       net::NetworkChangeNotifier::ConnectionType type) OVERRIDE;
 
-  // Counts requests that went through the data reduction proxy and counts
-  // requests that were eligible to go through the proxy.
-  void IncrementRequestCounts(bool actual);
+  // Called when request counts change. Resets counts if they exceed thresholds,
+  // and calls MaybeNotifyUnavailability otherwise.
+  void OnRequestCountChanged();
+
+  // Clears request counts unconditionally.
   void ClearRequestCounts();
 
   // Checks if the availability status of the data reduction proxy has changed,
@@ -94,7 +98,7 @@ class DataReductionProxyUsageStats
   // data reduction proxy is considered unavailable if and only if no requests
   // went through the proxy but some eligible requests were service by other
   // routes.
-  void MaybeNotifyUnavailability();
+  void NotifyUnavailabilityIfChanged();
   void NotifyUnavailabilityOnUIThread(bool unavailable);
 
   DataReductionProxyParams* data_reduction_proxy_params_;
@@ -111,14 +115,12 @@ class DataReductionProxyUsageStats
   // unreachable if no successful requests are made through it despite a
   // non-zero number of requests being eligible.
 
-  // Count of requests which will be tried to be sent through data reduction
-  // proxy. The count is only based on the config and not the bad proxy list.
-  // Explicit bypasses are not part of this count. This is the desired behavior
-  // since otherwise both counts would be identical.
-  unsigned long eligible_num_requests_through_proxy_;
+  // Count of successful requests through the data reduction proxy.
+  unsigned long successful_requests_through_proxy_count_;
 
-  // Count of successful requests through data reduction proxy.
-  unsigned long actual_num_requests_through_proxy_;
+  // Count of network errors encountered when connecting to a data reduction
+  // proxy.
+  unsigned long proxy_net_errors_count_;
 
   // Whether or not the data reduction proxy is unavailable.
   bool unavailable_;

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
+#include "net/base/host_port_pair.h"
 #include "net/base/request_priority.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_status.h"
@@ -13,27 +14,25 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using base::MessageLoop;
-using base::MessageLoopProxy;
-using data_reduction_proxy::DataReductionProxyParams;
-using net::TestDelegate;
-using net::TestURLRequestContext;
-using net::URLRequest;
-using net::URLRequestStatus;
 using testing::Return;
 
 namespace {
 
-class DataReductionProxyParamsMock : public DataReductionProxyParams {
+class DataReductionProxyParamsMock :
+    public data_reduction_proxy::DataReductionProxyParams {
  public:
-  DataReductionProxyParamsMock() : DataReductionProxyParams(0) {}
+  DataReductionProxyParamsMock() :
+      data_reduction_proxy::DataReductionProxyParams(0) {}
   virtual ~DataReductionProxyParamsMock() {}
 
-  MOCK_METHOD1(IsDataReductionProxyEligible, bool(const net::URLRequest*));
+  MOCK_CONST_METHOD2(
+      IsDataReductionProxy,
+      bool(const net::HostPortPair& host_port_pair,
+           data_reduction_proxy::DataReductionProxyTypeInfo* proxy_info));
   MOCK_CONST_METHOD2(
       WasDataReductionProxyUsed,
       bool(const net::URLRequest*,
-           data_reduction_proxy::DataReductionProxyTypeInfo* proxy_servers));
+           data_reduction_proxy::DataReductionProxyTypeInfo* proxy_info));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyParamsMock);
@@ -46,7 +45,7 @@ namespace data_reduction_proxy {
 class DataReductionProxyUsageStatsTest : public testing::Test {
  public:
   DataReductionProxyUsageStatsTest()
-      : loop_proxy_(MessageLoopProxy::current().get()),
+      : loop_proxy_(base::MessageLoopProxy::current().get()),
         context_(true),
         unavailable_(false) {
     context_.Init();
@@ -58,21 +57,24 @@ class DataReductionProxyUsageStatsTest : public testing::Test {
     unavailable_ = unavailable;
   }
 
-  // Required for MessageLoopProxy::current().
+  // Required for base::MessageLoopProxy::current().
   base::MessageLoopForUI loop_;
-  MessageLoopProxy* loop_proxy_;
+  base::MessageLoopProxy* loop_proxy_;
 
  protected:
-  TestURLRequestContext context_;
-  TestDelegate delegate_;
+  net::TestURLRequestContext context_;
+  net::TestDelegate delegate_;
   DataReductionProxyParamsMock mock_params_;
-  scoped_ptr<URLRequest> mock_url_request_;
+  scoped_ptr<net::URLRequest> mock_url_request_;
   bool unavailable_;
 };
 
 TEST_F(DataReductionProxyUsageStatsTest, IsDataReductionProxyUnreachable) {
+  net::ProxyServer fallback_proxy_server =
+      net::ProxyServer::FromURI("foo.com", net::ProxyServer::SCHEME_HTTP);
+  data_reduction_proxy::DataReductionProxyTypeInfo proxy_info;
   struct TestCase {
-    bool is_proxy_eligible;
+    bool fallback_proxy_server_is_data_reduction_proxy;
     bool was_proxy_used;
     bool is_unreachable;
   };
@@ -82,6 +84,11 @@ TEST_F(DataReductionProxyUsageStatsTest, IsDataReductionProxyUnreachable) {
       false,
       false
     },
+    {
+      false,
+      true,
+      false
+    },
     {
       true,
       true,
@@ -96,12 +103,12 @@ TEST_F(DataReductionProxyUsageStatsTest, IsDataReductionProxyUnreachable) {
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
     TestCase test_case = test_cases[i];
 
-    EXPECT_CALL(mock_params_,
-                IsDataReductionProxyEligible(mock_url_request_.get()))
-                    .WillRepeatedly(Return(test_case.is_proxy_eligible));
+    EXPECT_CALL(mock_params_, IsDataReductionProxy(testing::_, testing::_))
+        .WillRepeatedly(testing::Return(
+            test_case.fallback_proxy_server_is_data_reduction_proxy));
     EXPECT_CALL(mock_params_,
                 WasDataReductionProxyUsed(mock_url_request_.get(), NULL))
-        .WillRepeatedly(Return(test_case.was_proxy_used));
+        .WillRepeatedly(testing::Return(test_case.was_proxy_used));
 
     scoped_ptr<DataReductionProxyUsageStats> usage_stats(
         new DataReductionProxyUsageStats(
@@ -110,11 +117,74 @@ TEST_F(DataReductionProxyUsageStatsTest, IsDataReductionProxyUnreachable) {
         base::Bind(&DataReductionProxyUsageStatsTest::NotifyUnavailable,
                    base::Unretained(this)));
 
+    usage_stats->OnProxyFallback(fallback_proxy_server,
+                                 net::ERR_PROXY_CONNECTION_FAILED);
     usage_stats->OnUrlRequestCompleted(mock_url_request_.get(), false);
-    MessageLoop::current()->RunUntilIdle();
+    base::MessageLoop::current()->RunUntilIdle();
 
     EXPECT_EQ(test_case.is_unreachable, unavailable_);
   }
 }
 
+TEST_F(DataReductionProxyUsageStatsTest, ProxyUnreachableThenReachable) {
+  net::ProxyServer fallback_proxy_server =
+      net::ProxyServer::FromURI("foo.com", net::ProxyServer::SCHEME_HTTP);
+  scoped_ptr<DataReductionProxyUsageStats> usage_stats(
+      new DataReductionProxyUsageStats(
+          &mock_params_, loop_proxy_));
+  usage_stats->set_unavailable_callback(
+      base::Bind(&DataReductionProxyUsageStatsTest::NotifyUnavailable,
+                 base::Unretained(this)));
+
+  EXPECT_CALL(mock_params_, IsDataReductionProxy(testing::_, testing::_))
+      .WillOnce(testing::Return(true));
+  EXPECT_CALL(mock_params_,
+              WasDataReductionProxyUsed(mock_url_request_.get(), NULL))
+      .WillOnce(testing::Return(true));
+
+  // proxy falls back
+  usage_stats->OnProxyFallback(fallback_proxy_server,
+                               net::ERR_PROXY_CONNECTION_FAILED);
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_TRUE(unavailable_);
+
+  // proxy succeeds
+  usage_stats->OnUrlRequestCompleted(mock_url_request_.get(), false);
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_FALSE(unavailable_);
+}
+
+TEST_F(DataReductionProxyUsageStatsTest, ProxyReachableThenUnreachable) {
+  net::ProxyServer fallback_proxy_server =
+      net::ProxyServer::FromURI("foo.com", net::ProxyServer::SCHEME_HTTP);
+  scoped_ptr<DataReductionProxyUsageStats> usage_stats(
+      new DataReductionProxyUsageStats(
+          &mock_params_, loop_proxy_));
+  usage_stats->set_unavailable_callback(
+      base::Bind(&DataReductionProxyUsageStatsTest::NotifyUnavailable,
+                 base::Unretained(this)));
+  EXPECT_CALL(mock_params_,
+              WasDataReductionProxyUsed(mock_url_request_.get(), NULL))
+      .WillOnce(testing::Return(true));
+  EXPECT_CALL(mock_params_, IsDataReductionProxy(testing::_, testing::_))
+      .WillRepeatedly(testing::Return(true));
+
+  // Proxy succeeds.
+  usage_stats->OnUrlRequestCompleted(mock_url_request_.get(), false);
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_FALSE(unavailable_);
+
+  // Then proxy falls back indefinitely.
+  usage_stats->OnProxyFallback(fallback_proxy_server,
+                               net::ERR_PROXY_CONNECTION_FAILED);
+  usage_stats->OnProxyFallback(fallback_proxy_server,
+                                 net::ERR_PROXY_CONNECTION_FAILED);
+  usage_stats->OnProxyFallback(fallback_proxy_server,
+                                 net::ERR_PROXY_CONNECTION_FAILED);
+  usage_stats->OnProxyFallback(fallback_proxy_server,
+                                 net::ERR_PROXY_CONNECTION_FAILED);
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_TRUE(unavailable_);
+}
+
 }  // namespace data_reduction_proxy