Reduce rebuild cost of VERSION changes

Touching VERSION elicits a regeneration of all mojom message IDs on
platforms where message ID scrambling is enabled.

Before this CL, that regeneration step induces a rebuild of over
13000 targets when building chrome, as all mojom headers are
regenerated, and dependencies on mojom headers abound.

After this CL, the regeneration step only forces a rebuild of
a new per-mojom header dedicated exclusively to message ID
definitions, affecting only the corresponding mojom.cc files
and reducing the rebuild cost to about 2700 targets.

This also introduces a new GN arg |enable_mojom_message_id_scrambling|
which defaults to |true|, enabling the current scrambling behavior on
all supported platforms. If set to |false|, message IDs will never
be scrambled and changes to VERSION won't affect mojom in any way.

Bug: 808162
Change-Id: I8c4e4fa1dcc8d1d4349febee7dea6cce824ca0a6
Reviewed-on: https://chromium-review.googlesource.com/900528Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537446}

mojo/public/tools/bindings/BUILD.gn

@@ -17,6 +17,7 @@ action("precompile_templates") {
     "$mojom_generator_root/generators/cpp_templates/interface_response_validator_declaration.tmpl",
     "$mojom_generator_root/generators/cpp_templates/interface_stub_declaration.tmpl",
     "$mojom_generator_root/generators/cpp_templates/module-shared-internal.h.tmpl",
+    "$mojom_generator_root/generators/cpp_templates/module-shared-message-ids.h.tmpl",
     "$mojom_generator_root/generators/cpp_templates/module-shared.cc.tmpl",
     "$mojom_generator_root/generators/cpp_templates/module-shared.h.tmpl",
     "$mojom_generator_root/generators/cpp_templates/module.cc.tmpl",

mojo/public/tools/bindings/generators/cpp_templates/module-shared-internal.h.tmpl

@@ -9,8 +9,6 @@
 #ifndef {{header_guard}}
 #define {{header_guard}}
 
-#include <stdint.h>
-
 #include "mojo/public/cpp/bindings/lib/array_internal.h"
 #include "mojo/public/cpp/bindings/lib/bindings_internal.h"
 #include "mojo/public/cpp/bindings/lib/map_data_internal.h"
@@ -83,11 +81,6 @@ using {{enum|get_name_for_kind(flatten_nested_kind=True)}}_Data =
 {#--- Interface parameter definitions #}
 {%- for interface in interfaces %}
 {%-   for method in interface.methods %}
-{%-     if method.ordinal_comment %}
-// {{method.ordinal_comment}}
-{%-     endif %}
-{%-     set method_name = "k%s_%s_Name"|format(interface.name, method.name) %}
-constexpr uint32_t {{method_name}} = {{method.ordinal}};
 {%-     set struct = method.param_struct %}
 {%      include "struct_declaration.tmpl" %}
 {%-     if method.response_parameters != None %}

mojo/public/tools/bindings/generators/cpp_templates/module-shared-message-ids.h.tmpl

+// 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.
+
+{%- set header_guard = "%s_SHARED_MESSAGE_IDS_H_"|format(
+        module.path|upper|replace("/","_")|replace(".","_")|
+            replace("-", "_")) %}
+
+#ifndef {{header_guard}}
+#define {{header_guard}}
+
+#include <stdint.h>
+
+{%- for namespace in namespaces_as_array %}
+namespace {{namespace}} {
+{%- endfor %}
+
+namespace internal {
+
+{% for interface in interfaces -%}
+{%-   for method in interface.methods -%}
+{%-     set method_name = "k%s_%s_Name"|format(interface.name, method.name) -%}
+{%-     if method.ordinal_comment %}
+// {{method.ordinal_comment}}
+{%-     endif %}
+constexpr uint32_t {{method_name}} = {{method.ordinal}};
+{%-  endfor %}
+{%- endfor %}
+
+}  // namespace internal
+{%- for namespace in namespaces_as_array|reverse %}
+}  // namespace {{namespace}}
+{%- endfor %}
+
+#endif  // {{header_guard}}

mojo/public/tools/bindings/generators/cpp_templates/module-shared.cc.tmpl

@@ -42,7 +42,6 @@ namespace internal {
 {#--- Interface parameter definitions #}
 {%- for interface in interfaces %}
 {%-   for method in interface.methods %}
-{%-     set method_name = "k%s_%s_Name"|format(interface.name, method.name) %}
 {%-     set struct = method.param_struct %}
 {%      include "struct_definition.tmpl" %}
 {%-     if method.response_parameters != None %}

mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl

@@ -34,6 +34,8 @@
 #include "mojo/public/cpp/bindings/lib/validation_errors.h"
 #include "mojo/public/interfaces/bindings/interface_control_messages.mojom.h"
 
+#include "{{module.path}}-shared-message-ids.h"
+
 {%- if for_blink %}
 #include "mojo/public/cpp/bindings/lib/wtf_serialization.h"
 {%- endif %}

mojo/public/tools/bindings/generators/mojom_cpp_generator.py

@@ -389,6 +389,10 @@ class Generator(generator.Generator):
   def _GenerateModuleSharedInternalHeader(self):
     return self._GetJinjaExports()
 
+  @UseJinja("module-shared-message-ids.h.tmpl")
+  def _GenerateModuleSharedMessageIdsHeader(self):
+    return self._GetJinjaExports()
+
   @UseJinja("module-shared.cc.tmpl")
   def _GenerateModuleSharedSource(self):
     return self._GetJinjaExports()
@@ -397,12 +401,16 @@ class Generator(generator.Generator):
     self.module.Stylize(generator.Stylizer())
 
     if self.generate_non_variant_code:
-      self.Write(self._GenerateModuleSharedHeader(),
-                 "%s-shared.h" % self.module.path)
-      self.Write(self._GenerateModuleSharedInternalHeader(),
-                 "%s-shared-internal.h" % self.module.path)
-      self.Write(self._GenerateModuleSharedSource(),
-                 "%s-shared.cc" % self.module.path)
+      if self.generate_message_ids:
+        self.Write(self._GenerateModuleSharedMessageIdsHeader(),
+           "%s-shared-message-ids.h" % self.module.path)
+      else:
+        self.Write(self._GenerateModuleSharedHeader(),
+                   "%s-shared.h" % self.module.path)
+        self.Write(self._GenerateModuleSharedInternalHeader(),
+                   "%s-shared-internal.h" % self.module.path)
+        self.Write(self._GenerateModuleSharedSource(),
+                   "%s-shared.cc" % self.module.path)
     else:
       suffix = "-%s" % self.variant if self.variant else ""
       self.Write(self._GenerateModuleHeader(),

mojo/public/tools/bindings/mojom.gni

@@ -23,6 +23,12 @@ declare_args() {
   # with typemapping disabled, so it is never valid to set this to |false| in
   # any Chromium build configuration.
   enable_mojom_typemapping = true
+
+  # Controls message ID scrambling behavior. If |true|, message IDs are
+  # scrambled (i.e. randomized based on the contents of //chrome/VERSION) on
+  # non-Chrome OS desktop platforms. Set to |false| to disable message ID
+  # scrambling on all platforms.
+  enable_mojom_message_id_scrambling = true
 }
 
 # NOTE: We would like to avoid scrambling message IDs where it doesn't add
@@ -39,8 +45,9 @@ declare_args() {
 # consequently also when building for NaCl toolchains.) For this reason we
 # check |target_os| explicitly, as it's consistent across all toolchains.
 enable_scrambled_message_ids =
-    is_mac || is_win || (is_linux && !is_chromeos) ||
-    ((enable_nacl || is_nacl || is_nacl_nonsfi) && target_os != "chromeos")
+    enable_mojom_message_id_scrambling &&
+    (is_mac || is_win || (is_linux && !is_chromeos) ||
+     ((enable_nacl || is_nacl || is_nacl_nonsfi) && target_os != "chromeos"))
 
 mojom_generator_root = "//mojo/public/tools/bindings"
 mojom_generator_script = "$mojom_generator_root/mojom_bindings_generator.py"
@@ -82,10 +89,23 @@ if (enable_scrambled_message_ids) {
     }
   }
 
-  mojom_generator_sources += [ mojom_message_id_salt_path ]
+  assert(mojom_message_id_salt_path != "")
+  message_scrambling_args = [
+    "--scrambled_message_id_salt_path",
+    rebase_path(mojom_message_id_salt_path, root_build_dir),
+  ]
+  message_scrambling_inputs = [ mojom_message_id_salt_path ]
+
   if (mojom_message_id_salt_suffix_path != "") {
-    mojom_generator_sources += [ mojom_message_id_salt_suffix_path ]
+    message_scrambling_args += [
+      "--scrambled_message_id_salt_path",
+      rebase_path(mojom_message_id_salt_suffix_path, root_build_dir),
+    ]
+    message_scrambling_inputs += [ mojom_message_id_salt_suffix_path ]
   }
+} else {
+  message_scrambling_args = []
+  message_scrambling_inputs = []
 }
 
 if (enable_mojom_typemapping) {
@@ -398,6 +418,8 @@ template("mojom") {
     }
   }
 
+  generator_cpp_message_ids_target_name = "${target_name}__generate_message_ids"
+
   # Generate code that is shared by different variants.
   if (defined(invoker.sources)) {
     common_generator_args = [
@@ -414,25 +436,6 @@ template("mojom") {
       rebase_path("$root_gen_dir/mojo/public/tools/bindings", root_build_dir),
     ]
 
-    if (!defined(invoker.scramble_message_ids) || invoker.scramble_message_ids) {
-      # Scramble message IDs if enabled, unless the target has explicitly opted
-      # out.
-      if (enable_scrambled_message_ids) {
-        assert(mojom_message_id_salt_path != "")
-        common_generator_args += [
-          "--scrambled_message_id_salt_path",
-          rebase_path(mojom_message_id_salt_path, root_build_dir),
-        ]
-
-        if (mojom_message_id_salt_suffix_path != "") {
-          common_generator_args += [
-            "--scrambled_message_id_salt_path",
-            rebase_path(mojom_message_id_salt_suffix_path, root_build_dir),
-          ]
-        }
-      }
-    }
-
     if (defined(invoker.disallow_native_types) &&
         invoker.disallow_native_types) {
       common_generator_args += [ "--disallow_native_types" ]
@@ -482,6 +485,32 @@ template("mojom") {
       }
     }
 
+    action_foreach(generator_cpp_message_ids_target_name) {
+      script = mojom_generator_script
+      inputs = mojom_generator_sources
+      sources = invoker.sources
+      deps = [
+        ":$parsed_target_name",
+        "//mojo/public/tools/bindings:precompile_templates",
+      ]
+      outputs = [
+        "{{source_gen_dir}}/{{source_name_part}}.mojom-shared-message-ids.h",
+      ]
+      args = common_generator_args
+      args += [
+        "--generate_non_variant_code",
+        "--generate_message_ids",
+        "-g",
+        "c++",
+      ]
+
+      if (!defined(invoker.scramble_message_ids) ||
+          invoker.scramble_message_ids) {
+        inputs += message_scrambling_inputs
+        args += message_scrambling_args
+      }
+    }
+
     generator_shared_cpp_outputs = [
       "{{source_gen_dir}}/{{source_name_part}}.mojom-shared-internal.h",
       "{{source_gen_dir}}/{{source_name_part}}.mojom-shared.cc",
@@ -513,6 +542,9 @@ template("mojom") {
         ]
       }
     }
+  } else {
+    group(generator_cpp_message_ids_target_name) {
+    }
   }
 
   shared_cpp_sources_target_name = "${target_name}_shared_cpp_sources"
@@ -595,7 +627,6 @@ template("mojom") {
     enabled_sources = []
     if (defined(invoker.sources)) {
       generator_cpp_outputs = []
-      generator_java_outputs = []
       variant_dash_suffix = ""
       if (defined(variant)) {
         variant_dash_suffix = "-${variant}"
@@ -658,10 +689,6 @@ template("mojom") {
         }
       }
 
-      if (!cpp_only) {
-        generator_java_outputs =
-            [ "{{source_gen_dir}}/{{source_name_part}}.mojom.srcjar" ]
-      }
       generator_target_name = "${target_name}${variant_suffix}__generator"
       action_foreach(generator_target_name) {
         script = mojom_generator_script
@@ -672,20 +699,12 @@ template("mojom") {
           ":$type_mappings_target_name",
           "//mojo/public/tools/bindings:precompile_templates",
         ]
-        outputs = generator_cpp_outputs + generator_java_outputs
+        outputs = generator_cpp_outputs
         args = common_generator_args
-
-        if (cpp_only) {
-          args += [
-            "-g",
-            "c++",
-          ]
-        } else {
-          args += [
-            "-g",
-            "c++,java",
-          ]
-        }
+        args += [
+          "-g",
+          "c++",
+        ]
 
         if (defined(bindings_configuration.variant)) {
           args += [
@@ -832,6 +851,7 @@ template("mojom") {
         sources = process_file_template(enabled_sources, generator_cpp_outputs)
       }
       deps = [
+        ":$generator_cpp_message_ids_target_name",
         "//mojo/public/cpp/bindings:struct_traits",
         "//mojo/public/interfaces/bindings:bindings__generator",
         "//mojo/public/interfaces/bindings:bindings_shared__generator",
@@ -917,6 +937,37 @@ template("mojom") {
     if (!cpp_only && is_android) {
       import("//build/config/android/rules.gni")
 
+      java_generator_target_name = target_name + "_java__generator"
+      if (enabled_sources != []) {
+        generator_java_outputs =
+            [ "{{source_gen_dir}}/{{source_name_part}}.mojom.srcjar" ]
+        action_foreach(java_generator_target_name) {
+          script = mojom_generator_script
+          inputs = mojom_generator_sources
+          sources = enabled_sources
+          deps = [
+            ":$parsed_target_name",
+            ":$type_mappings_target_name",
+            "//mojo/public/tools/bindings:precompile_templates",
+          ]
+          outputs = generator_java_outputs
+          args = common_generator_args
+          args += [
+            "-g",
+            "java",
+          ]
+
+          if (!defined(invoker.scramble_message_ids) ||
+              invoker.scramble_message_ids) {
+            inputs += message_scrambling_inputs
+            args += message_scrambling_args
+          }
+        }
+      } else {
+        group(java_generator_target_name) {
+        }
+      }
+
       java_srcjar_target_name = target_name + "_java_sources"
       action(java_srcjar_target_name) {
         script = "//mojo/public/tools/gn/zip.py"
@@ -938,7 +989,7 @@ template("mojom") {
         deps = []
         if (enabled_sources != []) {
           deps = [
-            ":$generator_target_name",
+            ":$java_generator_target_name",
           ]
         }
       }
@@ -989,6 +1040,12 @@ template("mojom") {
           "-g",
           "javascript",
         ]
+
+        if (!defined(invoker.scramble_message_ids) ||
+            invoker.scramble_message_ids) {
+          inputs += message_scrambling_inputs
+          args += message_scrambling_args
+        }
       }
     }
 

mojo/public/tools/bindings/mojom_bindings_generator.py

@@ -220,7 +220,8 @@ class MojomProcessor(object):
             generate_non_variant_code=args.generate_non_variant_code,
             support_lazy_serialization=args.support_lazy_serialization,
             disallow_native_types=args.disallow_native_types,
-            disallow_interfaces=args.disallow_interfaces)
+            disallow_interfaces=args.disallow_interfaces,
+            generate_message_ids=args.generate_message_ids)
         filtered_args = []
         if hasattr(generator_module, 'GENERATOR_PREFIX'):
           prefix = '--' + generator_module.GENERATOR_PREFIX + '_'
@@ -410,6 +411,11 @@ def main():
       help="Disallows interface definitions within the mojom file. It is an "
       "error to specify this flag when processing a mojom file which defines "
       "any interface.", action="store_true")
+  generate_parser.add_argument(
+      "--generate_message_ids",
+      help="Generates only the message IDs header for C++ bindings. Note that "
+      "this flag only matters if --generate_non_variant_code is also "
+      "specified.", action="store_true")
   generate_parser.set_defaults(func=_Generate)
 
   precompile_parser = subparsers.add_parser("precompile",

mojo/public/tools/bindings/pylib/mojom/generate/generator.py

@@ -164,7 +164,7 @@ class Generator(object):
                js_bindings_mode="new", export_attribute=None,
                export_header=None, generate_non_variant_code=False,
                support_lazy_serialization=False, disallow_native_types=False,
-               disallow_interfaces=False):
+               disallow_interfaces=False, generate_message_ids=False):
     self.module = module
     self.output_dir = output_dir
     self.typemap = typemap or {}
@@ -179,6 +179,7 @@ class Generator(object):
     self.support_lazy_serialization = support_lazy_serialization
     self.disallow_native_types = disallow_native_types
     self.disallow_interfaces = disallow_interfaces
+    self.generate_message_ids = generate_message_ids
 
   def Write(self, contents, filename):
     if self.output_dir is None: