Add HTTP response code to the Secure Proxy Check event.

Previously, only the net_error was stored - unfortunately, depending on
how a carrier and/or transparent proxy enforces the secure proxy check,
getting a net_error of success while getting a non-200 HTTP response code
is acceptable.

BUG=475267

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

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

8 files changed, 40 insertions, 25 deletions

diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
index c884740..989a4e7d 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
@@ -62,7 +62,7 @@ class SecureProxyChecker : public net::URLFetcherDelegate {
     std::string response;
     source->GetResponseAsString(&response);
 
-    fetcher_callback_.Run(response, status);
+    fetcher_callback_.Run(response, status, source->GetResponseCode());
   }
 
   void CheckIfSecureProxyIsAllowed(const GURL& secure_proxy_check_url,
@@ -370,10 +370,14 @@ void DataReductionProxyConfig::LogProxyState(bool enabled,
 }
 
 void DataReductionProxyConfig::HandleSecureProxyCheckResponse(
-    const std::string& response, const net::URLRequestStatus& status) {
+    const std::string& response,
+    const net::URLRequestStatus& status,
+    int http_response_code) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
+  bool success_response = ("OK" == response.substr(0, 2));
   if (event_store_) {
-    event_store_->EndSecureProxyCheck(bound_net_log_, status.error());
+    event_store_->EndSecureProxyCheck(bound_net_log_, status.error(),
+                                      http_response_code, success_response);
   }
 
   if (status.status() == net::URLRequestStatus::FAILED) {
@@ -388,7 +392,7 @@ void DataReductionProxyConfig::HandleSecureProxyCheckResponse(
                                 std::abs(status.error()));
   }
 
-  if ("OK" == response.substr(0, 2)) {
+  if (success_response) {
     DVLOG(1) << "The data reduction proxy is unrestricted.";
 
     if (enabled_by_user_) {
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
index 9acf691..23e67341 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
@@ -38,8 +38,9 @@ class URLRequestStatus;
 
 namespace data_reduction_proxy {
 
-typedef base::Callback<void(const std::string&, const net::URLRequestStatus&)>
-    FetcherResponseCallback;
+typedef base::Callback<void(const std::string&,
+                            const net::URLRequestStatus&,
+                            int)> FetcherResponseCallback;
 
 class DataReductionProxyConfigValues;
 class DataReductionProxyConfigurator;
@@ -229,8 +230,9 @@ class DataReductionProxyConfig
 
   // Parses the secure proxy check responses and appropriately configures the
   // Data Reduction Proxy rules.
-  virtual void HandleSecureProxyCheckResponse(
-      const std::string& response, const net::URLRequestStatus& status);
+  void HandleSecureProxyCheckResponse(const std::string& response,
+                                      const net::URLRequestStatus& status,
+                                      int http_response_code);
 
   // Adds the default proxy bypass rules for the Data Reduction Proxy.
   void AddDefaultProxyBypassRules();
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
index dbbde52..c28aaf2 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
@@ -112,9 +112,4 @@ void MockDataReductionProxyConfig::UpdateConfigurator(bool enabled,
                                                restricted, at_startup);
 }
 
-void MockDataReductionProxyConfig::HandleSecureProxyCheckResponse(
-    const std::string& response, const net::URLRequestStatus& status) {
-  DataReductionProxyConfig::HandleSecureProxyCheckResponse(response, status);
-}
-
 }  // namespace data_reduction_proxy
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
index 759b29c..d8259df 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
@@ -125,12 +125,6 @@ class MockDataReductionProxyConfig : public TestDataReductionProxyConfig {
                           bool alternative_enabled,
                           bool restricted,
                           bool at_startup) override;
-
-  // HandleSecureProxyCheckResponse should always call
-  // RecordSecureProxyCheckFetchResult exactly once.
-  void HandleSecureProxyCheckResponse(
-      const std::string& response,
-      const net::URLRequestStatus& status) override;
 };
 
 }  // namespace data_reduction_proxy
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
index 1b9c9d9..4318e59 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
@@ -13,6 +13,7 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
+#include "net/http/http_status_code.h"
 #include "net/log/test_net_log.h"
 #include "net/proxy/proxy_server.h"
 #include "net/url_request/test_url_fetcher_factory.h"
@@ -109,11 +110,12 @@ class DataReductionProxyConfigTest : public testing::Test {
   class TestResponder {
    public:
     void ExecuteCallback(FetcherResponseCallback callback) {
-      callback.Run(response, status);
+      callback.Run(response, status, http_response_code);
     }
 
     std::string response;
     net::URLRequestStatus status;
+    int http_response_code;
   };
 
   void CheckSecureProxyCheckOnIPChange(
@@ -131,6 +133,7 @@ class DataReductionProxyConfigTest : public testing::Test {
     responder.response = response;
     responder.status =
         net::URLRequestStatus(net::URLRequestStatus::SUCCESS, net::OK);
+    responder.http_response_code = net::HTTP_OK;
     EXPECT_CALL(*config(), SecureProxyCheck(_, _))
         .Times(1)
         .WillRepeatedly(testing::WithArgs<1>(
diff --git a/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc b/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
index ef12af5..9612dee 100644
--- a/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
+++ b/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
@@ -123,6 +123,17 @@ base::Value* UrlBypassTypeCallback(
   return dict;
 }
 
+base::Value* EndCanaryRequestCallback(int net_error,
+                                      int http_response_code,
+                                      bool succeeded,
+                                      net::NetLog::LogLevel /* log_level */) {
+  base::DictionaryValue* dict = new base::DictionaryValue();
+  dict->SetInteger("net_error", net_error);
+  dict->SetInteger("http_response_code", http_response_code);
+  dict->SetBoolean("check_succeeded", succeeded);
+  return dict;
+}
+
 }  // namespace
 
 namespace data_reduction_proxy {
@@ -229,9 +240,11 @@ void DataReductionProxyEventStore::BeginSecureProxyCheck(
 
 void DataReductionProxyEventStore::EndSecureProxyCheck(
     const net::BoundNetLog& net_log,
-    int net_error) {
-  const net::NetLog::ParametersCallback& parameters_callback =
-      net::NetLog::IntegerCallback("net_error", net_error);
+    int net_error,
+    int http_response_code,
+    bool succeeded) {
+  const net::NetLog::ParametersCallback& parameters_callback = base::Bind(
+      &EndCanaryRequestCallback, net_error, http_response_code, succeeded);
   PostBoundNetLogSecureProxyCheckEvent(
       net_log,
       net::NetLog::TYPE_DATA_REDUCTION_PROXY_CANARY_REQUEST,
diff --git a/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h b/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h
index ff3e3ea..3105f67 100644
--- a/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h
+++ b/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h
@@ -88,7 +88,10 @@ class DataReductionProxyEventStore {
 
   // Adds a DATA_REDUCTION_PROXY_CANARY_REQUEST event to the event store
   // when the secure proxy request has ended.
-  void EndSecureProxyCheck(const net::BoundNetLog& net_log, int net_error);
+  void EndSecureProxyCheck(const net::BoundNetLog& net_log,
+                           int net_error,
+                           int http_response_code,
+                           bool succeeded);
 
   // Creates a Value summary of Data Reduction Proxy related information:
   // - Whether the proxy is enabled
diff --git a/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store_unittest.cc b/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store_unittest.cc
index 6f266dd..f476479 100644
--- a/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store_unittest.cc
+++ b/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/test/test_simple_task_runner.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h"
+#include "net/http/http_status_code.h"
 #include "net/log/net_log.h"
 #include "net/log/test_net_log.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -125,7 +126,7 @@ TEST_F(DataReductionProxyEventStoreTest, TestBeginSecureProxyCheck) {
 TEST_F(DataReductionProxyEventStoreTest, TestEndSecureProxyCheck) {
   EXPECT_EQ(0u, proxy()->stored_events_.size());
   EXPECT_EQ(CHECK_UNKNOWN, proxy()->secure_proxy_check_state_);
-  proxy()->EndSecureProxyCheck(bound_net_log(), 0);
+  proxy()->EndSecureProxyCheck(bound_net_log(), 0, net::HTTP_OK, true);
   task_runner()->RunPendingTasks();
   EXPECT_EQ(1u, proxy()->stored_events_.size());
   EXPECT_EQ(1u, net_log()->GetSize());