shill: Add portal metrics

Add support for Network.*.PortalAttempts,
Network.*.PortalAttemptsToOnline and Network.*.PortalResult.

BUG=chromium-os:27670
TEST=Unit tests

Change-Id: I0589c0f811a46f249ebb97540fc9d8a6ed7293ad
Reviewed-on: https://gerrit.chromium.org/gerrit/18036
Commit-Ready: Thieu Le <thieule@chromium.org>
Reviewed-by: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
diff --git a/device.cc b/device.cc
index 0d63170..d2be40e 100644
--- a/device.cc
+++ b/device.cc
@@ -28,6 +28,7 @@
 #include "shill/event_dispatcher.h"
 #include "shill/http_proxy.h"
 #include "shill/manager.h"
+#include "shill/metrics.h"
 #include "shill/property_accessor.h"
 #include "shill/refptr_types.h"
 #include "shill/routing_table.h"
@@ -92,6 +93,7 @@
       portal_detector_callback_(
           Bind(&Device::PortalDetectorCallback, Unretained(this))),
       technology_(technology),
+      portal_attempts_to_online_(0),
       dhcp_provider_(DHCPProvider::GetInstance()),
       routing_table_(RoutingTable::GetInstance()),
       rtnl_handler_(RTNLHandler::GetInstance()) {
@@ -364,6 +366,7 @@
     // time we report the state change to the manager, the service
     // has its connection.
     SetServiceState(Service::kStateConnected);
+    portal_attempts_to_online_ = 0;
     // Subtle: Start portal detection after transitioning the service
     // to the Connected state because this call may immediately transition
     // to the Online state.
@@ -583,10 +586,33 @@
           << ": received final status: "
           << PortalDetector::StatusToString(result.status);
 
+  portal_attempts_to_online_ += result.num_attempts;
+
+  metrics()->SendEnumToUMA(
+      metrics()->GetFullMetricName(Metrics::kMetricPortalResult, technology()),
+      Metrics::PortalDetectionResultToEnum(result),
+      Metrics::kPortalResultMax);
+
   if (result.status == PortalDetector::kStatusSuccess) {
     SetServiceConnectedState(Service::kStateOnline);
+
+    metrics()->SendToUMA(
+        metrics()->GetFullMetricName(
+            Metrics::kMetricPortalAttemptsToOnline, technology()),
+        portal_attempts_to_online_,
+        Metrics::kMetricPortalAttemptsToOnlineMin,
+        Metrics::kMetricPortalAttemptsToOnlineMax,
+        Metrics::kMetricPortalAttemptsToOnlineNumBuckets);
   } else {
     SetServiceConnectedState(Service::kStatePortal);
+
+    metrics()->SendToUMA(
+        metrics()->GetFullMetricName(
+            Metrics::kMetricPortalAttempts, technology()),
+        result.num_attempts,
+        Metrics::kMetricPortalAttemptsMin,
+        Metrics::kMetricPortalAttemptsMax,
+        Metrics::kMetricPortalAttemptsNumBuckets);
   }
 }
 
diff --git a/device.h b/device.h
index 0c7f22d..76ed0e4 100644
--- a/device.h
+++ b/device.h
@@ -266,6 +266,10 @@
   base::Callback<void(const PortalDetector::Result &)>
       portal_detector_callback_;
   Technology::Identifier technology_;
+  // The number of portal detection attempts from Connected to Online state.
+  // This includes all failure/timeout attempts and the final successful
+  // attempt.
+  int portal_attempts_to_online_;
 
   // Maintain a reference to the connected / connecting service
   ServiceRefPtr selected_service_;
diff --git a/device_unittest.cc b/device_unittest.cc
index 01a1caa..2f4dd17 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -28,6 +28,7 @@
 #include "shill/mock_glib.h"
 #include "shill/mock_ipconfig.h"
 #include "shill/mock_manager.h"
+#include "shill/mock_metrics.h"
 #include "shill/mock_portal_detector.h"
 #include "shill/mock_routing_table.h"
 #include "shill/mock_rtnl_handler.h"
@@ -42,6 +43,7 @@
 using std::vector;
 using ::testing::_;
 using ::testing::AtLeast;
+using ::testing::Mock;
 using ::testing::NiceMock;
 using ::testing::Return;
 using ::testing::ReturnRef;
