Introduce NetworkState::is_captive_portal()

chromeos/network/network_state.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/network_state.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "chromeos/device_event_log.h"
 #include "chromeos/network/network_profile_handler.h"
@@ skipping 11 matching lines @@
                                     std::vector<std::string>* result) {
   for (size_t i = 0; i < string_list.GetSize(); ++i) {
     std::string str;
     if (!string_list.GetString(i, &str))
       return false;
     result->push_back(str);
   }
   return true;
 }
 
+bool IsCaptivePortalState(const base::DictionaryValue& properties, bool log) {
+  std::string state;
+  properties.GetStringWithoutPathExpansion(shill::kStateProperty, &state);
+  if (state == shill::kStatePortal)

[pneubeck (no reviews), 2015/01/24 10:06:56]

shouldn't this be negated?

if (state != kStatePortal)
  return false;

[stevenjb, 2015/01/26 20:16:30]

Yes, thanks. Too many balls in the air.

+    return false;
+  std::string portal_detection_phase, portal_detection_status;
+  if (!properties.GetStringWithoutPathExpansion(
+          shill::kPortalDetectionFailedPhaseProperty,
+          &portal_detection_phase) ||
+      !properties.GetStringWithoutPathExpansion(
+          shill::kPortalDetectionFailedStatusProperty,
+          &portal_detection_status)) {
+    // If Shill (or a stub) has not set PortalDetectionFailedStatus
+    // or PortalDetectionFailedPhase, assume we are in captive portal state.
+    return true;
+  }
+  bool is_captive_portal =

[pneubeck (no reviews), 2015/01/24 10:06:56]

please add a comment what the rational of this is, e.g.:

"Shill reports in which phase it reached conclusion that it believes the device
behind a captive portal. We want to rely only on incorrect content being
returned and ignore other reasons."

[stevenjb, 2015/01/26 20:16:29]

Done.

+      portal_detection_status == shill::kPortalDetectionStatusFailure &&
+      portal_detection_phase == shill::kPortalDetectionPhaseContent;
+  if (!log)
+    return is_captive_portal;
+
+  std::string name;
+  properties.GetStringWithoutPathExpansion(shill::kNameProperty, &name);
+  if (name.empty())
+    properties.GetStringWithoutPathExpansion(shill::kSSIDProperty, &name);
+  if (portal_detection_status != shill::kPortalDetectionStatusFailure ||

[pneubeck (no reviews), 2015/01/24 10:06:56]

nit: condition can be replaced by !is_captive_portal.

[stevenjb, 2015/01/26 20:16:29]

Ditto, thanks, done.

+      portal_detection_phase != shill::kPortalDetectionPhaseContent) {
+    NET_LOG(EVENT) << "State is 'portal' but not in captive portal state:"
+                   << " name=" << name << " status=" << portal_detection_status
+                   << " phase=" << portal_detection_phase;
+  } else {
+    NET_LOG(EVENT) << "Network is in captive portal state: " << name;
+  }
+  return is_captive_portal;
+}
+
 }  // namespace
 
 namespace chromeos {
 
 NetworkState::NetworkState(const std::string& path)
     : ManagedState(MANAGED_TYPE_NETWORK, path),
       visible_(false),
+      prefix_length_(0),
       connectable_(false),
-      prefix_length_(0),
+      is_captive_portal_(false),
       signal_strength_(0),
       cellular_out_of_credits_(false) {
 }
 
 NetworkState::~NetworkState() {
 }
 
 bool NetworkState::PropertyChanged(const std::string& key,
                                    const base::Value& value) {
   // Keep care that these properties are the same as in |GetProperties|.
@@ skipping 83 matching lines @@
     NET_LOG(ERROR) << "NetworkState has no type: "
                    << shill_property_util::GetNetworkIdFromProperties(
                           properties);
     return false;
   }
 
   // By convention, all visible WiFi and WiMAX networks have a
   // SignalStrength > 0.
   if ((type() == shill::kTypeWifi || type() == shill::kTypeWimax) &&
       visible() && signal_strength_ <= 0) {
-      signal_strength_ = 1;
+    signal_strength_ = 1;
   }
 
+  // Any change to connection state will trigger a complete property update,
+  // so we update is_captive_portal_ here.
+  is_captive_portal_ = IsCaptivePortalState(properties, true /* log */);
+
   // Ensure that the network has a valid name.
   return UpdateName(properties);
 }
 
 void NetworkState::GetStateProperties(base::DictionaryValue* dictionary) const {
   ManagedState::GetStateProperties(dictionary);
 
   // Properties shared by all types.
   dictionary->SetStringWithoutPathExpansion(shill::kGuidProperty, guid());
   dictionary->SetStringWithoutPathExpansion(shill::kSecurityClassProperty,
@@ skipping 18 matching lines @@
   }
 
   // Wifi properties
   if (NetworkTypePattern::WiFi().MatchesType(type())) {
     dictionary->SetStringWithoutPathExpansion(shill::kEapMethodProperty,
                                               eap_method());
   }
 
   // Mobile properties
   if (NetworkTypePattern::Mobile().MatchesType(type())) {
-    dictionary->SetStringWithoutPathExpansion(
-        shill::kNetworkTechnologyProperty,
-        network_technology());
+    dictionary->SetStringWithoutPathExpansion(shill::kNetworkTechnologyProperty,
+                                              network_technology());
     dictionary->SetStringWithoutPathExpansion(shill::kActivationStateProperty,
                                               activation_state());
     dictionary->SetStringWithoutPathExpansion(shill::kRoamingStateProperty,
                                               roaming());
     dictionary->SetBooleanWithoutPathExpansion(shill::kOutOfCreditsProperty,
                                                cellular_out_of_credits());
   }
 }
 
 void NetworkState::IPConfigPropertiesChanged(
     const base::DictionaryValue& properties) {
-  for (base::DictionaryValue::Iterator iter(properties);
-       !iter.IsAtEnd(); iter.Advance()) {
+  for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
+       iter.Advance()) {
     std::string key = iter.key();
     const base::Value& value = iter.value();
 
     if (key == shill::kAddressProperty) {
       GetStringValue(key, value, &ip_address_);
     } else if (key == shill::kGatewayProperty) {
       GetStringValue(key, value, &gateway_);
     } else if (key == shill::kNameServersProperty) {
       const base::ListValue* dns_servers;
       if (value.GetAsList(&dns_servers)) {
@@ skipping 44 matching lines @@
 
 bool NetworkState::IsInProfile() const {
   // kTypeEthernetEap is always saved. We need this check because it does
   // not show up in the visible list, but its properties may not be available
   // when it first shows up in ServiceCompleteList. See crbug.com/355117.
   return !profile_path_.empty() || type() == shill::kTypeEthernetEap;
 }
 
 bool NetworkState::IsPrivate() const {
   return !profile_path_.empty() &&
-      profile_path_ != NetworkProfileHandler::GetSharedProfilePath();
+         profile_path_ != NetworkProfileHandler::GetSharedProfilePath();
 }
 
 std::string NetworkState::GetDnsServersAsString() const {
   std::string result;
   for (size_t i = 0; i < dns_servers_.size(); ++i) {
     if (i != 0)
       result += ",";
     result += dns_servers_[i];
   }
   return result;
 }
 
 std::string NetworkState::GetNetmask() const {
   return network_util::PrefixLengthToNetmask(prefix_length_);
 }
 
 std::string NetworkState::GetSpecifier() const {
   if (!update_received()) {
-    NET_LOG(ERROR) << "GetSpecifier called before update: " <<  path();
+    NET_LOG(ERROR) << "GetSpecifier called before update: " << path();
     return std::string();
   }
   if (type() == shill::kTypeWifi)
     return name() + "_" + security_class_;
   if (!name().empty())
     return name();
   return type();  // For unnamed networks such as ethernet.
 }
 
 void NetworkState::SetGuid(const std::string& guid) {
@@ skipping 18 matching lines @@
 }
 
 // static
 bool NetworkState::StateIsConnecting(const std::string& connection_state) {
   return (connection_state == shill::kStateAssociation ||
           connection_state == shill::kStateConfiguration ||
           connection_state == shill::kStateCarrier);
 }
 
 // static
+bool NetworkState::NetworkStateIsCaptivePortal(
+    const base::DictionaryValue& shill_properties) {
+  return IsCaptivePortalState(shill_properties, false /* log */);
+}
+
+// static
 bool NetworkState::ErrorIsValid(const std::string& error) {
   // Shill uses "Unknown" to indicate an unset or cleared error state.
   return !error.empty() && error != kErrorUnknown;
 }
 
 }  // namespace chromeos