[build] Add ability to define manifest package in GN

The build system will verify that the manifest packages match if you
specify it in both GN and the manifest.

Bug: 891996
Change-Id: I39c3f158d01908b971a3ef47cb90af73019284a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1667687
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671814}

build/android/gyp/compile_resources.py

@@ -153,6 +153,8 @@ def _ParseArgs(args):
       help="android:targetSdkVersion for APK.")
   input_opts.add_argument(
       '--max-sdk-version', help="android:maxSdkVersion for APK.")
+  input_opts.add_argument(
+      '--manifest-package', help='Package name of the AndroidManifest.xml.')
 
   input_opts.add_argument(
       '--locale-whitelist',
@@ -462,6 +464,7 @@ def maybe_extract_version(j):
   manifest_utils.AssertUsesSdk(manifest_node, options.min_sdk_version,
                                options.target_sdk_version,
                                options.max_sdk_version)
+  manifest_utils.AssertPackage(manifest_node, options.manifest_package)
 
   manifest_node.set('platformBuildVersionCode', version_code)
   manifest_node.set('platformBuildVersionName', version_name)

build/android/gyp/lint.py

@@ -35,6 +35,7 @@ def _OnStaleMd5(lint_path,
                 android_sdk_version,
                 srcjars,
                 min_sdk_version,
+                manifest_package,
                 resource_sources,
                 disable=None,
                 classpath=None,
@@ -196,16 +197,19 @@ def PathInDir(d, src):
     lint_manifest_path = os.path.join(project_dir, 'AndroidManifest.xml')
     shutil.copyfile(os.path.abspath(manifest_path), lint_manifest_path)
 
-    # Check that minSdkVersion is correct and add it to the manifest in case it
-    # does not exist.
+    # Check that minSdkVersion and package is correct and add it to the manifest
+    # in case it does not exist.
     doc, manifest, _ = manifest_utils.ParseManifest(lint_manifest_path)
     manifest_utils.AssertUsesSdk(manifest, min_sdk_version)
+    manifest_utils.AssertPackage(manifest, manifest_package)
     uses_sdk = manifest.find('./uses-sdk')
     if uses_sdk is None:
       uses_sdk = ElementTree.Element('uses-sdk')
       manifest.insert(0, uses_sdk)
     uses_sdk.set('{%s}minSdkVersion' % manifest_utils.ANDROID_NAMESPACE,
                  min_sdk_version)
+    if manifest_package:
+      manifest.set('package', manifest_package)
     manifest_utils.SaveManifest(doc, lint_manifest_path)
 
     cmd.append(project_dir)
@@ -340,6 +344,8 @@ def main():
       '--min-sdk-version',
       required=True,
       help='Minimal SDK version to lint against.')
+  parser.add_argument(
+      '--manifest-package', help='Package name of the AndroidManifest.xml.')
 
   args = parser.parse_args(build_utils.ExpandFileArgs(sys.argv[1:]))
 
@@ -414,6 +420,7 @@ def main():
                           args.android_sdk_version,
                           args.srcjars,
                           args.min_sdk_version,
+                          args.manifest_package,
                           resource_sources,
                           disable=disable,
                           classpath=classpath,

build/android/gyp/merge_manifest.py

@@ -29,19 +29,24 @@
 
 
 @contextlib.contextmanager
-def _ProcessManifest(manifest_path, min_sdk_version, target_sdk_version):
+def _ProcessManifest(manifest_path, min_sdk_version, target_sdk_version,
+                     manifest_package):
   """Patches an Android manifest to always include the 'tools' namespace
   declaration, as it is not propagated by the manifest merger from the SDK.
 
   See https://issuetracker.google.com/issues/63411481
   """
   doc, manifest, _ = manifest_utils.ParseManifest(manifest_path)
-  package = manifest_utils.GetPackage(manifest)
   manifest_utils.AssertUsesSdk(manifest, min_sdk_version, target_sdk_version)
+  assert manifest_utils.GetPackage(manifest) or manifest_package, \
+            'Must set manifest package in GN or in AndroidManifest.xml'
+  manifest_utils.AssertPackage(manifest, manifest_package)
+  if manifest_package:
+    manifest.set('package', manifest_package)
   tmp_prefix = os.path.basename(manifest_path)
   with tempfile.NamedTemporaryFile(prefix=tmp_prefix) as patched_manifest:
     manifest_utils.SaveManifest(doc, patched_manifest.name)
-    yield patched_manifest.name, package
+    yield patched_manifest.name, manifest_utils.GetPackage(manifest)
 
 
 def _BuildManifestMergerClasspath(build_vars):
@@ -74,6 +79,9 @@ def main(argv):
       '--target-sdk-version',
       required=True,
       help='android:targetSdkVersion for merging.')