@@ -69,6 +71,7 @@
   virtual ~DeviceTest() {}
 
   virtual void SetUp() {
+    device_->metrics_ = &metrics_;
     device_->routing_table_ = &routing_table_;
     device_->rtnl_handler_ = &rtnl_handler_;
   }
@@ -93,6 +96,7 @@
   MockControl control_interface_;
   DeviceRefPtr device_;
   MockDeviceInfo device_info_;
+  MockMetrics metrics_;
   MockRoutingTable routing_table_;
   StrictMock<MockRTNLHandler> rtnl_handler_;
 };
@@ -354,6 +358,8 @@
   }
 
  protected:
+  static const int kPortalAttempts;
+
   bool StartPortalDetection() { return device_->StartPortalDetection(); }
   void StopPortalDetection() { device_->StopPortalDetection(); }
 
@@ -383,6 +389,8 @@
   MockPortalDetector *portal_detector_;
 };
 
+const int DevicePortalDetectionTest::kPortalAttempts = 2;
+
 TEST_F(DevicePortalDetectionTest, PortalDetectionDisabled) {
   EXPECT_CALL(*service_.get(), IsConnected())
       .WillRepeatedly(Return(true));
@@ -450,6 +458,7 @@
   PortalDetectorCallback(PortalDetector::Result(
       PortalDetector::kPhaseUnknown,
       PortalDetector::kStatusFailure,
+      kPortalAttempts,
       false));
 }
 
@@ -457,11 +466,25 @@
   EXPECT_CALL(*service_.get(), IsConnected())
       .WillOnce(Return(true));
   EXPECT_CALL(*service_.get(), SetState(Service::kStatePortal));
+  EXPECT_CALL(metrics_,
+              SendEnumToUMA("Network.Shill.Unknown.PortalResult",
+                            Metrics::kPortalResultConnectionFailure,
+                            Metrics::kPortalResultMax));
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttemptsToOnline",
+                        _, _, _, _)).Times(0);
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttempts",
+                        kPortalAttempts,
+                        Metrics::kMetricPortalAttemptsMin,
+                        Metrics::kMetricPortalAttemptsMax,
+                        Metrics::kMetricPortalAttemptsNumBuckets));
   EXPECT_CALL(*connection_.get(), is_default())
       .WillOnce(Return(false));
   PortalDetectorCallback(PortalDetector::Result(
-      PortalDetector::kPhaseUnknown,
+      PortalDetector::kPhaseConnection,
       PortalDetector::kStatusFailure,
+      kPortalAttempts,
       true));
 }
 
@@ -469,9 +492,70 @@
   EXPECT_CALL(*service_.get(), IsConnected())
       .WillOnce(Return(true));
   EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
+  EXPECT_CALL(metrics_,
+              SendEnumToUMA("Network.Shill.Unknown.PortalResult",
+                            Metrics::kPortalResultSuccess,
+                            Metrics::kPortalResultMax));
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttemptsToOnline",
+                        kPortalAttempts,
+                        Metrics::kMetricPortalAttemptsToOnlineMin,
+                        Metrics::kMetricPortalAttemptsToOnlineMax,
+                        Metrics::kMetricPortalAttemptsToOnlineNumBuckets));
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttempts",
+                        _, _, _, _)).Times(0);
   PortalDetectorCallback(PortalDetector::Result(
-      PortalDetector::kPhaseUnknown,
+      PortalDetector::kPhaseContent,
       PortalDetector::kStatusSuccess,
+      kPortalAttempts,
+      true));
+}
+
+TEST_F(DevicePortalDetectionTest, PortalDetectionSuccessAfterFailure) {
+  EXPECT_CALL(*service_.get(), IsConnected())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*service_.get(), SetState(Service::kStatePortal));
+  EXPECT_CALL(metrics_,
+              SendEnumToUMA("Network.Shill.Unknown.PortalResult",
+                            Metrics::kPortalResultConnectionFailure,
+                            Metrics::kPortalResultMax));
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttemptsToOnline",
+                        _, _, _, _)).Times(0);
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttempts",
+                        kPortalAttempts,
+                        Metrics::kMetricPortalAttemptsMin,
+                        Metrics::kMetricPortalAttemptsMax,
+                        Metrics::kMetricPortalAttemptsNumBuckets));
+  EXPECT_CALL(*connection_.get(), is_default())
+      .WillOnce(Return(false));
+  PortalDetectorCallback(PortalDetector::Result(
+      PortalDetector::kPhaseConnection,
+      PortalDetector::kStatusFailure,
+      kPortalAttempts,
+      true));
+  Mock::VerifyAndClearExpectations(&metrics_);
+
+  EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
+  EXPECT_CALL(metrics_,
+              SendEnumToUMA("Network.Shill.Unknown.PortalResult",
+                            Metrics::kPortalResultSuccess,
+                            Metrics::kPortalResultMax));
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttemptsToOnline",
+                        kPortalAttempts * 2,
+                        Metrics::kMetricPortalAttemptsToOnlineMin,
+                        Metrics::kMetricPortalAttemptsToOnlineMax,
+                        Metrics::kMetricPortalAttemptsToOnlineNumBuckets));
+  EXPECT_CALL(metrics_,
+              SendToUMA("Network.Shill.Unknown.PortalAttempts",
+                        _, _, _, _)).Times(0);
+  PortalDetectorCallback(PortalDetector::Result(
+      PortalDetector::kPhaseContent,
+      PortalDetector::kStatusSuccess,
+      kPortalAttempts,
       true));
 }
 
