This moves the ONC parsing code into chromeos/network/onc

chromeos/network/onc/onc_certificate_importer.h

 // 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.
 
-#ifndef CHROME_BROWSER_CHROMEOS_NETWORK_SETTINGS_ONC_CERTIFICATE_IMPORTER_H_
-#define CHROME_BROWSER_CHROMEOS_NETWORK_SETTINGS_ONC_CERTIFICATE_IMPORTER_H_
+#ifndef CHROMEOS_NETWORK_ONC_ONC_CERTIFICATE_IMPORTER_H_
+#define CHROMEOS_NETWORK_ONC_ONC_CERTIFICATE_IMPORTER_H_
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/chromeos/cros/network_ui_data.h"
+#include "chromeos/chromeos_export.h"
+#include "chromeos/network/onc/onc_constants.h"
 
 namespace base {
 class DictionaryValue;
 class ListValue;
 }
 
 namespace net {
 class X509Certificate;
 typedef std::vector<scoped_refptr<X509Certificate> > CertificateList;
 }
 
 namespace chromeos {
 namespace onc {
 
+CHROMEOS_EXPORT extern const char kErrorCertDataMalformed[];

[pneubeck (no reviews), 2012/12/04 10:43:56]

If we want to use the strings as replacements for error codes, then it should be
clear which strings can be returned. If we share the strings between different
classes/functions, then it's impossible to figure that out except by looking at
the implementation.

Therefore, I would consider putting the strings (except standard codes like
kSuccess) into the class definition and in case of the Decrypt function, into a
namespace.

[Greg Spencer (Chromium), 2012/12/07 18:12:27]

We're not returning these anymore.

+CHROMEOS_EXPORT extern const char kErrorCertDataMissing[];
+CHROMEOS_EXPORT extern const char kErrorCertDelete[];
+CHROMEOS_EXPORT extern const char kErrorCertGuidCollision[];
+CHROMEOS_EXPORT extern const char kErrorCertGuidMissing[];
+CHROMEOS_EXPORT extern const char kErrorCertImport[];
+CHROMEOS_EXPORT extern const char kErrorCertTrustInvalid[];
+CHROMEOS_EXPORT extern const char kErrorCertTrustUnknown[];
+CHROMEOS_EXPORT extern const char kErrorCertTypeMissing[];
+CHROMEOS_EXPORT extern const char kErrorUnknown[];
+
 // This class handles certificate imports from ONC (both policy and user
 // imports) into the certificate store. In particular, the GUID of certificates
 // is stored together with the certificate as Nickname.
-class CertificateImporter {
+class CHROMEOS_EXPORT CertificateImporter {
  public:
+
   // Certificates pushed from a policy source with Web trust are only imported
   // with ParseCertificate() if the |allow_web_trust_from_policy| permission is
   // granted.
-  CertificateImporter(NetworkUIData::ONCSource onc_source,
+  CertificateImporter(ONCSource onc_source,
                       bool allow_web_trust_from_policy);
 
   // Parses and stores the certificates in |onc_certificates| into the
   // certificate store. If the "Remove" field of a certificate is enabled, then
-  // removes the certificate from the store instead of importing. Returns false
-  // and sets |error| to a user readable message if an error occured. In that
-  // case, some of the certificates may already be stored/removed. Otherwise, if
-  // no error occured, returns true and doesn't modify |error|.
-  bool ParseAndStoreCertificates(const base::ListValue& onc_certificates,
-                                 std::string* error);
+  // removes the certificate from the store instead of importing. Returns the
+  // result of the parse operation. In case of an error, some of the
+  // certificates may already be stored/removed. Otherwise, if no error
+  // occurred, returns PARSE_OK.
+  bool ParseAndStoreCertificates(

[pneubeck (no reviews), 2012/12/04 10:43:56]

You don't document the return value anymore.

I would write "...sets |result|..." instead of "Returns the result" to
distinguish |result| and return value.

Please, also refer to the error constants above.

PARSE_OK? 

[Greg Spencer (Chromium), 2012/12/07 18:12:27]

Fixed this: it now returns an error code, and documents the error code.

+      const base::ListValue& onc_certificates,
+      std::string* result);
 
   // Parses and stores/removes |certificate| in/from the certificate
-  // store. Returns false if an error occured. Returns true otherwise.
-  bool ParseAndStoreCertificate(const base::DictionaryValue& certificate);
+  // store. Returns false if an error occurred. Returns true otherwise.
+  bool ParseAndStoreCertificate(
+      const base::DictionaryValue& certificate,
+      std::string* result);
 
   // Lists the certificates that have the string |label| as their certificate
   // nickname (exact match).
   static void ListCertsWithNickname(const std::string& label,
                                     net::CertificateList* result);
 
  protected:
   // Deletes any certificate that has the string |label| as its nickname (exact
   // match).
   static bool DeleteCertAndKeyByNickname(const std::string& label);
 
  private:
   bool ParseServerOrCaCertificate(
     const std::string& cert_type,
     const std::string& guid,
-    const base::DictionaryValue& certificate);
+    const base::DictionaryValue& certificate,
+    std::string* result);
   bool ParseClientCertificate(

[pneubeck (no reviews), 2012/12/04 10:43:56]

nit: add an empty line

[Greg Spencer (Chromium), 2012/12/07 18:12:27]

Done.

     const std::string& guid,
-    const base::DictionaryValue& certificate);
+    const base::DictionaryValue& certificate,
+    std::string* result);
 
   // Where the ONC blob comes from.
-  NetworkUIData::ONCSource onc_source_;
+  ONCSource onc_source_;
 
   // Whether certificates with Web trust should be stored when pushed from a
   // policy source.
   bool allow_web_trust_from_policy_;
 
-  std::string error_;
-
   DISALLOW_COPY_AND_ASSIGN(CertificateImporter);
 };
 
 }  // chromeos
 }  // onc
 
-#endif  // CHROME_BROWSER_CHROMEOS_NETWORK_SETTINGS_ONC_CERTIFICATE_IMPORTER_H_
+#endif  // CHROMEOS_NETWORK_ONC_ONC_CERTIFICATE_IMPORTER_H_