+  parser.add_argument(
+      '--manifest-package',
+      help='Package name of the merged AndroidManifest.xml.')
   args = parser.parse_args(argv)
 
   classpath = _BuildManifestMergerClasspath(
@@ -94,7 +102,8 @@ def main(argv):
       cmd += ['--libs', ':'.join(extras)]
 
     with _ProcessManifest(args.root_manifest, args.min_sdk_version,
-                          args.target_sdk_version) as tup:
+                          args.target_sdk_version,
+                          args.manifest_package) as tup:
       root_manifest, package = tup
       cmd += [
           '--main',
@@ -116,6 +125,7 @@ def main(argv):
     _, manifest, _ = manifest_utils.ParseManifest(output.name)
     manifest_utils.AssertUsesSdk(manifest, args.min_sdk_version,
                                  args.target_sdk_version)
+    manifest_utils.AssertPackage(manifest, package)
 
   if args.depfile:
     inputs = extras + classpath.split(':')

build/android/gyp/util/manifest_utils.py

@@ -93,6 +93,20 @@ def AssertUsesSdk(manifest_node,
         (prefix, value, sdk_version))
 
 
+def AssertPackage(manifest_node, package):
+  """Asserts that manifest package has desired value.
+
+  Will only assert if both |package| is not None and the package is set in the
+  manifest.
+  """
+  package_value = GetPackage(manifest_node)
+  if package_value is None or package is None:
+    return
+  assert package_value == package, (
+      'Package in Android manifest is %s but we expect %s' % (package_value,
+                                                              package))
+
+
 def _SortAndStripElementTree(tree, reverse_toplevel=False):
   for node in tree:
     if node.text and node.text.isspace():

build/config/android/internal_rules.gni

@@ -913,6 +913,10 @@ if (enable_java_templates) {
           ]
         }
       }
+
+      if (defined(invoker.manifest_package)) {
+        args += [ "--manifest-package=${invoker.manifest_package}" ]
+      }
     }
   }
 
@@ -1760,6 +1764,10 @@ if (enable_java_templates) {
         "--min-sdk-version=${invoker.min_sdk_version}",
         "--target-sdk-version=${invoker.target_sdk_version}",
       ]
+
+      if (defined(invoker.manifest_package)) {
+        args += [ "--manifest-package=${invoker.manifest_package}" ]
+      }
     }
   }
 
@@ -2348,6 +2356,9 @@ if (enable_java_templates) {
           args += [ "--fail-if-unexpected-android-manifest" ]
         }
       }
