Show parse errors in the UI when loading ONC files.

chrome/browser/chromeos/cros/network_library.cc

 // Copyright (c) 2011 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.h"
 
 #include <dbus/dbus-glib.h>
 #include <dbus/dbus-gtype-specialized.h>
 #include <glib-object.h>
 
@@ skipping 1727 matching lines @@
   // virtual DisconnectFromNetwork implemented in derived classes.
   virtual void ForgetNetwork(const std::string& service_path) OVERRIDE;
   virtual void EnableEthernetNetworkDevice(bool enable) OVERRIDE;
   virtual void EnableWifiNetworkDevice(bool enable) OVERRIDE;
   virtual void EnableCellularNetworkDevice(bool enable) OVERRIDE;
   // virtual EnableOfflineMode implemented in derived classes.
   // virtual GetIPConfigs implemented in derived classes.
   // virtual SetIPConfig implemented in derived classes.
   virtual void SwitchToPreferredNetwork() OVERRIDE;
   virtual bool LoadOncNetworks(const std::string& onc_blob,
-                               const std::string& passcode) OVERRIDE;
+                               const std::string& passcode,
+                               std::string* error) OVERRIDE;
   virtual bool SetActiveNetwork(ConnectionType type,
                                 const std::string& service_path) OVERRIDE;
 
  protected:
   typedef ObserverList<NetworkObserver> NetworkObserverList;
   typedef std::map<std::string, NetworkObserverList*> NetworkObserverMap;
 
   typedef ObserverList<NetworkDeviceObserver> NetworkDeviceObserverList;
   typedef std::map<std::string, NetworkDeviceObserverList*>
       NetworkDeviceObserverMap;
@@ skipping 1068 matching lines @@
     if (!wifi->preferred())  // All preferred networks are sorted in front.
       break;
     if (wifi->auto_connect()) {
       ConnectToWifiNetwork(wifi);
       break;
     }
   }
 }
 
 bool NetworkLibraryImplBase::LoadOncNetworks(const std::string& onc_blob,
-                                             const std::string& passcode) {
+                                             const std::string& passcode,
+                                             std::string* error) {
   // TODO(gspencer): Add support for decrypting onc files. crbug.com/19397
   OncNetworkParser parser(onc_blob);
 
+  if (!parser.parse_error().empty()) {
+    if (error)
+      *error = parser.parse_error();
+    return false;
+  }
+
   for (int i = 0; i < parser.GetCertificatesSize(); i++) {
     // Insert each of the available certs into the certificate DB.
     if (parser.ParseCertificate(i).get() == NULL) {
       DLOG(WARNING) << "Cannot parse certificate in ONC file";
+      if (error)

[kmixter1, 2011/12/14 19:46:38]

There are cases where false is returned but error is unset which is inconsistent
with the function comment.  Should document when the function sets error (or
always set it at the top).

[kmixter1, 2011/12/14 19:48:36]

Sorry, misread the code.  I think error is always set.

+        *error = parser.parse_error();
       return false;
     }
   }
 
   for (int i = 0; i < parser.GetNetworkConfigsSize(); i++) {
     // Parse Open Network Configuration blob into a temporary Network object.
     scoped_ptr<Network> network(parser.ParseNetwork(i));
     if (!network.get()) {
       DLOG(WARNING) << "Cannot parse network in ONC file";
+      if (error)
+        *error = parser.parse_error();
       return false;
     }
 
     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";
     }
 
     CallConfigureService(network->unique_id(), &dict);
   }
-  return (parser.GetNetworkConfigsSize() != 0 ||
-          parser.GetCertificatesSize() != 0);
+
+  if (parser.GetNetworkConfigsSize() != 0 ||
+      parser.GetCertificatesSize() != 0) {
+      if (error)
+        *error = l10n_util::GetStringUTF8(
+            IDS_NETWORK_CONFIG_ERROR_NETWORK_IMPORT);
+      return false;
+  }
+  return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////
 // Testing functions.
 
 bool NetworkLibraryImplBase::SetActiveNetwork(
     ConnectionType type, const std::string& service_path) {
   Network* network = NULL;
   if (!service_path.empty())
     network = FindNetworkByPath(service_path);
@@ skipping 2212 matching lines @@
         "      \"GUID\": \"guid\","
         "      \"Type\": \"WiFi\","
         "      \"WiFi\": {"
         "        \"Security\": \"WEP\","
         "        \"SSID\": \"MySSID\","
         "      }"
         "    }"
         "  ],"
         "  \"Certificates\": []"
         "}");
-  LoadOncNetworks(test_blob, "");
+  LoadOncNetworks(test_blob, "", NULL);
 }
 
 ////////////////////////////////////////////////////////////////////////////
 // NetworkLibraryImplStub private methods.
 
 void NetworkLibraryImplStub::AddStubNetwork(
     Network* network, NetworkProfileType profile_type) {
   network->priority_order_ = network_priority_order_++;
   network->CalculateUniqueId();
   if (!network->unique_id().empty())
@@ skipping 227 matching lines @@
   return impl;
 }
 
 /////////////////////////////////////////////////////////////////////////////
 
 }  // namespace chromeos
 
 // Allows InvokeLater without adding refcounting. This class is a Singleton and
 // won't be deleted until its last InvokeLater is run.
 DISABLE_RUNNABLE_METHOD_REFCOUNT(chromeos::NetworkLibraryImplBase);