Generate the Chrome policy schema at compile time.

components/policy/core/common/schema.h

Index: components/policy/core/common/schema.h
diff --git a/components/policy/core/common/schema.h b/components/policy/core/common/schema.h
index 7a734bf02110cda5ad1c967adbb4d7f9606b6c6d..47ed5daf4605e6165144437b1494efdd4be85a21 100644
--- a/components/policy/core/common/schema.h
+++ b/components/policy/core/common/schema.h
@@ -6,17 +6,16 @@
 #define COMPONENTS_POLICY_CORE_COMMON_SCHEMA_H_
 
 #include <string>
-#include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/scoped_vector.h"
 #include "base/values.h"
 #include "components/policy/policy_export.h"
 
 namespace policy {
 namespace internal {
 
+struct POLICY_EXPORT SchemaData;
 struct POLICY_EXPORT SchemaNode;
 struct POLICY_EXPORT PropertyNode;
 struct POLICY_EXPORT PropertiesNode;
@@ -33,10 +32,9 @@ class POLICY_EXPORT Schema {
   // Builds an empty, invalid schema.
   Schema();
 
-  // Builds a schema pointing to the inner structure of |schema|. If |schema|
-  // is NULL then this Schema instance will be invalid.
-  // Does not take ownership of |schema|.
-  explicit Schema(const internal::SchemaNode* schema);
+  // Builds a schema pointing to the inner structure of |data|,

[pastarmovj, 2013/10/22 09:30:46]

nitty nit: It seems like you can fit more of this comment on the previous line.

[Joao da Silva, 2013/10/22 12:53:27]

That's right, but the whole comment doesn't file in a single line. I find it
more readable to break on the comma than in the middle of a part of the
sentence. WDYT?

[pastarmovj, 2013/10/22 13:42:53]

sgtm

+  // rooted at |node|.
+  Schema(const internal::SchemaData* data, const internal::SchemaNode* node);
 
   Schema(const Schema& schema);
 
@@ -44,14 +42,15 @@ class POLICY_EXPORT Schema {
 
   // Returns true if this Schema is valid. Schemas returned by the methods below
   // may be invalid, and in those cases the other methods must not be used.
-  bool valid() const { return schema_ != NULL; }
+  bool valid() const { return data_ != NULL; }
 
   base::Value::Type type() const;
 
   // Used to iterate over the known properties of TYPE_DICTIONARY schemas.
   class POLICY_EXPORT Iterator {
    public:
-    explicit Iterator(const internal::PropertiesNode* properties);
+    Iterator(const internal::SchemaData* data,
+             const internal::PropertiesNode* node);
     Iterator(const Iterator& iterator);
     ~Iterator();
 
@@ -70,6 +69,7 @@ class POLICY_EXPORT Schema {
     Schema schema() const;
 
    private:
+    const internal::SchemaData* data_;
     const internal::PropertyNode* it_;
     const internal::PropertyNode* end_;
   };
@@ -99,7 +99,8 @@ class POLICY_EXPORT Schema {
   Schema GetItems() const;
 
  private:
-  const internal::SchemaNode* schema_;
+  const internal::SchemaData* data_;
+  const internal::SchemaNode* node_;
 };
 
 // Owns schemas for policies of a given component.
@@ -109,13 +110,13 @@ class POLICY_EXPORT SchemaOwner {
 
   // The returned Schema is valid only during the lifetime of the SchemaOwner
   // that created it. It may be obtained multiple times.
-  Schema schema() const { return Schema(root_); }
+  Schema schema() const;
 
   // Returns a SchemaOwner that references static data. This can be used by
   // the embedder to pass structures generated at compile time, which can then
   // be quickly loaded at runtime.
   // Note: PropertiesNodes must have their PropertyNodes sorted by key.

[pastarmovj, 2013/10/22 09:30:46]

This note is a bit of a tricky thing. There is no direct refernce to
PropertiesNodes type in this function and therefore it is hard to guess what is
meant here. Either remove the comment, extend it or move it to where it actually
is important that this property is true.

[Joao da Silva, 2013/10/22 12:53:27]

Right, removed the comment. This is already detailed in schema_internal.h and in
the generator script.

-  static scoped_ptr<SchemaOwner> Wrap(const internal::SchemaNode* schema);
+  static scoped_ptr<SchemaOwner> Wrap(const internal::SchemaData* data);
 
   // Parses the JSON schema in |schema| and returns a SchemaOwner that owns
   // the internal representation. If |schema| is invalid then NULL is returned
@@ -124,30 +125,30 @@ class POLICY_EXPORT SchemaOwner {
                                        std::string* error);
 
  private:
-  explicit SchemaOwner(const internal::SchemaNode* root);
+  class Internal;

[pastarmovj, 2013/10/22 09:30:46]

I don't like this name very much. It doesn't tell what it really is - a data
container. I think InternalSchemaStorage or something like this might be more
useful name.

[Joao da Silva, 2013/10/22 12:53:27]

Renamed to InternalStorage.

 
-  // Parses the JSON schema in |schema| and returns the root SchemaNode if
-  // successful, otherwise returns NULL. Any intermediate objects built by
-  // this method are appended to the ScopedVectors.
-  const internal::SchemaNode* Parse(const base::DictionaryValue& schema,
-                                    std::string* error);
+  SchemaOwner(const internal::SchemaData* data, scoped_ptr<Internal> internal);
+
+  // Parses the JSON schema in |schema| and returns the index of the
+  // corresponding SchemaNode in |internal->schema_nodes|, which gets populated
+  // with any necessary intermediate nodes. If |schema| is invalid then -1 is
+  // returned and |error| is set to the error cause.
+  static int Parse(const base::DictionaryValue& schema,

[pastarmovj, 2013/10/22 09:30:46]

I think this function as well as the two helpers below belong to the Internal
class as actual member functions since they require the instance to work.

[Joao da Silva, 2013/10/22 12:53:27]

Good observation, done. This header is much cleaner now :-)

+                   std::string* error,
+                   Internal* internal);
 
   // Helper for Parse().
-  const internal::SchemaNode* ParseDictionary(
-      const base::DictionaryValue& schema,
-      std::string* error);
+  static int ParseDictionary(const base::DictionaryValue& schema,
+                             std::string* error,
+                             Internal* internal);
 
   // Helper for Parse().
-  const internal::SchemaNode* ParseList(const base::DictionaryValue& schema,
-                                        std::string* error);
-
-  const internal::SchemaNode* root_;
-  ScopedVector<internal::SchemaNode> schema_nodes_;
-  // Note: |property_nodes_| contains PropertyNode[] elements and must be
-  // cleared manually to properly use delete[].
-  std::vector<internal::PropertyNode*> property_nodes_;
-  ScopedVector<internal::PropertiesNode> properties_nodes_;
-  ScopedVector<std::string> keys_;
+  static int ParseList(const base::DictionaryValue& schema,
+                       std::string* error,
+                       Internal* internal);
+
+  scoped_ptr<Internal> internal_;

[dconnelly, 2013/10/22 11:55:52]

Please add a comment explaining that internal_ will hold NULL when the instance
was obtained using Wrap() instead of Parse().

[Joao da Silva, 2013/10/22 12:53:27]

Done.

+  const internal::SchemaData* data_;
 
   DISALLOW_COPY_AND_ASSIGN(SchemaOwner);
 };