Call Device.SetCarrier when the ONC Carrier property is specified.

chromeos/network/managed_network_configuration_handler_impl.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/network/managed_network_configuration_handler_impl.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/guid.h"
@@ skipping 208 matching lines @@
           &onc::kNetworkWithStateSignature));
   callback.Run(service_path, *onc_network);
 }
 
 // SetProperties
 
 void ManagedNetworkConfigurationHandlerImpl::SetProperties(
     const std::string& service_path,
     const base::DictionaryValue& user_settings,
     const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) {
   const NetworkState* state =
       network_state_handler_->GetNetworkStateFromServicePath(
           service_path, true /* configured_only */);
   if (!state) {
     InvokeErrorCallback(service_path, error_callback, kUnknownNetwork);
     return;
   }
 
   std::string guid = state->guid();
   DCHECK(!guid.empty());
@@ skipping 58 matching lines @@
   VLOG(2) << "This configuration is " << (network_policy ? "" : "not ")
           << "managed.";
 
   scoped_ptr<base::DictionaryValue> shill_dictionary(
       policy_util::CreateShillConfiguration(*profile,
                                             guid,
                                             &policies->global_network_config,
                                             network_policy,
                                             validated_user_settings.get()));
 
+  // 'Carrier' needs to be handled specially if set.

[pneubeck (no reviews), 2015/03/27 18:26:46]

BIG NOTE:
you do this now only for SetProperties but not for the initial CreateNetwork

btw. in the current form, you can also considering moving it to NCH, which might
catch more uses and might be cleaner.

[stevenjb, 2015/03/27 23:18:04]

I prefer keeping this as an ONC special case instead of a Shill special case.

Yeah, I'm not sure what to do about createConfiguration. It doesn't make sense
to have CreateConfiguration fail if SetCarrier fails since the configuration
will be created, but I don't like not providing feedback either. For now I added
documentation to the property in networking_private.idl specifying that it can
be specified in setProperties but not createConfiguration.

+  base::DictionaryValue* cellular;

[pneubeck (no reviews), 2015/03/27 18:26:46]

pls initialize to nullptr

[stevenjb, 2015/03/27 23:18:04]

We always disagree on this. It is really not necessary checking the result of
Value::Get*. I'll set it as always because it's not worthy of discussion.

+  if (validated_user_settings->GetDictionaryWithoutPathExpansion(
+          ::onc::network_config::kCellular, &cellular)) {
+    std::string carrier;
+    if (cellular->GetStringWithoutPathExpansion(::onc::cellular::kCarrier,
+                                                &carrier)) {
+      network_device_handler_->SetCarrier(
+          state->device_path(), carrier,

[pneubeck (no reviews), 2015/03/27 18:26:46]

do you have to check whether device_path is valid?

[stevenjb, 2015/03/27 23:18:04]

device_path must always be valid for NetworkState.

+          base::Bind(
+              &ManagedNetworkConfigurationHandlerImpl::SetShillProperties,

[pneubeck (no reviews), 2015/03/27 18:26:46]

We really need a helper to collect callbacks of several >>parallel<< callbacks.
These callback sequences are a pain ...

[stevenjb, 2015/03/27 23:18:04]

Agreed.

+              weak_ptr_factory_.GetWeakPtr(), service_path,
+              base::Passed(&shill_dictionary), callback, error_callback),
+          error_callback);
+      return;
+    }
+  }
+
+  SetShillProperties(service_path, shill_dictionary.Pass(), callback,
+                     error_callback);
+}
+
+void ManagedNetworkConfigurationHandlerImpl::SetShillProperties(
+    const std::string& service_path,
+    scoped_ptr<base::DictionaryValue> shill_dictionary,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
   network_configuration_handler_->SetShillProperties(
       service_path, *shill_dictionary,
       NetworkConfigurationObserver::SOURCE_USER_ACTION, callback,
       error_callback);
 }
 
 void ManagedNetworkConfigurationHandlerImpl::CreateConfiguration(
     const std::string& userhash,
     const base::DictionaryValue& properties,
     const network_handler::StringResultCallback& callback,
@@ skipping 483 matching lines @@
     scoped_ptr<base::DictionaryValue> network_properties,
     GetDevicePropertiesCallback send_callback,
     const std::string& error_name,
     scoped_ptr<base::DictionaryValue> error_data) {
   NET_LOG_ERROR("Error getting device properties", service_path);
   send_callback.Run(service_path, network_properties.Pass());
 }
 
 
 }  // namespace chromeos