Better support for Cellular roaming state in ONC

chromeos/dbus/fake_shill_manager_client.cc

 // Copyright 2013 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/dbus/fake_shill_manager_client.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 91 matching lines @@
   DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface()
       ->SetServiceProperty(service_path,
                            shill::kStateProperty,
                            base::StringValue(shill::kStatePortal));
 }
 
 const char* kTechnologyUnavailable = "unavailable";
 const char* kNetworkActivated = "activated";
 const char* kNetworkDisabled = "disabled";
 const char* kCellularServicePath = "/service/cellular1";
+const char* kRoamingRequired = "required";

[armansito, 2015/02/24 03:05:03]

nit: The commit message calls this state "always", so update the commit message
to say "required".

P.S.: Now that I've looked at the rest of the CL, is this a fake-only parameter?

[stevenjb, 2015/02/24 16:31:17]

Yes, this is only for the fake implementation. Changed the ONC state to better
match the meaning.

 
 }  // namespace
 
 // static
 const char FakeShillManagerClient::kFakeEthernetNetworkGuid[] = "eth1_guid";
 
 FakeShillManagerClient::FakeShillManagerClient()
     : interactive_delay_(0),
       weak_ptr_factory_(this) {
   ParseCommandLineSwitch();
@@ skipping 640 matching lines @@
         "/device/cellular1", shill::kTypeCellular, "stub_cellular_device1");
     devices->SetDeviceProperty("/device/cellular1",
                                shill::kCarrierProperty,
                                base::StringValue(shill::kCarrierSprint));
     base::ListValue carrier_list;
     carrier_list.AppendString(shill::kCarrierSprint);
     carrier_list.AppendString(shill::kCarrierGenericUMTS);
     devices->SetDeviceProperty("/device/cellular1",
                                shill::kSupportedCarriersProperty,
                                carrier_list);
+    if (roaming_state_ == kRoamingRequired) {
+      devices->SetDeviceProperty("/device/cellular1",
+                                 shill::kProviderRequiresRoamingProperty,
+                                 base::FundamentalValue(true));
+    }
 
     services->AddService(kCellularServicePath,
                          "cellular1_guid",
                          "cellular1" /* name */,
                          shill::kTypeCellular,
                          state,
                          add_to_visible);
     base::StringValue technology_value(shill::kNetworkTechnologyGsm);
     devices->SetDeviceProperty("/device/cellular1",
                                shill::kTechnologyFamilyProperty,
@@ skipping 10 matching lines @@
       services->SetServiceProperty(kCellularServicePath,
                                    shill::kConnectableProperty,
                                    base::FundamentalValue(true));
     } else {
       services->SetServiceProperty(
           kCellularServicePath,
           shill::kActivationStateProperty,
           base::StringValue(shill::kActivationStateNotActivated));
     }
 
+    std::string roaming_state;
+    if (roaming_state_ == kRoamingRequired)
+      roaming_state = shill::kRoamingStateRoaming;
+    else if (roaming_state_.empty())
+      roaming_state = shill::kRoamingStateHome;
+    else
+      roaming_state = roaming_state_;

[armansito, 2015/02/24 03:05:03]

I would add a comment above saying that values of |roaming_state_| other than
kRoamingRequired are identical to the corresponding shill properties so it's OK
to directly assign. 

[stevenjb, 2015/02/24 16:31:17]

Done.

     services->SetServiceProperty(kCellularServicePath,
                                  shill::kRoamingStateProperty,
-                                 base::StringValue(shill::kRoamingStateHome));
+                                 base::StringValue(roaming_state));
     profiles->AddService(shared_profile, kCellularServicePath);
   }
 
   // VPN
   state = GetInitialStateForType(shill::kTypeVPN, &enabled);
   if (state != kTechnologyUnavailable) {
     // Set the "Provider" dictionary properties. Note: when setting these in
     // Shill, "Provider.Type", etc keys are used, but when reading the values
     // "Provider" . "Type", etc keys are used. Here we are setting the values
     // that will be read (by the UI, tests, etc).
@@ skipping 226 matching lines @@
     shill_device_property_map_
         [shill::kTypeCellular][shill::kTechnologyFamilyProperty] =
             new base::StringValue(shill::kNetworkTechnologyGsm);
     return true;
   } else if (arg0 == "tdls_busy") {
     if (!arg1.empty())
       base::StringToInt(arg1, &s_tdls_busy_count);
     else
       s_tdls_busy_count = 1;
     return true;
+  } else if (arg0 == "roaming") {
+    // "home", "roaming", or "required"
+    roaming_state_ = arg1;
+    return true;
   }
   return SetInitialNetworkState(arg0, arg1);
 }
 
 bool FakeShillManagerClient::SetInitialNetworkState(std::string type_arg,
                                                     std::string state_arg) {
   std::string state;
   state_arg = base::StringToLowerASCII(state_arg);
   if (state_arg.empty() || state_arg == "1" || state_arg == "on" ||
       state_arg == "enabled" || state_arg == "connected" ||
@@ skipping 68 matching lines @@
   *enabled = true;
   if ((state == shill::kStatePortal && type != shill::kTypeWifi) ||
       (state == kNetworkActivated && type != shill::kTypeCellular)) {
     LOG(WARNING) << "Invalid state: " << state << " for " << type;
     return shill::kStateIdle;
   }
   return state;
 }
 
 }  // namespace chromeos