Support EnableIf attribute to conditionally disable definitions.

Add support for a new EnableIf attribute to conditionally
enable fields/types in mojom definitions.

In order to do this, mojom parsing and code generation has
been split into two separate GN stages. The mojom bindings
generator has been refactored to separate the logic for
parsing and code generation and the GN mojom template has
been updated to express these two distinct stages.

The parse stage now prunes a mojom's AST - filtering
definitions based on the enabled features. These
intermediate ASTs are then pickled to gen/ to be later
read by the code generation stage.

Bug: 676224
Change-Id: I5fc106b43e8ac48339c63c48f7ce42ba5145d174
Reviewed-on: https://chromium-review.googlesource.com/846399
Commit-Queue: Eve Martin-Jones <evem@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Chris Watkins <watk@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533796}

mojo/public/tools/bindings/README.md

@@ -395,6 +395,14 @@ interesting attributes supported today.
     field, enum value, interface method, or method parameter was introduced.
     See [Versioning](#Versioning) for more details.
 
+**`[EnableIf=value]`**
+:   The `EnableIf` attribute is used to conditionally enable definitions when
+    the mojom is parsed. If the `mojom` target in the GN file does not include
+    the matching `value` in the list of `enabled_features`, the definition
+    will be disabled. This is useful for mojom definitions that only make
+    sense on one platform. Note that the `EnableIf` attribute can only be set
+    once per definition.
+
 ## Generated Code For Target Languages
 
 When the bindings generator successfully processes an input Mojom file, it emits

mojo/public/tools/bindings/mojom.gni

@@ -232,6 +232,12 @@ if (enable_mojom_typemapping) {
 #       EXPORT macro depends on whether the corresponding IMPL macro is defined,
 #       per standard practice with Chromium component exports.
 #
+#   enabled_features (optional)
+#       Definitions in a mojom file can be guarded by an EnableIf attribute. If
+#       the value specified by the attribute does not match any items in the
+#       list of enabled_features, the definition will be disabled, with no code
+#       emitted for it.
+#
 # The following parameters are used to support the component build. They are
 # needed so that bindings which are linked with a component can use the same
 # export settings for classes. The first three are for the chromium variant, and
@@ -331,6 +337,48 @@ template("mojom") {
     }
   }
 
+  if (defined(invoker.sources)) {
+    parser_target_name = "${target_name}__parser"
+    action_foreach(parser_target_name) {
+      script = mojom_generator_script
+      sources = invoker.sources
+      outputs = [
+        "{{source_gen_dir}}/{{source_name_part}}.p",
+      ]
+      args = [
+        "parse",
+        "{{source}}",
+        "-o",
+        rebase_path(root_gen_dir, root_build_dir),
+        "-d",
+        rebase_path("//", root_build_dir),
+      ]
+      if (defined(invoker.enabled_features)) {
+        foreach(enabled_feature, invoker.enabled_features) {
+          args += [
+            "--enable_feature",
+            enabled_feature,
+          ]
+        }
+      }
+    }
+  }
+
+  parsed_target_name = "${target_name}__parsed"
+  group(parsed_target_name) {
+    public_deps = []
+    if (defined(invoker.sources)) {
+      public_deps += [ ":$parser_target_name" ]
+    }
+    foreach(d, all_deps) {
+      # Resolve the name, so that a target //mojo/something becomes
+      # //mojo/something:something and we can append the parsed
+      # suffix to get the mojom dependency name.
+      full_name = get_label_info("$d", "label_no_toolchain")
+      public_deps += [ "${full_name}__parsed" ]
+    }
+  }
+
   # Generate code that is shared by different variants.
   if (defined(invoker.sources)) {
     common_generator_args = [
@@ -436,6 +484,7 @@ template("mojom") {
       inputs = mojom_generator_sources
       sources = invoker.sources
       deps = [
+        ":$parsed_target_name",
         "//mojo/public/tools/bindings:precompile_templates",
       ]
       outputs = generator_shared_cpp_outputs
@@ -445,13 +494,6 @@ template("mojom") {
         "-g",
         "c++",
       ]
-      depfile = "{{source_gen_dir}}/${generator_shared_target_name}_{{source_name_part}}.d"
-      args += [
-        "--depfile",
-        depfile,
-        "--depfile_target",
-        "{{source_gen_dir}}/{{source_name_part}}.mojom-shared-internal.h",
-      ]
 
       if (defined(shared_component_export_macro)) {
         args += [
@@ -667,6 +709,7 @@ template("mojom") {
         inputs = mojom_generator_sources
         sources = invoker.sources
         deps = [
+          ":$parsed_target_name",
           ":$type_mappings_target_name",
           "//mojo/public/tools/bindings:precompile_templates",
         ]
@@ -691,14 +734,6 @@ template("mojom") {
             bindings_configuration.variant,
           ]
         }
-        depfile =
-            "{{source_gen_dir}}/${generator_target_name}_{{source_name_part}}.d"
-        args += [
-          "--depfile",
-          depfile,
-          "--depfile_target",
-          "{{source_gen_dir}}/{{source_name_part}}.mojom${variant_dash_suffix}.cc",
-        ]
 
         args += [
           "--typemap",
@@ -989,6 +1024,7 @@ template("mojom") {
           sources += invoker.sources
         }
         deps = [
+          ":$parsed_target_name",
           "//mojo/public/tools/bindings:precompile_templates",
         ]
         outputs = generator_js_outputs

mojo/public/tools/bindings/mojom_bindings_generator.py

@@ -7,6 +7,7 @@
 
 
 import argparse
+import cPickle
 import hashlib
 import importlib
 import json
@@ -42,6 +43,7 @@ import mojom.fileutil as fileutil
 from mojom.generate import translate
 from mojom.generate import template_expander
 from mojom.generate.generator import AddComputedData
+from mojom.parse.conditional_features import RemoveDisabledDefinitions
 from mojom.parse.parser import Parse
 
 
@@ -146,7 +148,6 @@ class MojomProcessor(object):
   def __init__(self, should_generate):
     self._should_generate = should_generate
     self._processed_files = {}
-    self._parsed_files = {}
     self._typemap = {}
 
   def LoadTypemaps(self, typemaps):
@@ -162,19 +163,24 @@ class MojomProcessor(object):
           language_map.update(typemap)
           self._typemap[language] = language_map
 
-  def ProcessFile(self, args, remaining_args, generator_modules, filename):
-    self._ParseFileAndImports(RelativePath(filename, args.depth),
-                              args.import_directories, [])
-
-    return self._GenerateModule(args, remaining_args, generator_modules,
-        RelativePath(filename, args.depth))
-
   def _GenerateModule(self, args, remaining_args, generator_modules,
-                      rel_filename):
+                      rel_filename, imported_filename_stack):
     # Return the already-generated module.
     if rel_filename.path in self._processed_files:
       return self._processed_files[rel_filename.path]
-    tree = self._parsed_files[rel_filename.path]
+
+    if rel_filename.path in imported_filename_stack:
+      print "%s: Error: Circular dependency" % rel_filename.path + \
+          MakeImportStackMessage(imported_filename_stack + [rel_filename.path])
+      sys.exit(1)
+
+    pickle_path = _GetPicklePath(rel_filename, args.output_dir)
+    try:
+      with open(pickle_path, "rb") as f:
+        tree = cPickle.load(f)
+    except (IOError, cPickle.UnpicklingError) as e:
+      print "%s: Error: %s" % (pickle_path, str(e))
+      sys.exit(1)
 
     dirname = os.path.dirname(rel_filename.path)
 
@@ -186,7 +192,8 @@ class MojomProcessor(object):
           RelativePath(dirname, rel_filename.source_root),
           parsed_imp.import_filename, args.import_directories)
       imports[parsed_imp.import_filename] = self._GenerateModule(
-          args, remaining_args, generator_modules, rel_import_file)
+          args, remaining_args, generator_modules, rel_import_file,
+          imported_filename_stack + [rel_filename.path])
 
     # Set the module path as relative to the source root.
     # Normalize to unix-style path here to keep the generators simpler.
@@ -225,42 +232,6 @@ class MojomProcessor(object):
     self._processed_files[rel_filename.path] = module
     return module
 
-  def _ParseFileAndImports(self, rel_filename, import_directories,
-      imported_filename_stack):
-    # Ignore already-parsed files.
-    if rel_filename.path in self._parsed_files:
-      return
-
-    if rel_filename.path in imported_filename_stack:
-      print "%s: Error: Circular dependency" % rel_filename.path + \
-          MakeImportStackMessage(imported_filename_stack + [rel_filename.path])
-      sys.exit(1)
-
-    try:
-      with open(rel_filename.path) as f:
-        source = f.read()
-    except IOError as e:
-      print "%s: Error: %s" % (rel_filename.path, e.strerror) + \
-          MakeImportStackMessage(imported_filename_stack + [rel_filename.path])
-      sys.exit(1)
-
-    try:
-      tree = Parse(source, rel_filename.path)
-    except Error as e:
-      full_stack = imported_filename_stack + [rel_filename.path]
-      print str(e) + MakeImportStackMessage(full_stack)
-      sys.exit(1)
-
-    dirname = os.path.split(rel_filename.path)[0]
-    for imp_entry in tree.import_list:
-      import_file_entry = FindImportFile(
-          RelativePath(dirname, rel_filename.source_root),
-          imp_entry.import_filename, import_directories)
-      self._ParseFileAndImports(import_file_entry, import_directories,
-          imported_filename_stack + [rel_filename.path])
-
-    self._parsed_files[rel_filename.path] = tree
-
 
 def _Generate(args, remaining_args):
   if args.variant == "none":
@@ -279,17 +250,55 @@ def _Generate(args, remaining_args):
   processor = MojomProcessor(lambda filename: filename in args.filename)
   processor.LoadTypemaps(set(args.typemaps))
   for filename in args.filename:
-    processor.ProcessFile(args, remaining_args, generator_modules, filename)
-  if args.depfile:
-    assert args.depfile_target
-    with open(args.depfile, 'w') as f:
-      f.write('%s: %s' % (
-          args.depfile_target,
-          ' '.join(processor._parsed_files.keys())))
+    processor._GenerateModule(args, remaining_args, generator_modules,
+                              RelativePath(filename, args.depth), [])
 
   return 0
 
 
+def _GetPicklePath(rel_filename, output_dir):
+  filename, _ = os.path.splitext(rel_filename.relative_path())
+  pickle_path = filename + '.p'
+  return os.path.join(output_dir, pickle_path)
+
+
+def _PickleAST(ast, output_file):
+  full_dir = os.path.dirname(output_file)
+  fileutil.EnsureDirectoryExists(full_dir)
+
+  try:
+    with open(output_file, "wb") as f:
+      cPickle.dump(ast, f)
+  except (IOError, cPickle.PicklingError) as e:
+    print "%s: Error: %s" % (output_file, str(e))
+    sys.exit(1)
+
+
+def _ParseFile(args, rel_filename):
+  try:
+    with open(rel_filename.path) as f:
+      source = f.read()
+  except IOError as e:
+    print "%s: Error: %s" % (rel_filename.path, e.strerror)
+    sys.exit(1)
+
+  try:
+    tree = Parse(source, rel_filename.path)
+    RemoveDisabledDefinitions(tree, args.enabled_features)
+  except Error as e:
+    print "%s: Error: %s" % (rel_filename.path, str(e))
+    sys.exit(1)
+
+  _PickleAST(tree, _GetPicklePath(rel_filename, args.output_dir))
+
+
+def _Parse(args, _):
+  fileutil.EnsureDirectoryExists(args.output_dir)
+  for filename in args.filename:
+    _ParseFile(args, RelativePath(filename, args.depth))
+  return 0
+
+
 def _Precompile(args, _):
   generator_modules = LoadGenerators(",".join(_BUILTIN_GENERATORS.keys()))
 
@@ -305,6 +314,29 @@ def main():
                       help="use Python modules bundled in the SDK")
 
   subparsers = parser.add_subparsers()
+
+  parse_parser = subparsers.add_parser(
+      "parse", description="Parse mojom to AST and remove disabled definitions."
+                           " Pickle pruned AST into output_dir.")
+  parse_parser.add_argument("filename", nargs="+", help="mojom input file")
+  parse_parser.add_argument(
+      "-o",
+      "--output_dir",
+      dest="output_dir",
+      default=".",
+      help="output directory for generated files")
+  parse_parser.add_argument(
+      "-d", "--depth", dest="depth", default=".", help="depth from source root")
+  parse_parser.add_argument(
+      "--enable_feature",
+      dest = "enabled_features",
+      default=[],
+      action="append",
+      help="Controls which definitions guarded by an EnabledIf attribute "
+      "will be enabled. If an EnabledIf attribute does not specify a value "
+      "that matches one of the enabled features, it will be disabled.")
+  parse_parser.set_defaults(func=_Parse)
+
   generate_parser = subparsers.add_parser(
       "generate", description="Generate bindings from mojom files.")
   generate_parser.add_argument("filename", nargs="+",
@@ -359,12 +391,6 @@ def main():
   generate_parser.add_argument(
       "--generate_non_variant_code", action="store_true",
       help="Generate code that is shared by different variants.")
-  generate_parser.add_argument(
-      "--depfile",
-      help="A file into which the list of input files will be written.")
-  generate_parser.add_argument(
-      "--depfile_target",
-      help="The target name to use in the depfile.")
   generate_parser.add_argument(
       "--scrambled_message_id_salt_path",
       dest="scrambled_message_id_salt_paths",

mojo/public/tools/bindings/pylib/mojom/parse/conditional_features.py

+# Copyright 2018 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.
+
+"""Helpers for processing conditionally enabled features in a mojom."""
+
+from . import ast
+from ..error import Error
+
+class EnableIfError(Error):
+  """ Class for errors from ."""
+
+  def __init__(self, filename, message, lineno=None):
+    Error.__init__(self, filename, message, lineno=lineno, addenda=None)
+
+def _IsEnabled(definition, enabled_features):
+  """Returns true if a definition is enabled.
+
+  A definition is enabled if it has no EnableIf attribute, or if the value of
+  the EnableIf attribute is in enabled_features.
+  """
+  if not hasattr(definition, "attribute_list"):
+    return True
+  if not definition.attribute_list:
+    return True
+
+  already_defined = False
+  for a in definition.attribute_list:
+    if a.key == 'EnableIf':
+      if already_defined:
+        raise EnableIfError(definition.filename,
+          "EnableIf attribute may only be defined once per field.",
+          definition.lineno)
+      already_defined = True
+
+  for attribute in definition.attribute_list:
+    if attribute.key == 'EnableIf' and attribute.value not in enabled_features:
+      return False
+  return True
+
+
+def _FilterDisabledFromNodeList(node_list, enabled_features):
+  if not node_list:
+    return
+  assert isinstance(node_list, ast.NodeListBase)
+  node_list.items = [
+      item for item in node_list.items if _IsEnabled(item, enabled_features)
+  ]
+  for item in node_list.items:
+    _FilterDefinition(item, enabled_features)
+
+
+def _FilterDefinition(definition, enabled_features):
+  """Filters definitions with a body."""
+  if isinstance(definition, ast.Enum):
+    _FilterDisabledFromNodeList(definition.enum_value_list, enabled_features)
+  elif isinstance(definition, ast.Interface):
+    _FilterDisabledFromNodeList(definition.body, enabled_features)
+  elif isinstance(definition, ast.Method):
+    _FilterDisabledFromNodeList(definition.parameter_list, enabled_features)
+    _FilterDisabledFromNodeList(definition.response_parameter_list,
+                                enabled_features)
+  elif isinstance(definition, ast.Struct):
+    _FilterDisabledFromNodeList(definition.body, enabled_features)
+  elif isinstance(definition, ast.Union):
+    _FilterDisabledFromNodeList(definition.body, enabled_features)
+
+
+def RemoveDisabledDefinitions(mojom, enabled_features):
+  """Removes conditionally disabled definitions from a Mojom node."""
+  mojom.definition_list = [
+      definition for definition in mojom.definition_list
+          if _IsEnabled(definition, enabled_features)
+  ]
+  for definition in mojom.definition_list:
+    _FilterDefinition(definition, enabled_features)

mojo/public/tools/bindings/pylib/mojom_tests/parse/conditional_features_unittest.py

+# Copyright 2018 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.
+
+import imp
+import os
+import sys
+import unittest
+
+
+def _GetDirAbove(dirname):
+  """Returns the directory "above" this file containing |dirname| (which must
+  also be "above" this file)."""
+  path = os.path.abspath(__file__)
+  while True:
+    path, tail = os.path.split(path)
+    assert tail
+    if tail == dirname:
+      return path
+
+
+try:
+  imp.find_module('mojom')
+except ImportError:
+  sys.path.append(os.path.join(_GetDirAbove('pylib'), 'pylib'))
+import mojom.parse.ast as ast
+import mojom.parse.conditional_features as conditional_features
+import mojom.parse.parser as parser
+
+ENABLED_FEATURES = frozenset({'red', 'green', 'blue'})
+
+
+class ConditionalFeaturesTest(unittest.TestCase):
+  """Tests |mojom.parse.conditional_features|."""
+
+  def parseAndAssertEqual(self, source, expected_source):
+    definition = parser.Parse(source, "my_file.mojom")
+    conditional_features.RemoveDisabledDefinitions(definition, ENABLED_FEATURES)
+    expected = parser.Parse(expected_source, "my_file.mojom")
+    self.assertEquals(definition, expected)
+
+  def testFilterConst(self):
+    """Test that Consts are correctly filtered."""
+    const_source = """
+      [EnableIf=blue]
+      const int kMyConst1 = 1;
+      [EnableIf=orange]
+      const double kMyConst2 = 2;
+      const int kMyConst3 = 3;
+    """
+    expected_source = """
+      [EnableIf=blue]
+      const int kMyConst1 = 1;
+      const int kMyConst3 = 3;
+    """
+    self.parseAndAssertEqual(const_source, expected_source)
+
+  def testFilterEnum(self):
+    """Test that EnumValues are correctly filtered from an Enum."""
+    enum_source = """
+      enum MyEnum {
+        [EnableIf=purple]
+        VALUE1,
+        [EnableIf=blue]
+        VALUE2,
+        VALUE3,
+      };
+    """
+    expected_source = """
+      enum MyEnum {
+        [EnableIf=blue]
+        VALUE2,
+        VALUE3
+      };
+    """
+    self.parseAndAssertEqual(enum_source, expected_source)
+
+  def testFilterInterface(self):
+    """Test that definitions are correctly filtered from an Interface."""
+    interface_source = """
+      interface MyInterface {
+        [EnableIf=blue]
+        enum MyEnum {
+          [EnableIf=purple]
+          VALUE1,
+          VALUE2,
+        };
+        [EnableIf=blue]
+        const int32 kMyConst = 123;
+        [EnableIf=purple]
+        MyMethod();
+      };
+    """
+    expected_source = """
+      interface MyInterface {
+        [EnableIf=blue]
+        enum MyEnum {
+          VALUE2,
+        };
+        [EnableIf=blue]
+        const int32 kMyConst = 123;
+      };
+    """
+    self.parseAndAssertEqual(interface_source, expected_source)
+
+  def testFilterMethod(self):
+    """Test that Parameters are correctly filtered from a Method."""
+    method_source = """
+      interface MyInterface {
+        [EnableIf=blue]
+        MyMethod([EnableIf=purple] int32 a) => ([EnableIf=red] int32 b);
+      };
+    """
+    expected_source = """
+      interface MyInterface {
+        [EnableIf=blue]
+        MyMethod() => ([EnableIf=red] int32 b);
+      };
+    """
+    self.parseAndAssertEqual(method_source, expected_source)
+
+  def testFilterStruct(self):
+    """Test that definitions are correctly filtered from a Struct."""
+    struct_source =  """
+      struct MyStruct {
+        [EnableIf=blue]
+        enum MyEnum {
+          VALUE1,
+          [EnableIf=purple]
+          VALUE2,
+        };
+        [EnableIf=yellow]
+        const double kMyConst = 1.23;
+        [EnableIf=green]
+        int32 a;
+        double b;
+        [EnableIf=purple]
+        int32 c;
+        [EnableIf=blue]
+        double d;
+        int32 e;
+        [EnableIf=orange]
+        double f;
+      };
+    """
+    expected_source = """
+      struct MyStruct {
+        [EnableIf=blue]
+        enum MyEnum {
+          VALUE1,
+        };
+        [EnableIf=green]
+        int32 a;
+        double b;
+        [EnableIf=blue]
+        double d;
+        int32 e;
+      };
+    """
+    self.parseAndAssertEqual(struct_source, expected_source)
+
+  def testFilterUnion(self):
+    """Test that UnionFields are correctly filtered from a Union."""
+    union_source = """
+      union MyUnion {
+        [EnableIf=yellow]
+        int32 a;
+        [EnableIf=red]
+        bool b;
+      };
+    """
+    expected_source = """
+      union MyUnion {
+        [EnableIf=red]
+        bool b;
+      };
+    """
+    self.parseAndAssertEqual(union_source, expected_source)
+
+  def testSameNameFields(self):
+    mojom_source = """
+      enum Foo {
+        [EnableIf=red]
+        VALUE1 = 5,
+        [EnableIf=yellow]
+        VALUE1 = 6,
+      };
+      [EnableIf=red]
+      const double kMyConst = 1.23;
+      [EnableIf=yellow]
+      const double kMyConst = 4.56;
+    """
+    expected_source = """
+      enum Foo {
+        [EnableIf=red]
+        VALUE1 = 5,
+      };
+      [EnableIf=red]
+      const double kMyConst = 1.23;
+    """
+    self.parseAndAssertEqual(mojom_source, expected_source)
+
+  def testMultipleEnableIfs(self):
+    source = """
+      enum Foo {
+        [EnableIf=red,EnableIf=yellow]
+        kBarValue = 5,
+      };
+    """
+    definition = parser.Parse(source, "my_file.mojom")
+    self.assertRaises(conditional_features.EnableIfError,
+      conditional_features.RemoveDisabledDefinitions,
+      definition, ENABLED_FEATURES)
+
+if __name__ == '__main__':
+  unittest.main()