diff --git a/metrics.cc b/metrics.cc
index 2ba58d9..48f5293 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -55,6 +55,22 @@
 const int Metrics::kTimerHistogramMillisecondsMin = 1;
 const int Metrics::kTimerHistogramNumBuckets = 50;
 
+const char Metrics::kMetricPortalAttempts[] =
+    "Network.Shill.%s.PortalAttempts";
+const int Metrics::kMetricPortalAttemptsMax =
+    PortalDetector::kMaxRequestAttempts;
+const int Metrics::kMetricPortalAttemptsMin = 1;
+const int Metrics::kMetricPortalAttemptsNumBuckets =
+    Metrics::kMetricPortalAttemptsMax;
+
+const char Metrics::kMetricPortalAttemptsToOnline[] =
+    "Network.Shill.%s.PortalAttemptsToOnline";
+const int Metrics::kMetricPortalAttemptsToOnlineMax = 100;
+const int Metrics::kMetricPortalAttemptsToOnlineMin = 1;
+const int Metrics::kMetricPortalAttemptsToOnlineNumBuckets = 10;
+
+const char Metrics::kMetricPortalResult[] = "Network.Shill.%s.PortalResult";
+
 // static
 const uint16 Metrics::kWiFiBandwidth5MHz = 5;
 const uint16 Metrics::kWiFiBandwidth20MHz = 20;
@@ -156,6 +172,69 @@
   }
 }
 
+// static
+Metrics::PortalResult Metrics::PortalDetectionResultToEnum(
+      const PortalDetector::Result &result) {
+  DCHECK(result.final);
+  PortalResult retval = kPortalResultUnknown;
+  // The only time we should end a successful portal detection is when we're
+  // in the Content phase.  If we end with kStatusSuccess in any other phase,
+  // then this indicates that something bad has happened.
+  switch (result.phase) {
+    case PortalDetector::kPhaseDNS:
+      if (result.status == PortalDetector::kStatusFailure)
+        retval = kPortalResultDNSFailure;
+      else if (result.status == PortalDetector::kStatusTimeout)
+        retval = kPortalResultDNSTimeout;
+      else
+        LOG(DFATAL) << __func__ << ": Final result status " << result.status
+                    << " is not allowed in the DNS phase";
+      break;
+
+    case PortalDetector::kPhaseConnection:
+      if (result.status == PortalDetector::kStatusFailure)
+        retval = kPortalResultConnectionFailure;
+      else if (result.status == PortalDetector::kStatusTimeout)
+        retval = kPortalResultConnectionTimeout;
+      else
+        LOG(DFATAL) << __func__ << ": Final result status " << result.status
+                    << " is not allowed in the Connection phase";
+      break;
+
+    case PortalDetector::kPhaseHTTP:
+      if (result.status == PortalDetector::kStatusFailure)
+        retval = kPortalResultHTTPFailure;
+      else if (result.status == PortalDetector::kStatusTimeout)
+        retval = kPortalResultHTTPTimeout;
+      else
+        LOG(DFATAL) << __func__ << ": Final result status " << result.status
+                    << " is not allowed in the HTTP phase";
+      break;
+
+    case PortalDetector::kPhaseContent:
+      if (result.status == PortalDetector::kStatusSuccess)
+        retval = kPortalResultSuccess;
+      else if (result.status == PortalDetector::kStatusFailure)
+        retval = kPortalResultContentFailure;
+      else if (result.status == PortalDetector::kStatusTimeout)
+        retval = kPortalResultContentTimeout;
+      else
+        LOG(DFATAL) << __func__ << ": Final result status " << result.status
+                    << " is not allowed in the Content phase";
+      break;
+
+    case PortalDetector::kPhaseUnknown:
+      retval = kPortalResultUnknown;
+      break;
+
+    default:
+      LOG(DFATAL) << __func__ << ": Invalid phase " << result.phase;
+      break;
+  }
+
+  return retval;
+}
+
 void Metrics::RegisterService(const Service *service) {
   shared_ptr<ServiceMetrics> service_metrics(new ServiceMetrics);
   services_metrics_[service] = service_metrics;
diff --git a/metrics.h b/metrics.h
index 125fc22..d4e7e69 100644
--- a/metrics.h
+++ b/metrics.h
@@ -12,6 +12,7 @@
 #include <metrics/metrics_library.h>
 #include <metrics/timer.h>
 
+#include "shill/portal_detector.h"
 #include "shill/power_manager.h"
 #include "shill/refptr_types.h"
 #include "shill/service.h"
@@ -101,6 +102,21 @@
     kWiFiSecurityMax
   };
 
