Refactor NetworkLibraryImplBase::LoadOncNetworks.

chrome/browser/chromeos/cros/network_library_impl_base.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 "chrome/browser/chromeos/cros/network_library_impl_base.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/memory/scoped_vector.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
+#include "chrome/browser/chromeos/cros/network_constants.h"
 #include "chrome/browser/chromeos/cros/onc_network_parser.h"
 #include "chrome/browser/chromeos/network_login_observer.h"
 #include "chromeos/network/onc/onc_certificate_importer.h"
 #include "chromeos/network/onc/onc_constants.h"
 #include "chromeos/network/onc/onc_signature.h"
 #include "chromeos/network/onc/onc_utils.h"
 #include "chromeos/network/onc/onc_validator.h"
 #include "content/public/browser/browser_thread.h"
 #include "crypto/nss_util.h"  // crypto::GetTPMTokenInfo() for 802.1X and VPN.
 #include "grit/generated_resources.h"
@@ skipping 1096 matching lines @@
     }
   }
 
   std::set<std::string> removal_ids;
   std::set<std::string>& network_ids(network_source_map_[source]);
   network_ids.clear();
   if (has_network_configurations) {
     VLOG(2) << "ONC file has " << network_configs->GetSize() << " networks";
     OncNetworkParser parser(*network_configs, source);
 
-    // Parse all networks. Bail out if that fails.
-    NetworkOncMap added_onc_map;
-    ScopedVector<Network> networks;
     for (int i = 0; i < parser.GetNetworkConfigsSize(); i++) {
       // Parse Open Network Configuration blob into a temporary Network object.
       bool marked_for_removal = false;
-      Network* network = parser.ParseNetwork(i, &marked_for_removal);
+      scoped_ptr<Network> network(parser.ParseNetwork(i, &marked_for_removal));
       if (!network) {
         LOG(ERROR) << "Error during ONC parsing network at index " << i
                    << " from " << onc::GetSourceAsString(source);
         success = false;
         continue;
       }
 
       // Disallow anything but WiFi and Ethernet for device-level policy (which
       // corresponds to shared networks). See also http://crosbug.com/28741.
       if (source == onc::ONC_SOURCE_DEVICE_POLICY &&
           network->type() != TYPE_WIFI &&
           network->type() != TYPE_ETHERNET) {
         LOG(WARNING) << "Ignoring device-level policy-pushed network of type "
                      << network->type();
-        delete network;
         continue;
       }
 
-      networks.push_back(network);
-      if (!(source == onc::ONC_SOURCE_USER_IMPORT &&
-            marked_for_removal)) {
-        added_onc_map[network->unique_id()] = parser.GetNetworkConfig(i);
-      }
-
-      if (marked_for_removal)
+      if (source == onc::ONC_SOURCE_USER_IMPORT && marked_for_removal) {
+        // User import supports the removal of networks by ID.
         removal_ids.insert(network->unique_id());
-    }
-
-    // Update the ONC map.
-    for (NetworkOncMap::iterator iter(added_onc_map.begin());
-         iter != added_onc_map.end(); ++iter) {
-      const base::DictionaryValue*& entry = network_onc_map_[iter->first];
-      delete entry;
-      entry = iter->second->DeepCopy();
-    }
-
-    // Configure the networks. While doing so, collect unique identifiers of the
-    // networks that are defined in the ONC blob in |network_ids|. They're later
-    // used to clean out any previously-existing networks that had been
-    // configured through policy but are no longer specified in the updated ONC
-    // blob.
-    for (std::vector<Network*>::iterator iter(networks.begin());
-         iter != networks.end(); ++iter) {
-      Network* network = *iter;
-
-      // Don't configure a network that is supposed to be removed. For
-      // policy-managed networks, the "remove" functionality of ONC is ignored.
-      if (source == onc::ONC_SOURCE_USER_IMPORT &&
-          removal_ids.find(network->unique_id()) != removal_ids.end()) {
         continue;
       }
 
-      DictionaryValue dict;
+      if (marked_for_removal) {
+        // Don't configure a network that is supposed to be removed. For

[Greg Spencer (Chromium), 2012/12/21 17:52:45]

nit: You could put the comment above the "if" and remove the braces.

[pneubeck (no reviews), 2013/01/08 13:40:23]

Done.

+        // policy-managed networks, the "remove" functionality of ONC is
+        // irrelevant. Instead in general, all previously configured networks

[Greg Spencer (Chromium), 2012/12/21 17:52:45]

nit: add a comma: "Instead, in general,"

[pneubeck (no reviews), 2013/01/08 13:40:23]

Done.

+        // that are no longer configured are removed.
+        continue;
+      }
+
+      // Update the ONC map.
+      const base::DictionaryValue*& entry =
+          network_onc_map_[network->unique_id()];
+      delete entry;
+      entry = parser.GetNetworkConfig(i)->DeepCopy();
+
+      // Configure the network.
+      base::DictionaryValue dict;
       for (Network::PropertyMap::const_iterator props =
                network->property_map_.begin();
            props != network->property_map_.end(); ++props) {
         std::string key =
             NativeNetworkParser::property_mapper()->GetKey(props->first);
         if (!key.empty())
           dict.SetWithoutPathExpansion(key, props->second->DeepCopy());
         else
           VLOG(2) << "Property " << props->first << " will not be sent";
       }
 
       // Set the appropriate profile for |source|.
       if (profile != NULL)
         dict.SetString(flimflam::kProfileProperty, profile->path);
 
       // For Ethernet networks, apply them to the current Ethernet service.
       if (network->type() == TYPE_ETHERNET) {
         const EthernetNetwork* ethernet = ethernet_network();
         if (ethernet) {
           CallConfigureService(ethernet->unique_id(), &dict);
         } else {
           LOG(WARNING) << "Tried to import ONC with an Ethernet network when "
                        << "there is no active Ethernet connection.";
         }
       } else {
         CallConfigureService(network->unique_id(), &dict);
       }
 
+      // Store the unique identifier of the network that is defined in the ONC
+      // blob in |network_ids|. The identifiers are later used to clean out any
+      // previously-existing networks that had been configured through policy
+      // but are no longer specified in the updated ONC blob.
       network_ids.insert(network->unique_id());
     }
   }
 
   if (from_policy) {
     // For policy-managed networks, go through the list of existing remembered
     // networks and clean out the ones that no longer have a definition in the
     // ONC blob. We first collect the networks and do the actual deletion later
     // because ForgetNetwork() changes the remembered network vectors.
     ForgetNetworksById(source, network_ids, false);
@@ skipping 518 matching lines @@
   GetTpmInfo();
   return tpm_slot_;
 }
 
 const std::string& NetworkLibraryImplBase::GetTpmPin() {
   GetTpmInfo();
   return tpm_pin_;
 }
 
 }  // namespace chromeos