[JSON Schema Compiler] Always consider enum dependencies "hard"

The json_schema_compiler can treat dependencies as "hard" or "soft".
Hard dependencies require a #include in the API's .h file, and are
needed for e.g.  types that aren't wrapped in a pointers.  Soft
dependencies can have a forward include.

Since enums are never wrapped in pointers (they instead use a _NONE
value to signify absence), they always need to generate hard
dependencies.

Bug: 1119660
Change-Id: Id30ca82ce4a84838518129eef39fa540574b8ba0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366560Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801677}

tools/json_schema_compiler/cpp_type_generator.py

@@ -164,11 +164,17 @@ class CppTypeGenerator(object):
     c = Code()
     if self._default_namespace.manifest_keys:
       c.Append('#include "base/strings/string_piece.h"')
+
+    # Note: It's possible that there are multiple dependencies from the same
+    # API. Make sure to only include them once.
+    added_paths = set()
     for namespace, dependencies in self._NamespaceTypeDependencies().items():
       for dependency in dependencies:
         if dependency.hard or include_soft:
-          c.Append('#include "%s/%s.h"' % (namespace.source_file_dir,
-                                           namespace.unix_name))
+          path = '%s/%s.h' % (namespace.source_file_dir, namespace.unix_name)
+          if path not in added_paths:
+            added_paths.add(path)
+            c.Append('#include "%s"' % path)
     return c
 
   def _FindType(self, full_name):
@@ -232,7 +238,13 @@ class CppTypeGenerator(object):
     """
     deps = set()
     if type_.property_type == PropertyType.REF:
-      deps.add(_TypeDependency(self._FindType(type_.ref_type), hard=hard))
+      underlying_type = self._FindType(type_.ref_type)
+      # Enums from other namespaces are always hard dependencies, since
+      # optional enums are represented via the _NONE value instead of a
+      # pointer.
+      dep_is_hard = (True if underlying_type.property_type == PropertyType.ENUM
+                     else hard)
+      deps.add(_TypeDependency(underlying_type, hard=dep_is_hard))
     elif type_.property_type == PropertyType.ARRAY:
       # Types in containers are hard dependencies because they are stored
       # directly and use move semantics.

tools/json_schema_compiler/cpp_type_generator_test.py

@@ -54,6 +54,18 @@ class CppTypeGeneratorTest(unittest.TestCase):
     self.objects_movable = self.models['objects_movable'].AddNamespace(
         self.objects_movable_idl[0], 'path/to/objects_movable.idl',
         include_compiler_options=True)
+    self.simple_api_json = CachedLoad('test/simple_api.json')
+    self.simple_api = self.models['simple_api'].AddNamespace(
+        self.simple_api_json[0], 'path/to/simple_api.json')
+    self.crossref_enums_json = CachedLoad('test/crossref_enums.json')
+    self.crossref_enums = self.models['crossref_enums'].AddNamespace(
+        self.crossref_enums_json[0], 'path/to/crossref_enums.json')
+    self.crossref_enums_array_json = CachedLoad(
+        'test/crossref_enums_array.json')
+    self.crossref_enums_array = (
+        self.models['crossref_enums_array'].AddNamespace(
+            self.crossref_enums_array_json[0],
+            'path/to/crossref_enums_array.json'))
 
   def testGenerateIncludesAndForwardDeclarations(self):
     m = model.Model()
@@ -202,5 +214,41 @@ class CppTypeGeneratorTest(unittest.TestCase):
             self.permissions.functions['contains'].callback.params[0].type_,
         is_in_container=True))
 
+  def testHardIncludesForEnums(self):
+    """Tests that enums generate hard includes. Note that it's important to use
+    use a separate file (cross_enums) here to isolate the test case so that
+    other types don't cause the hard-dependency.
+    """
+    m = model.Model()
+    m.AddNamespace(self.crossref_enums_json[0],
+                   'path/to/crossref_enums.json',
+                   environment=CppNamespaceEnvironment('%(namespace)s'))
+    m.AddNamespace(self.simple_api_json[0],
+                   'path/to/simple_api.json',
+                   environment=CppNamespaceEnvironment('%(namespace)s'))
+    manager = CppTypeGenerator(self.models.get('crossref_enums'),
+                               _FakeSchemaLoader(m))
+
+    self.assertEquals('#include "path/to/simple_api.h"',
+                      manager.GenerateIncludes().Render())
+
+  def testHardIncludesForEnumArrays(self):
+    """Tests that enums in arrays generate hard includes. Note that it's
+    important to use a separate file (cross_enums_array) here to isolate the
+    test case so that other types don't cause the hard-dependency.
+    """
+    m = model.Model()
+    m.AddNamespace(self.crossref_enums_json[0],
+                   'path/to/crossref_enums_array.json',
+                   environment=CppNamespaceEnvironment('%(namespace)s'))
+    m.AddNamespace(self.simple_api_json[0],
+                   'path/to/simple_api.json',
+                   environment=CppNamespaceEnvironment('%(namespace)s'))
+    manager = CppTypeGenerator(self.models.get('crossref_enums_array'),
+                               _FakeSchemaLoader(m))
+
+    self.assertEquals('#include "path/to/simple_api.h"',
+                      manager.GenerateIncludes().Render())
+
 if __name__ == '__main__':
   unittest.main()

tools/json_schema_compiler/test/crossref_enums.json

+[
+  {
+    "namespace": "crossref_enums",
+    "description": "The crossref API with only cross ref enums",
+    "dependencies": ["simple_api"],
+    "types": [
+      {
+        "id": "CrossrefType",
+        "type": "object",
+        "properties": {
+          "testEnumOptional": {
+            "$ref": "simple_api.TestEnum",
+            "optional": true
+          },
+          "testEnumOptionalExtra": {
+            "$ref": "simple_api.TestEnum",
+            "optional": true
+          }
+        }
+      }
+    ]
+  }
+]

tools/json_schema_compiler/test/crossref_enums_array.json

+[
+  {
+    "namespace": "crossref_enums_array",
+    "description": "The crossref API with only cross ref enums",
+    "dependencies": ["simple_api"],
+    "types": [
+      {
+        "id": "CrossrefType",
+        "type": "object",
+        "properties": {
+          "testEnumArray": {
+            "type": "array",
+            "items": { "$ref": "simple_api.TestEnum" }
+          }
+        }
+      }
+    ]
+  }
+]