Better support for Cellular roaming state in ONC

chromeos/dbus/fake_shill_manager_client.cc

Index: chromeos/dbus/fake_shill_manager_client.cc
diff --git a/chromeos/dbus/fake_shill_manager_client.cc b/chromeos/dbus/fake_shill_manager_client.cc
index d0e392fd66a6a852d542d9d20b2704a11084bf57..deba233bac4ff51f1530a7d2e7c10a87bf7679b4 100644
--- a/chromeos/dbus/fake_shill_manager_client.cc
+++ b/chromeos/dbus/fake_shill_manager_client.cc
@@ -109,6 +109,7 @@ 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
 
@@ -769,6 +770,11 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     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",
@@ -799,9 +805,16 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
           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);
   }
 
@@ -1048,6 +1061,10 @@ bool FakeShillManagerClient::ParseOption(const std::string& arg0,
     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);
 }