+      if (defined(invoker.manifest_package)) {
+        args += [ "--manifest-package=${invoker.manifest_package}" ]
+      }
     }
 
     if (defined(invoker.post_process_script)) {
@@ -3423,8 +3434,13 @@ if (enable_java_templates) {
     # TODO(agrieve): Enable lint for _has_sources rather than just _java_files.
     _lint_enabled = _java_files != [] && _supports_android && _chromium_code &&
                     !disable_android_lint
-    if (!_lint_enabled && defined(invoker.min_sdk_version)) {
-      not_needed(invoker, [ "min_sdk_version" ])
+    if (!_lint_enabled) {
+      if (defined(invoker.min_sdk_version)) {
+        not_needed(invoker, [ "min_sdk_version" ])
+      }
+      if (defined(invoker.manifest_package)) {
+        not_needed(invoker, [ "manifest_package" ])
+      }
     }
     if (defined(invoker.enable_errorprone)) {
       _enable_errorprone = invoker.enable_errorprone
@@ -3496,7 +3512,11 @@ if (enable_java_templates) {
       if (_lint_enabled) {
         _android_lint_target = "${_main_target_name}__lint"
         android_lint(_android_lint_target) {
-          forward_variables_from(invoker, [ "min_sdk_version" ])
+          forward_variables_from(invoker,
+                                 [
+                                   "min_sdk_version",
+                                   "manifest_package",
+                                 ])
           if (invoker.type == "android_apk" ||
               invoker.type == "android_app_bundle_module") {
             forward_variables_from(invoker, [ "android_manifest" ])

build/config/android/rules.gni

@@ -2319,6 +2319,7 @@ if (enable_java_templates) {
         "$target_gen_dir/${_template_name}_manifest/AndroidManifest.xml"
     _merge_manifest_target = "${_template_name}__merge_manifests"
     merge_manifests(_merge_manifest_target) {
+      forward_variables_from(invoker, [ "manifest_package" ])
       input_manifest = _android_root_manifest
       output_manifest = _android_manifest
       build_config = _build_config
@@ -2377,6 +2378,7 @@ if (enable_java_templates) {
                              [
                                "aapt_locale_whitelist",
                                "app_as_shared_lib",
+                               "manifest_package",
                                "max_sdk_version",
                                "no_xml_namespaces",
                                "package_name",
@@ -2690,11 +2692,13 @@ if (enable_java_templates) {
                                "apk_under_test",
                                "base_module_target",
                                "chromium_code",
-                               "jacoco_never_instrument",
+                               "classpath_deps",
                                "enable_class_deps_output",
+                               "jacoco_never_instrument",
                                "java_files",
                                "javac_args",
                                "loadable_modules",
+                               "manifest_package",
                                "native_lib_placeholders",
                                "no_build_hooks",
                                "secondary_abi_loadable_modules",
@@ -3298,6 +3302,7 @@ if (enable_java_templates) {
                                "keystore_path",
                                "load_library_from_apk",
                                "loadable_modules",
+                               "manifest_package",
                                "max_sdk_version",
                                "min_sdk_version",
                                "native_lib_placeholders",
@@ -3415,6 +3420,7 @@ if (enable_java_templates) {
                                "jni_registration_header",
                                "jni_sources_blacklist",
                                "load_library_from_apk",
+                               "manifest_package",
                                "max_sdk_version",
                                "min_sdk_version",
                                "native_lib_version_arg",

chrome/android/BUILD.gn

@@ -43,11 +43,12 @@ if (dfmify_devtools) {
   import("//chrome/android/features/devtools/devtools_module_tmpl.gni")
 }
 
-manifest_package = "org.chromium.chrome"
-test_manifest_package = "org.chromium.chrome.tests"
+chrome_public_manifest_package = "org.chromium.chrome"
+chrome_public_test_manifest_package = "org.chromium.chrome.tests"
 
-chrome_public_jinja_variables = default_chrome_public_jinja_variables +
-                                [ "manifest_package=$manifest_package" ]
+chrome_public_jinja_variables =
+    default_chrome_public_jinja_variables +
+    [ "manifest_package=$chrome_public_manifest_package" ]
 chrome_public_android_manifest =
     "$target_gen_dir/chrome_public_apk/AndroidManifest.xml"
 chrome_modern_public_android_manifest =
@@ -706,7 +707,7 @@ junit_binary("chrome_junit_tests") {
     "//url/mojom:url_mojom_gurl_java",
   ]
 
-  package_name = manifest_package
+  package_name = chrome_public_manifest_package
 }
 
 process_version("chrome_version_constants") {
@@ -1120,7 +1121,7 @@ jinja_template_resources("chrome_public_apk_template_resources") {
     "java/res_template/xml/syncadapter.xml",
   ]
   res_dir = "java/res_template"
-  variables = [ "manifest_package=$manifest_package" ]
+  variables = [ "manifest_package=$chrome_public_manifest_package" ]
 }
 
 jinja_template_resources("chrome_test_apk_template_resources") {
@@ -1137,7 +1138,7 @@ jinja_template_resources("chrome_test_apk_template_resources") {
   deps = [
     ":chrome_public_apk_template_resources",
   ]
-  variables = [ "manifest_package=$test_manifest_package" ]
+  variables = [ "manifest_package=$chrome_public_test_manifest_package" ]
 }
 
 config("orderfile_config") {
@@ -1842,7 +1843,7 @@ jinja_template("chrome_public_test_apk_manifest") {
   output = chrome_public_test_apk_manifest
   variables = default_chrome_public_jinja_variables
   variables += [
-    "manifest_package=$test_manifest_package",
+    "manifest_package=$chrome_public_test_manifest_package",
     "min_sdk_version=19",
     "target_sdk_version=$android_sdk_version",
   ]
@@ -1872,7 +1873,7 @@ jinja_template("monochrome_public_test_ar_apk_manifest") {
               monochrome_android_manifest_jinja_variables +
               [
                 "target_sdk_version=$android_sdk_version",
-                "test_manifest_package=$test_manifest_package",
+                "test_manifest_package=$chrome_public_test_manifest_package",
                 "webview_library=libmonochrome.so",
                 "include_arcore_manifest_flag=true",
               ]
@@ -2125,7 +2126,7 @@ if (defined(expected_static_initializer_count)) {
 # bundle target template.
 if (enable_vr) {
   vr_module_tmpl("vr_public_bundle_module") {
-    manifest_package = manifest_package
+    manifest_package = chrome_public_manifest_package
     module_name = "VrPublic"
     base_module_target = ":chrome_modern_public_base_bundle_module"
     version_code = chrome_modern_version_code
@@ -2246,7 +2247,7 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
 
   if (enable_arcore) {
     ar_module_tmpl("${target_name}__ar_bundle_module") {
-      manifest_package = manifest_package
+      manifest_package = chrome_public_manifest_package
       module_name = "Ar" + _bundle_name
       base_module_target = ":$_base_module_target_name"
       native_switches = _native_switches
@@ -2257,7 +2258,7 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
 
   if (enable_vr) {
     vr_module_tmpl("${target_name}__vr_bundle_module") {
-      manifest_package = manifest_package
+      manifest_package = chrome_public_manifest_package
       module_name = "Vr" + _bundle_name
       base_module_target = ":$_base_module_target_name"
       native_switches = _native_switches
@@ -2269,7 +2270,7 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
   }
 
   tab_ui_module_tmpl("${target_name}__tab_ui_bundle_module") {
-    manifest_package = manifest_package
+    manifest_package = chrome_public_manifest_package
     module_name = "TabUiMonochromePublic"
     base_module_target = ":$_base_module_target_name"
     uncompress_shared_libraries = true
@@ -2279,7 +2280,7 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
 
   autofill_assistant_module_tmpl(
       "${target_name}__autofill_assistant_bundle_module") {
-    manifest_package = manifest_package
+    manifest_package = chrome_public_manifest_package
     module_name = "AutofillAssistant" + _bundle_name
     base_module_target = ":$_base_module_target_name"
     uncompress_shared_libraries = true
@@ -2289,7 +2290,7 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
 
   if (dfmify_devtools) {
     devtools_module_tmpl("${target_name}__devtools_bundle_module") {
-      manifest_package = manifest_package
+      manifest_package = chrome_public_manifest_package
       module_name = "Devtools" + _bundle_name
       base_module_target = ":$_base_module_target_name"
       uncompress_shared_libraries = true

chrome/android/features/ar/AndroidManifest.xml

@@ -5,8 +5,7 @@
 
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:dist="http://schemas.android.com/apk/distribution"
-    featureSplit="ar"
-    package="{{manifest_package}}">
+    featureSplit="ar">
 
     <dist:module
         dist:onDemand="true"