+  enum PortalResult {
+    kPortalResultSuccess = 0,
+    kPortalResultDNSFailure = 1,
+    kPortalResultDNSTimeout = 2,
+    kPortalResultConnectionFailure = 3,
+    kPortalResultConnectionTimeout = 4,
+    kPortalResultHTTPFailure = 5,
+    kPortalResultHTTPTimeout = 6,
+    kPortalResultContentFailure = 7,
+    kPortalResultContentTimeout = 8,
+    kPortalResultUnknown = 9,
+
+    kPortalResultMax
+  };
+
   static const char kMetricDisconnect[];
   static const int kMetricDisconnectMax;
   static const int kMetricDisconnectMin;
@@ -129,6 +145,25 @@
   static const int kTimerHistogramMillisecondsMin;
   static const int kTimerHistogramNumBuckets;
 
+  // The number of portal detections attempted for each pass.
+  // This includes both failure/timeout attempts and successful attempt
+  // (if any).
+  static const char kMetricPortalAttempts[];
+  static const int kMetricPortalAttemptsMax;
+  static const int kMetricPortalAttemptsMin;
+  static const int kMetricPortalAttemptsNumBuckets;
+
+  // The total number of portal detections attempted between the Connected
+  // state and the Online state.  This includes both failure/timeout attempts
+  // and the final successful attempt.
+  static const char kMetricPortalAttemptsToOnline[];
+  static const int kMetricPortalAttemptsToOnlineMax;
+  static const int kMetricPortalAttemptsToOnlineMin;
+  static const int kMetricPortalAttemptsToOnlineNumBuckets;
+
+  // The result of the portal detection.
+  static const char kMetricPortalResult[];
+
   static const char kMetricPowerManagerKey[];
 
   Metrics();
@@ -140,6 +175,10 @@
   // Converts a flimflam security string into its UMA security enumerator.
   static WiFiSecurity WiFiSecurityStringToEnum(const std::string &security);
 
+  // Converts portal detection result to UMA portal result enumerator.
+  static PortalResult PortalDetectionResultToEnum(
+      const PortalDetector::Result &result);
+
   // Registers a service with this object so it can use the timers to track
   // state transition metrics.
   void RegisterService(const Service *service);
@@ -174,11 +213,11 @@
   void NotifyPowerStateChange(PowerManager::SuspendState new_state);
 
   // Sends linear histogram data to UMA.
-  bool SendEnumToUMA(const std::string &name, int sample, int max);
+  virtual bool SendEnumToUMA(const std::string &name, int sample, int max);
 
   // Send histogram data to UMA.
-  bool SendToUMA(const std::string &name, int sample, int min, int max,
-                 int num_buckets);
+  virtual bool SendToUMA(const std::string &name, int sample, int min,
+                         int max, int num_buckets);
 
  private:
   friend struct base::DefaultLazyInstanceTraits<Metrics>;
diff --git a/metrics_unittest.cc b/metrics_unittest.cc
index 49b89da..3c4aa6c 100644
--- a/metrics_unittest.cc
+++ b/metrics_unittest.cc
@@ -265,4 +265,83 @@
   metrics_.NotifyServiceDisconnect(service_);
 }
 
+TEST_F(MetricsTest, PortalDetectionResultToEnum) {
+  PortalDetector::Result result(PortalDetector::kPhaseDNS,
+                                PortalDetector::kStatusFailure, 0, true);
+  EXPECT_EQ(Metrics::kPortalResultDNSFailure,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseDNS;
+  result.status = PortalDetector::kStatusTimeout;
+  EXPECT_EQ(Metrics::kPortalResultDNSTimeout,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseConnection;
+  result.status = PortalDetector::kStatusFailure;
+  EXPECT_EQ(Metrics::kPortalResultConnectionFailure,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseConnection;
+  result.status = PortalDetector::kStatusTimeout;
+  EXPECT_EQ(Metrics::kPortalResultConnectionTimeout,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseHTTP;
+  result.status = PortalDetector::kStatusFailure;
+  EXPECT_EQ(Metrics::kPortalResultHTTPFailure,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseHTTP;
+  result.status = PortalDetector::kStatusTimeout;
+  EXPECT_EQ(Metrics::kPortalResultHTTPTimeout,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseContent;
+  result.status = PortalDetector::kStatusSuccess;
+  EXPECT_EQ(Metrics::kPortalResultSuccess,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseContent;
+  result.status = PortalDetector::kStatusFailure;
+  EXPECT_EQ(Metrics::kPortalResultContentFailure,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseContent;
+  result.status = PortalDetector::kStatusTimeout;
+  EXPECT_EQ(Metrics::kPortalResultContentTimeout,
+            Metrics::PortalDetectionResultToEnum(result));
+
+  result.phase = PortalDetector::kPhaseUnknown;
+  result.status = PortalDetector::kStatusFailure;
+  EXPECT_EQ(Metrics::kPortalResultUnknown,
+            Metrics::PortalDetectionResultToEnum(result));
+}
+
+#ifndef NDEBUG
+
+typedef MetricsTest MetricsDeathTest;
+
+TEST_F(MetricsDeathTest, PortalDetectionResultToEnumDNSSuccess) {
+  PortalDetector::Result result(PortalDetector::kPhaseDNS,
+                                PortalDetector::kStatusSuccess, 0, true);
+  EXPECT_DEATH(Metrics::PortalDetectionResultToEnum(result),
+               "Final result status 1 is not allowed in the DNS phase");
+}
+
+TEST_F(MetricsDeathTest, PortalDetectionResultToEnumConnectionSuccess) {
+  PortalDetector::Result result(PortalDetector::kPhaseConnection,
+                                PortalDetector::kStatusSuccess, 0, true);
+  EXPECT_DEATH(Metrics::PortalDetectionResultToEnum(result),
+               "Final result status 1 is not allowed in the Connection phase");
+}
+
+TEST_F(MetricsDeathTest, PortalDetectionResultToEnumHTTPSuccess) {
+  PortalDetector::Result result(PortalDetector::kPhaseHTTP,
+                                PortalDetector::kStatusSuccess, 0, true);
+  EXPECT_DEATH(Metrics::PortalDetectionResultToEnum(result),
+               "Final result status 1 is not allowed in the HTTP phase");
+}
+
+#endif  // NDEBUG
+
 }  // namespace shill
diff --git a/mock_metrics.h b/mock_metrics.h
index d9c7c0d..64b5132 100644
--- a/mock_metrics.h
+++ b/mock_metrics.h
@@ -17,6 +17,10 @@
   virtual ~MockMetrics();
 
   MOCK_METHOD1(NotifyDefaultServiceChanged, void(const Service *service));
+  MOCK_METHOD3(SendEnumToUMA, bool(const std::string &name, int sample,
+                                   int max));
+  MOCK_METHOD5(SendToUMA, bool(const std::string &name, int sample, int min,
+                               int max, int num_buckets));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockMetrics);
diff --git a/portal_detector.cc b/portal_detector.cc
index 73e9bdd..33bb4d1 100644
--- a/portal_detector.cc
+++ b/portal_detector.cc
@@ -143,6 +143,7 @@
   if (result.status != kStatusSuccess && attempt_count_ < kMaxRequestAttempts) {
     StartAttempt(0);
   } else {
+    result.num_attempts = attempt_count_;
     result.final = true;
     Stop();
   }
diff --git a/portal_detector.h b/portal_detector.h
index 8997af8..1d1a906 100644
--- a/portal_detector.h
+++ b/portal_detector.h
@@ -59,19 +59,29 @@
   };
 
   struct Result {
-    Result() : phase(kPhaseUnknown), status(kStatusFailure), final(false) {}
+    Result()
+        : phase(kPhaseUnknown), status(kStatusFailure),
+          num_attempts(0), final(false) {}
     Result(Phase phase_in, Status status_in)
-        : phase(phase_in), status(status_in), final(false) {}
-    Result(Phase phase_in, Status status_in, bool final_in)
-        : phase(phase_in), status(status_in), final(final_in) {}
+        : phase(phase_in), status(status_in),
+          num_attempts(0), final(false) {}
+    Result(Phase phase_in, Status status_in, int num_attempts_in, bool final_in)
+        : phase(phase_in), status(status_in),
+          num_attempts(num_attempts_in), final(final_in) {}
     Phase phase;
     Status status;
+    // Total number of portal detections attempted.
+    // This includes failure, timeout and successful attempts.
+    // This only valid when |final| is true.
+    int num_attempts;
     bool final;
   };
 
   static const int kDefaultCheckIntervalSeconds;
   static const char kDefaultURL[];
   static const char kResponseExpected[];
+  // Maximum number of times the PortalDetector will attempt a connection.
+  static const int kMaxRequestAttempts;
 
   PortalDetector(ConnectionRefPtr connection,
                  EventDispatcher *dispatcher,
@@ -114,8 +124,6 @@
   FRIEND_TEST(PortalDetectorTest, StartAttemptAfterDelay);
   FRIEND_TEST(PortalDetectorTest, AttemptCount);
 
-  // Number of times to attempt connection.
-  static const int kMaxRequestAttempts;
   // Minimum time between attempts to connect to server.
   static const int kMinTimeBetweenAttemptsSeconds;
   // Time to wait for request to complete.
diff --git a/portal_detector_unittest.cc b/portal_detector_unittest.cc
index ccc1175..3544c8b 100644
--- a/portal_detector_unittest.cc
+++ b/portal_detector_unittest.cc
@@ -90,6 +90,8 @@
   }
 
  protected:
+  static const int kNumAttempts;
+
   class CallbackTarget {
    public:
     CallbackTarget()
@@ -206,6 +208,9 @@
   MockHTTPRequest *http_request_;
 };
 
+// static
+const int PortalDetectorTest::kNumAttempts = 0;
+
 TEST_F(PortalDetectorTest, Constructor) {
   ExpectReset();
 }
@@ -226,6 +231,7 @@
   ExpectAttemptRetry(PortalDetector::Result(
       PortalDetector::kPhaseConnection,
       PortalDetector::kStatusFailure,
+      kNumAttempts,
       false));
   portal_detector()->StartAttemptTask();
 }
@@ -289,6 +295,7 @@
                     PortalDetector::Result(
                         PortalDetector::kPhaseDNS,
                         PortalDetector::kStatusFailure,
+                        kNumAttempts,
                         false))))
         .Times(PortalDetector::kMaxRequestAttempts - 1);
 
@@ -298,6 +305,7 @@
                     PortalDetector::Result(
                         PortalDetector::kPhaseDNS,
                         PortalDetector::kStatusFailure,
+                        kNumAttempts,
                         true))))
         .Times(1);
   }
@@ -326,6 +334,7 @@
   ExpectAttemptRetry(PortalDetector::Result(
       PortalDetector::kPhaseContent,
       PortalDetector::kStatusFailure,
+      kNumAttempts,
       false));
   AppendReadData("X");
 }
@@ -335,6 +344,7 @@
   ExpectAttemptRetry(PortalDetector::Result(
       PortalDetector::kPhaseUnknown,
       PortalDetector::kStatusTimeout,
+      kNumAttempts,
       false));
 
   EXPECT_CALL(*http_request(), response_data())
@@ -353,6 +363,7 @@
   ExpectAttemptRetry(PortalDetector::Result(
       PortalDetector::kPhaseContent,
       PortalDetector::kStatusTimeout,
+      kNumAttempts,
       false));
 
   EXPECT_CALL(*http_request(), response_data())
@@ -373,6 +384,7 @@
                   PortalDetector::Result(
                       PortalDetector::kPhaseContent,
                       PortalDetector::kStatusSuccess,
+                      kNumAttempts,
                       true))));
   EXPECT_CALL(*http_request(), Stop())
       .Times(2);