[Extensions Schema Compiler] Extract C++ Type Generation

Extract C++ type generation from the json_schema_api gni
template into a separate generate_types gni template. This has the
advantage of cleaning up which arguments are necessary and used for
the template, as well as being able to isolate the different
generation steps (which may have different dependencies or
dependents).

As part of this, introduce separate "bulk" `api` targets in each
of the API directories, which each dependent can depend on. This
obviates the need to depend on e.g. `api` and `api_registration`.

A follow up will break up the JSON string generation bundle and the
extension function registration bundle.

Bug: 864576

Change-Id: I50a1fbe059ec5b7d73c405b230a221c1c6ce3af1
Reviewed-on: https://chromium-review.googlesource.com/1150930
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: Albert Chaulk <achaulk@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580799}

chrome/common/extensions/api/BUILD.gn

@@ -12,9 +12,10 @@ import("//tools/json_schema_compiler/json_schema_api.gni")
 
 assert(enable_extensions)
 
-json_schema_api("api") {
+# TODO(devlin): Enforce visibility restrictions on more of these targets?
+
+json_schema_api("generated_api_bundles") {
   sources = chrome_extensions_api_schema_sources
-  schemas = true
   bundle = true
   configs = [ "//build/config:precompiled_headers" ]
   bundle_name = "Chrome"
@@ -48,6 +49,21 @@ json_schema_api("api") {
   deps = [
     "//extensions/common/api",
   ]
+
+  visibility = [ ":api" ]
+}
+
+generated_types("generated_api_types") {
+  sources = chrome_extensions_api_schema_sources
+  configs = [ "//build/config:precompiled_headers" ]
+  schema_include_rules = chrome_extensions_api_schema_include_rules
+
+  root_namespace = chrome_extensions_api_root_namespace
+  deps = [
+    "//extensions/common/api",
+  ]
+
+  visibility = [ ":api" ]
 }
 
 json_features("api_features") {
@@ -74,6 +90,15 @@ json_features("manifest_features") {
   ]
 }
 
+# Public Targets
+
+group("api") {
+  public_deps = [
+    ":generated_api_bundles",
+    ":generated_api_types",
+  ]
+}
+
 group("extensions_features") {
   public_deps = [
     ":api_features",

chromecast/browser/BUILD.gn

@@ -308,7 +308,6 @@ cast_source_set("browser") {
 
     deps += [
       "//chromecast/common/extensions_api:api",
-      "//chromecast/common/extensions_api:api_registration",
       "//components/guest_view/browser",
       "//components/keyed_service/content",
       "//components/pref_registry",

chromecast/common/BUILD.gn

@@ -42,12 +42,11 @@ cast_source_set("common") {
 
     deps += [
       "//chromecast/common/extensions_api:api",
-      "//chromecast/common/extensions_api:api_registration",
       "//chromecast/common/extensions_api:extensions_features",
       "//components/version_info",
       "//extensions:extensions_resources_grd_grit",
       "//extensions/common",
-      "//extensions/common/api:generated_api",
+      "//extensions/common/api",
       "//extensions/shell:resources",
       "//extensions/shell:resources_grit",
     ]

chromecast/common/extensions_api/BUILD.gn

@@ -31,10 +31,9 @@ root_namespace = "extensions::cast::api::%(namespace)s"
 
 extensions_api_uncompiled_sources = [ "extension.json" ]
 
-json_schema_api("api") {
+json_schema_api("generated_api_strings_bundle") {
   sources = schema_sources
   uncompiled_sources = extensions_api_uncompiled_sources
-  schemas = true
   bundle = true
   bundle_name = "Cast"
   uncompiled_bundle_schema_sources = [ "tts.json" ]
@@ -43,19 +42,30 @@ json_schema_api("api") {
   deps = [
     "//extensions/common/api",
   ]
+  visibility = [ ":api" ]
 }
 
-json_schema_api("api_registration") {
+json_schema_api("generated_api_registration_bundle") {
   sources = schema_sources
   impl_dir = "//chromecast/browser/extensions/api"
   bundle_registration = true
   bundle_name = "Cast"
 
   deps = [
-    ":api",
     "//extensions/common",
     "//extensions/common/api",
   ]
+  visibility = [ ":api" ]
+}
+
+generated_types("generated_api_types") {
+  sources = schema_sources
+  schema_include_rules = extensions_api_schema_include_rules
+
+  deps = [
+    "//extensions/common/api",
+  ]
+  visibility = [ ":api" ]
 }
 
 json_features("cast_api_features") {
@@ -82,6 +92,16 @@ json_features("cast_manifest_features") {
   ]
 }
 
+# Public Targets
+
+group("api") {
+  public_deps = [
+    ":generated_api_registration_bundle",
+    ":generated_api_strings_bundle",
+    ":generated_api_types",
+  ]
+}
+
 group("extensions_features") {
   public_deps = [
     ":cast_api_features",

extensions/common/api/BUILD.gn

@@ -11,9 +11,10 @@ import("//tools/json_schema_compiler/json_schema_api.gni")
 
 assert(enable_extensions)
 
-json_schema_api("generated_api") {
+# TODO(devlin): Enforce visibility restrictions on more of these targets?
+
+json_schema_api("generated_api_bundles") {
   sources = extensions_api_schema_files
-  schemas = true
   bundle = true
   bundle_name = ""
   root_namespace = extensions_api_root_namespace
@@ -24,19 +25,22 @@ json_schema_api("generated_api") {
     "//base",
     "//extensions/buildflags",
   ]
+  visibility = [ ":api" ]
 }
 
-mojom("mojom") {
-  sources = [
-    "mime_handler.mojom",
+generated_types("generated_api_types") {
+  sources = extensions_api_schema_files
+  root_namespace = extensions_api_root_namespace
+  deps = [
+    "//base",
+    "//extensions/buildflags",
   ]
+  visibility = [ ":api" ]
 }
 
-group("api") {
-  public_deps = [
-    ":generated_api",
-    ":mojom",
-    "//extensions/buildflags",
+mojom("mojom") {
+  sources = [
+    "mime_handler.mojom",
   ]
 }
 
@@ -72,6 +76,17 @@ json_features("behavior_features") {
   ]
 }
 
+# Public Targets
+
+group("api") {
+  public_deps = [
+    ":generated_api_bundles",
+    ":generated_api_types",
+    ":mojom",
+    "//extensions/buildflags",
+  ]
+}
+
 group("extensions_features") {
   public_deps = [
     ":api_features",

extensions/shell/BUILD.gn

@@ -65,7 +65,6 @@ source_set("app_shell_lib") {
     "//extensions/renderer",
     "//extensions/shell/browser/system_logs",
     "//extensions/shell/common/api",
-    "//extensions/shell/common/api:api_registration",
     "//extensions/shell/common/api:extensions_features",
     "//google_apis",
     "//third_party/blink/public:blink",

extensions/shell/common/api/BUILD.gn

@@ -9,26 +9,36 @@ import("//tools/json_schema_compiler/json_schema_api.gni")
 assert(enable_extensions,
        "Cannot depend on extensions because enable_extensions=false.")
 
+# TODO(devlin): Enforce visibility restrictions on more of these targets?
+
 schema_sources = [ "identity.idl" ]
 root_namespace = "extensions::shell::api::%(namespace)s"
 
-json_schema_api("api") {
-  sources = schema_sources
-  schemas = true
-  bundle = true
-  bundle_name = "Shell"
-}
-
-json_schema_api("api_registration") {
+json_schema_api("generated_api_registration_bundle") {
   sources = schema_sources
   impl_dir = "//extensions/shell/browser/api"
   bundle_registration = true
   bundle_name = "Shell"
 
   deps = [
-    ":api",
     "//extensions/common",
   ]
+
+  visibility = [ ":api" ]
+}
+
+json_schema_api("generated_api_strings_bundle") {
+  sources = schema_sources
+  bundle = true
+  bundle_name = "Shell"
+
+  visibility = [ ":api" ]
+}
+
+generated_types("generated_api_types") {
+  sources = schema_sources
+
+  visibility = [ ":api" ]
 }
 
 json_features("shell_api_features") {
@@ -39,6 +49,16 @@ json_features("shell_api_features") {
   ]
 }
 
+# Public Targets
+
+group("api") {
+  public_deps = [
+    ":generated_api_registration_bundle",
+    ":generated_api_strings_bundle",
+    ":generated_api_types",
+  ]
+}
+
 group("extensions_features") {
   public_deps = [
     ":shell_api_features",

tools/json_schema_compiler/json_schema_api.gni

@@ -2,16 +2,19 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-# Defines a static library corresponding to the output of schema compiler tools
-# over a set of extensions API schemas (IDL or JSON format.) The library target
+# This file contains templates for generating static libraries based on the
+# corresponding output of the schema compiler tools. The output can be either
+# the generated C++ types (generated_types template), the bundled extension
+# function registration (json_schema_api template with
+# bundle_registration = true), or the bundled JSON strings of the APIs
+# (json_schema_api template with bundle = true). The generated library target
 # has implicit hard dependencies on all schema files listed by the invoker and
 # is itself a hard dependency.
 #
-# Invocations of this template may use the following variables:
+# Common variables that can be used in all templates are:
+# - sources [required] A list of schema files used to generate the C++ types.
 #
-# sources [required] A list of schema files to be compiled.
-#
-# root_namespace [required]
+# - root_namespace [required]
 #     A Python string substituion pattern used to generate the C++
 #     namespace for each API. Use %(namespace)s to replace with the API
 #     namespace, like "toplevel::%(namespace)s_api".
@@ -22,8 +25,38 @@
 #   Example:
 #     [ '/foo/bar:Foo::Bar::%(namespace)s' ]
 #
-# schemas [optional, default = false]
-#   Boolean indicating if the schema files should be generated.
+# - configs [optional]
+#     Extra gn configs to apply to the compile step.
+#
+# - deps [optional]
+#     If any deps are specified they will be inherited by the static library
+#     target.
+#
+# - visibility [optional]
+#     A specific visibility to apply for the generated static library. If
+#     omitted, visibility will be inherited from the invoker.
+
+# NOTE: Common variables here for when multiple templates use them.
+compiler_root = "//tools/json_schema_compiler"
+compiler_script = "$compiler_root/compiler.py"
+compiler_sources = [
+  "$compiler_root/cc_generator.py",
+  "$compiler_root/code.py",
+  "$compiler_root/compiler.py",
+  "$compiler_root/cpp_bundle_generator.py",
+  "$compiler_root/cpp_generator.py",
+  "$compiler_root/cpp_type_generator.py",
+  "$compiler_root/cpp_util.py",
+  "$compiler_root/h_generator.py",
+  "$compiler_root/idl_schema.py",
+  "$compiler_root/model.py",
+  "$compiler_root/util_cc_helper.py",
+]
+
+# Outputs either the bundle of extension function registrations or the bundle of
+# generated JSON strings for each API.
+#
+# Template-specific variables (in addition to the common ones described above):
 #
 # bundle [optional, default = false]
 #   Boolean indicating if the schema bundle files should be generated.
@@ -31,7 +64,7 @@
 # bundle_registration [optional, default = false]
 #   Boolean indicating if the API registration bundle files should be generated.
 #
-# bundle_name [required if bundle or bundle_registrations]:
+# bundle_name [required]
 #   A string to prepend to generated bundle class names, so that multiple
 #   bundle rules can be used without conflicting.  Only used with one of
 #   the cpp-bundle generators.
@@ -42,8 +75,7 @@
 #   when generating API registration bundles. Such headers, if found, are
 #   automatically included by the generated code.
 #
-# uncompiled_sources [optional, only used when bundle = true or
-#     bundle_registration = true]
+# uncompiled_sources [optional]
 #   A list of schema files which should not be compiled, but which should still
 #   be processed for API bundle generation.
 #
@@ -51,44 +83,19 @@
 #   A list of schema files which should not be compiled nor registered, but
 #   should still be processed for the schema generation.
 #
-# configs [optional]
-#   Extra configs to apply to the compile step.
-#
-# deps [optional]
-#   If any deps are specified they will be inherited by the static library
-#   target.
-#
-# visibility [optional]
-#   A specific visibility to apply for the generated static library. If omitted,
-#   visibility will be inherited from the invoker.
-
-# NOTE: Common variables here for when multiple templates use them.
-compiler_root = "//tools/json_schema_compiler"
-compiler_script = "$compiler_root/compiler.py"
-compiler_sources = [
-  "$compiler_root/cc_generator.py",
-  "$compiler_root/code.py",
-  "$compiler_root/compiler.py",
-  "$compiler_root/cpp_bundle_generator.py",
-  "$compiler_root/cpp_generator.py",
-  "$compiler_root/cpp_type_generator.py",
-  "$compiler_root/cpp_util.py",
-  "$compiler_root/h_generator.py",
-  "$compiler_root/idl_schema.py",
-  "$compiler_root/model.py",
-  "$compiler_root/util_cc_helper.py",
-]
-
+# TODO(devlin): Break this up into two targets, one for extension function
+# registration and one for generated JSON strings. https://crbug.com/864576.
 template("json_schema_api") {
   assert(defined(invoker.sources),
          "\"sources\" must be defined for the $target_name template.")
   assert(defined(invoker.root_namespace),
          "\"root_namespace\" must be defined for the $target_name template.")
 
-  schemas = defined(invoker.schemas) && invoker.schemas
   bundle = defined(invoker.bundle) && invoker.bundle
   bundle_registration =
       defined(invoker.bundle_registration) && invoker.bundle_registration
+  assert(bundle || bundle_registration,
+         "$target_name must specify \"bundle\" or \"bundle_registration\".")
 
   schema_include_rules = ""
   if (defined(invoker.schema_include_rules)) {
@@ -107,28 +114,6 @@ template("json_schema_api") {
   # target_name variable.
   root_target_name = target_name
 
-  if (schemas) {
-    schema_generator_name = target_name + "_schema_generator"
-    action_foreach(schema_generator_name) {
-      visibility = [":$root_target_name"]
-      script = compiler_script
-      sources = invoker.sources
-      inputs = compiler_sources
-      outputs = [
-        "$target_gen_dir/{{source_name_part}}.cc",
-        "$target_gen_dir/{{source_name_part}}.h",
-      ]
-      args = [
-        "{{source}}",
-        "--root=" + rebase_path("//", root_build_dir),
-        "--destdir=" + rebase_path(root_gen_dir, root_build_dir),
-        "--namespace=$root_namespace",
-        "--generator=cpp",
-        "--include-rules=$schema_include_rules",
-      ]
-    }
-  }
-
   if (bundle) {
     assert(defined(invoker.bundle_name),
            "\"bundle_name\" must be defined for bundles")
@@ -146,7 +131,7 @@ template("json_schema_api") {
 
     bundle_generator_schema_name = target_name + "_bundle_generator_schema"
     action(bundle_generator_schema_name) {
-      visibility = [":$root_target_name"]
+      visibility = [ ":$root_target_name" ]
       script = compiler_script
       inputs = compiler_sources + invoker.sources + uncompiled_sources +
                uncompiled_bundle_schema_sources
@@ -185,7 +170,7 @@ template("json_schema_api") {
     bundle_generator_registration_name =
         target_name + "_bundle_generator_registration"
     action(bundle_generator_registration_name) {
-      visibility = [":$root_target_name"]
+      visibility = [ ":$root_target_name" ]
       script = compiler_script
       inputs = compiler_sources + invoker.sources + uncompiled_sources
       outputs = [
@@ -214,13 +199,6 @@ template("json_schema_api") {
     lib_extra_configs += invoker.configs
   }
 
-  if (schemas) {
-    lib_sources += get_target_outputs(":$schema_generator_name")
-    lib_public_deps += [ ":$schema_generator_name" ]
-    lib_deps += [ "//tools/json_schema_compiler:generated_api_util" ]
-    lib_extra_configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
-  }
-
   if (bundle) {
     lib_sources += get_target_outputs(":$bundle_generator_schema_name")
     lib_deps += [ ":$bundle_generator_schema_name" ]
@@ -247,3 +225,73 @@ template("json_schema_api") {
     }
   }
 }
+
+# Generates the C++ types for the given APIs.
+template("generated_types") {
+  assert(defined(invoker.sources),
+         "\"sources\" must be defined for the $target_name template.")
+  assert(defined(invoker.root_namespace),
+         "\"root_namespace\" must be defined for the $target_name template.")
+
+  schema_include_rules = ""
+  if (defined(invoker.schema_include_rules)) {
+    schema_include_rules = invoker.schema_include_rules
+  }
+
+  generated_config_name = target_name + "_generated_config"
+  config(generated_config_name) {
+    include_dirs = [ root_gen_dir ]
+  }
+
+  root_namespace = invoker.root_namespace
+
+  # Save the target_name, since other targets (like the action() and
+  # action_foreach() below) need to reference them, but would have their own
+  # target_name variable.
+  root_target_name = target_name
+
+  schema_generator_name = target_name + "_schema_generator"
+  action_foreach(schema_generator_name) {
+    visibility = [ ":$root_target_name" ]
+    script = compiler_script
+    sources = invoker.sources
+    inputs = compiler_sources
+    outputs = [
+      "$target_gen_dir/{{source_name_part}}.cc",
+      "$target_gen_dir/{{source_name_part}}.h",
+    ]
+    args = [
+      "{{source}}",
+      "--root=" + rebase_path("//", root_build_dir),
+      "--destdir=" + rebase_path(root_gen_dir, root_build_dir),
+      "--namespace=$root_namespace",
+      "--generator=cpp",
+      "--include-rules=$schema_include_rules",
+    ]
+  }
+
+  # Compute the contents of the library/source set.
+  lib_sources = invoker.sources + get_target_outputs(":$schema_generator_name")
+  lib_public_deps = [ ":$schema_generator_name" ]
+  lib_deps = [ "//tools/json_schema_compiler:generated_api_util" ]
+  lib_extra_configs = [ "//build/config/compiler:no_size_t_to_int_warning" ]
+  if (defined(invoker.configs)) {
+    lib_extra_configs += invoker.configs
+  }
+
+  if (defined(invoker.deps)) {
+    lib_deps += invoker.deps
+  }
+
+  static_library(target_name) {
+    sources = lib_sources
+    deps = lib_deps
+    public_deps = lib_public_deps
+    configs += lib_extra_configs
+    public_configs = [ ":$generated_config_name" ]
+
+    if (defined(invoker.visibility)) {
+      visibility = invoker.visibility
+    }
+  }
+}

tools/json_schema_compiler/test/BUILD.gn

@@ -9,7 +9,7 @@ import("//tools/json_schema_compiler/json_schema_api.gni")
 
 assert(enable_extensions)
 
-json_schema_api("api") {
+generated_types("generated_types") {
   visibility = [ ":*" ]
 
   sources = [
@@ -34,7 +34,6 @@ json_schema_api("api") {
     "simple_api.json",
   ]
 
-  schemas = true
   root_namespace = "test::api::%(namespace)s"
 
   deps = [
@@ -79,7 +78,7 @@ source_set("unit_tests") {
   ]
 
   deps = [
-    ":api",
+    ":generated_types",
     "//base",
     "//testing/gtest",
   ]