Revert "[build] Cleanup for specifying [min|target|max]SdkVersion in GN"

This reverts commit 07f70727.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 671446 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzA3ZjcwNzI3ODc5Yjg5MGJjZGI4MTgzMDM2ZTg5NWRlMzc1Y2E2OTUM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/android-archive-rel/1542

Sample Failed Step: compile

Original change's description:
> [build] Cleanup for specifying [min|target|max]SdkVersion in GN
> 
> crrev/c/1650265 enforced setting [min|target|max]SdkVersion in GN.
> However, this is to strict for some dependencies, e.g. WebRTC.
> Therefore, relax rules so that you can specify the SDK versions in both
> GN and the AndroidManifest.xml. And the build system will assert that
> these values match.
> 
> + Update some SDK versions that had been changed inadvertently in
>   crrev/c/1650265.
> 
> TBR=tedchoc@chromium.org,torne@chromium.org
> 
> Bug: 891996
> Bug: 977417
> Bug: 977328
> Change-Id: I89b944c1f73a35ba8fcdafdf4bdf62508759811e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660361
> Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> Reviewed-by: Eric Stevenson <estevenson@chromium.org>
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#671446}


Change-Id: I06e05d04cf5e8cbc4c663bd6a5ebfda1ba0b2b3c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 977328
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1672178
Cr-Commit-Position: refs/heads/master@{#671461}

android_webview/BUILD.gn

@@ -1110,6 +1110,7 @@ if (public_android_sdk) {
     deps = upstream_only_webview_deps
     apk_name = "SystemWebView"
     min_sdk_version = 21
+    target_sdk_version = 28
   }
 
   android_resource_sizes_test("resource_sizes_system_webview_apk") {
@@ -1122,7 +1123,6 @@ if (public_android_sdk) {
   system_webview_apk_tmpl("trichrome_webview_apk") {
     android_manifest = trichrome_webview_android_manifest
     android_manifest_dep = ":trichrome_webview_manifest"
-    min_sdk_version = android_sdk_version
     deps = upstream_only_webview_deps
     apk_name = "TrichromeWebView"
     use_trichrome_library = true
@@ -1137,7 +1137,6 @@ if (public_android_sdk) {
   system_webview_apk_tmpl("trichrome_webview_for_bundle_apk") {
     android_manifest = trichrome_webview_android_manifest
     android_manifest_dep = ":trichrome_webview_manifest"
-    min_sdk_version = android_sdk_version
     deps = upstream_only_webview_deps
     apk_name = "TrichromeWebViewForBundle"
     use_trichrome_library = true

android_webview/system_webview_apk_tmpl.gni

@@ -32,8 +32,6 @@ template("system_webview_apk_tmpl") {
       "//base:base_java",
     ]
 
-    target_sdk_version = android_sdk_version
-
     if (!defined(alternative_android_sdk_dep)) {
       alternative_android_sdk_dep = webview_framework_dep
     }

build/android/gyp/compile_resources.py

@@ -459,9 +459,16 @@ def _FixManifest(options, temp_dir):
   doc, manifest_node, app_node = manifest_utils.ParseManifest(
       options.android_manifest)
 
-  manifest_utils.AssertUsesSdk(manifest_node, options.min_sdk_version,
-                               options.target_sdk_version,
-                               options.max_sdk_version)
+  manifest_utils.AssertNoUsesSdk(manifest_node)
+
+  uses_sdk_attributes = [
+      ('android:minSdkVersion', options.min_sdk_version),
+      ('android:targetSdkVersion', options.target_sdk_version),
+  ]
+  if options.max_sdk_version:
+    uses_sdk_attributes += [('android:maxSdkVersion', options.max_sdk_version)]
+  uses_sdk_node = manifest_utils.MakeElement('uses-sdk', uses_sdk_attributes)
+  manifest_node.insert(0, uses_sdk_node)
 
   manifest_node.set('platformBuildVersionCode', version_code)
   manifest_node.set('platformBuildVersionName', version_name)
@@ -720,7 +727,6 @@ def _PackageApk(options, build):
       'link',
       '--auto-add-overlay',
       '--no-version-vectors',
-      # Set SDK versions in case they are not set in the Android manifest.
       '--min-sdk-version',
       options.min_sdk_version,
       '--target-sdk-version',

build/android/gyp/lint.py

@@ -15,7 +15,6 @@ import shutil
 import sys
 import traceback
 from xml.dom import minidom
-from xml.etree import ElementTree
 
 from util import build_utils
 from util import manifest_utils
@@ -196,16 +195,19 @@ def _OnStaleMd5(lint_path,
     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.
+    # Add the minSdkVersion to the manifest.
     doc, manifest, _ = manifest_utils.ParseManifest(lint_manifest_path)
-    manifest_utils.AssertUsesSdk(manifest, min_sdk_version)
+    # TODO(crbug.com/891996): Only activate once downstream has been updated.
+    # manifest_utils.AssertNoUsesSdk(manifest)
+    if min_sdk_version:
+      # TODO(crbug.com/891996): Don't remove uses-sdk once downstream has been
+      # updated.
       uses_sdk = manifest.find('./uses-sdk')
-    if uses_sdk is None:
-      uses_sdk = ElementTree.Element('uses-sdk')
+      if uses_sdk is not None:
+        manifest.remove(uses_sdk)
+      uses_sdk = manifest_utils.MakeElement(
+          'uses-sdk', [('android:minSdkVersion', min_sdk_version)])
       manifest.insert(0, uses_sdk)
-    uses_sdk.set('{%s}minSdkVersion' % manifest_utils.ANDROID_NAMESPACE,
-                 min_sdk_version)
       manifest_utils.SaveManifest(doc, lint_manifest_path)
 
     cmd.append(project_dir)
@@ -304,6 +306,9 @@ def main():
   parser.add_argument('--android-sdk-version',
                       help='Version (API level) of the Android SDK used for '
                            'building.')
+  parser.add_argument('--create-cache', action='store_true',
+                      help='Mark the lint cache file as an output rather than '
+                      'an input.')
   parser.add_argument('--can-fail-build', action='store_true',
                       help='If set, script will exit with nonzero exit status'
                            ' if lint errors are present')
@@ -337,9 +342,7 @@ def main():
   parser.add_argument('--srcjars',
                       help='GN list of included srcjars.')
   parser.add_argument(
-      '--min-sdk-version',
-      required=True,
-      help='Minimal SDK version to lint against.')
+      '--min-sdk-version', help='Minimal SDK version to lint against.')
 
   args = parser.parse_args(build_utils.ExpandFileArgs(sys.argv[1:]))
 

build/android/gyp/merge_manifest.py

@@ -29,7 +29,7 @@ _MANIFEST_MERGER_JARS = [
 
 
 @contextlib.contextmanager
-def _ProcessManifest(manifest_path, min_sdk_version, target_sdk_version):
+def _ProcessManifest(manifest_path, allow_uses_sdk):
   """Patches an Android manifest to always include the 'tools' namespace
   declaration, as it is not propagated by the manifest merger from the SDK.
 
@@ -37,7 +37,8 @@ def _ProcessManifest(manifest_path, min_sdk_version, target_sdk_version):
   """
   doc, manifest, _ = manifest_utils.ParseManifest(manifest_path)
   package = manifest_utils.GetPackage(manifest)
-  manifest_utils.AssertUsesSdk(manifest, min_sdk_version, target_sdk_version)
+  if not allow_uses_sdk:
+    manifest_utils.AssertNoUsesSdk(manifest)
   tmp_prefix = os.path.basename(manifest_path)
   with tempfile.NamedTemporaryFile(prefix=tmp_prefix) as patched_manifest:
     manifest_utils.SaveManifest(doc, patched_manifest.name)
@@ -74,6 +75,11 @@ def main(argv):
       '--target-sdk-version',
       required=True,
       help='android:targetSdkVersion for merging.')
+  parser.add_argument(
+      '--allow-uses-sdk',
+      action='store_true',
+      help='Use only for third party code. '
+      'Don\'t fail if input manifest contains a <uses-sdk> element.')
   args = parser.parse_args(argv)
 
   classpath = _BuildManifestMergerClasspath(
@@ -93,8 +99,7 @@ def main(argv):
     if extras:
       cmd += ['--libs', ':'.join(extras)]
 
-    with _ProcessManifest(args.root_manifest, args.min_sdk_version,
-                          args.target_sdk_version) as tup:
+    with _ProcessManifest(args.root_manifest, args.allow_uses_sdk) as tup:
       root_manifest, package = tup
       cmd += [
           '--main',
@@ -112,10 +117,18 @@ def main(argv):
         fail_func=lambda returncode, stderr: returncode != 0 or
           build_utils.IsTimeStale(output.name, [root_manifest] + extras))
 
-    # Check for correct output.
-    _, manifest, _ = manifest_utils.ParseManifest(output.name)
-    manifest_utils.AssertUsesSdk(manifest, args.min_sdk_version,
-                                 args.target_sdk_version)
+    # Subsequent build system steps expect uses-sdk tag does not exist.
+    # Therefore, check it has the expected attribute values and remove it.
+    doc, manifest, _ = manifest_utils.ParseManifest(output.name)
+    uses_sdk = manifest.find('./uses-sdk')
+    assert uses_sdk.get(
+        '{%s}minSdkVersion' %
+        manifest_utils.ANDROID_NAMESPACE) == args.min_sdk_version
+    assert uses_sdk.get(
+        '{%s}targetSdkVersion' %
+        manifest_utils.ANDROID_NAMESPACE) == args.target_sdk_version
+    manifest.remove(uses_sdk)
+    manifest_utils.SaveManifest(doc, output.name)
 
   if args.depfile:
     inputs = extras + classpath.split(':')

build/android/gyp/util/manifest_utils.py

@@ -56,6 +56,18 @@ def ParseManifest(path):
   return doc, manifest_node, app_node
 
 
+def MakeElement(name, attributes=None):
+  _RegisterElementTreeNamespaces()
+  element = ElementTree.Element(name)
+  if attributes:
+    for attribute_name, attribute_val in attributes:
+      if attribute_name.startswith('android:'):
+        attribute_name = '{%s}%s' % (
+            ANDROID_NAMESPACE, attribute_name[attribute_name.find(':') + 1:])
+      element.set(attribute_name, attribute_val)
+  return element
+
+
 def SaveManifest(doc, path):
   with build_utils.AtomicOutput(path) as f:
     f.write(ElementTree.tostring(doc.getroot(), encoding='UTF-8'))
@@ -65,27 +77,9 @@ def GetPackage(manifest_node):
   return manifest_node.get('package')
 
 
-def AssertUsesSdk(manifest_node,
-                  min_sdk_version,
-                  target_sdk_version=None,
-                  max_sdk_version=None):
-  """Asserts values of attributes of <uses-sdk> element.
-
-  Will only assert if both the passed value is not None and the value of
-  attribute exist.
-  """
-  uses_sdk_node = manifest_node.find('./uses-sdk')
-  if uses_sdk_node is None:
-    return
-  for prefix, sdk_version in (('min', min_sdk_version), ('target',
-                                                         target_sdk_version),
-                              ('max', max_sdk_version)):
-    value = uses_sdk_node.get('{%s}%sSdkVersion' % (ANDROID_NAMESPACE, prefix))
-    if not value or not sdk_version:
-      continue
-    assert value == sdk_version, (
-        '%sSdkVersion in Android manifest is %s but we expect %s' %
-        (prefix, value, sdk_version))
+def AssertNoUsesSdk(manifest_node):
+  """Asserts that manifest has no <uses-sdk> element."""
+  assert manifest_node.find('./uses-sdk') is None, 'Must define uses-sdk in GN'
 
 
 def _SortAndStripElementTree(tree, reverse_toplevel=False):

build/config/android/internal_rules.gni

@@ -810,10 +810,6 @@ if (enable_java_templates) {
   android_default_aapt_path = "$android_sdk_build_tools/aapt"
 
   template("android_lint") {
-    _min_sdk_version = 19
-    if (defined(invoker.min_sdk_version)) {
-      _min_sdk_version = invoker.min_sdk_version
-    }
     action_with_pydeps(target_name) {
       forward_variables_from(invoker,
                              [
@@ -873,7 +869,6 @@ if (enable_java_templates) {
         "--result-path",
         rebase_path(_result_path, root_build_dir),
         "--include-unexpected-failures",
-        "--min-sdk-version=$_min_sdk_version",
       ]
       if (defined(invoker.android_manifest)) {
         inputs += [ invoker.android_manifest ]
@@ -888,7 +883,10 @@ if (enable_java_templates) {
       }
 
       if (defined(invoker.create_cache) && invoker.create_cache) {
-        args += [ "--silent" ]
+        args += [
+          "--create-cache",
+          "--silent",
+        ]
       } else {
         inputs += invoker.java_files
         inputs += [ invoker.build_config ]
@@ -913,6 +911,10 @@ if (enable_java_templates) {
           ]
         }
       }
+
+      if (defined(invoker.min_sdk_version)) {
+        args += [ "--min-sdk-version=${invoker.min_sdk_version}" ]
+      }
     }
   }
 
@@ -1760,6 +1762,12 @@ if (enable_java_templates) {
         "--min-sdk-version=${invoker.min_sdk_version}",
         "--target-sdk-version=${invoker.target_sdk_version}",
       ]
+
+      # TODO(crbug.com/891996): Only activate for non-Chromium code once
+      # downstream has been updated.
+      if ((defined(invoker.chromium_code) && !invoker.chromium_code) || true) {
+        args += [ "--allow-uses-sdk" ]
+      }
     }
   }
 

build/config/android/rules.gni

@@ -2321,6 +2321,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, [ "chromium_code" ])
       input_manifest = _android_root_manifest
       output_manifest = _android_manifest
       build_config = _build_config

chrome/android/BUILD.gn

@@ -1763,6 +1763,9 @@ if (public_android_sdk) {
     android_manifest = trichrome_library_android_manifest
     android_manifest_dep = ":trichrome_library_android_manifest"
 
+    # TODO(torne): make minsdk=Q once we no longer build hacky P version
+    min_sdk_version = android_sdk_version
+    target_sdk_version = android_sdk_version
     if (trichrome_synchronized_proguard) {
       static_library_dependent_targets = [
         {
@@ -1788,6 +1791,9 @@ if (public_android_sdk) {
     android_manifest = trichrome_library_android_manifest
     android_manifest_dep = ":trichrome_library_android_manifest"
 
+    # TODO(torne): make minsdk=Q once we no longer build hacky P version
+    min_sdk_version = android_sdk_version
+    target_sdk_version = android_sdk_version
     if (trichrome_synchronized_proguard) {
       static_library_dependent_targets = [
         {
@@ -2072,14 +2078,13 @@ if (enable_arcore) {
   }
 }
 
-# Chrome smoke test is a minimal test to ensure Chrome is not DOA.  It is
-# designed to be runnable against uninstrumented Chrome apks.
+# Chrome smoke test is a minimal test to ensure Chrome is not DOA.  It is deigned to
+# be runnable against uninstrumented Chrome apks.
 instrumentation_test_apk("chrome_smoke_test") {
   apk_name = "ChromeSmokeTest"
   apk_under_test = "//chrome/android:chrome_public_apk"
   android_manifest =
       "javatests/src/org/chromium/chrome/test/smoke/AndroidManifest.xml"
-  target_sdk_version = 28
   testonly = true
   java_files =
       [ "javatests/src/org/chromium/chrome/test/smoke/ChromeSmokeTest.java" ]

chrome/android/chrome_public_apk_tmpl.gni

@@ -136,8 +136,7 @@ template("chrome_public_common_apk_or_module_tmpl") {
     forward_variables_from(invoker, "*")
 
     if (_is_trichrome) {
-      # TODO(torne): make minsdk=Q once we no longer build hacky P version
-      min_sdk_version = android_sdk_version
+      min_sdk_version = 28
     } else if (_is_monochrome) {
       min_sdk_version = 24
     } else if (_is_modern) {

chrome/android/javatests/src/org/chromium/chrome/test/smoke/AndroidManifest.xml

@@ -8,6 +8,8 @@
       xmlns:android="http://schemas.android.com/apk/res/android"
       package="org.chromium.chrome.test.smoke">
 
+    <uses-sdk android:minSdkVersion="19" android:targetSdkVersion="28" />
+
     <uses-permission android:name="android.permission.RUN_INSTRUMENTATION" />
     <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
 

chrome/android/trichrome.gni

@@ -65,10 +65,7 @@ template("trichrome_library_apk_tmpl") {
     uncompress_dex = use_uncompressed_dex
     version_name = chrome_version_name
     version_code = trichrome_version_code
-
-    # TODO(torne): make minsdk=Q once we no longer build hacky P version
-    min_sdk_version = android_sdk_version
-    target_sdk_version = android_sdk_version
+    min_sdk_version = 28
     if (!enable_trichrome_synchronized_proguard) {
       generate_buildconfig_java = false
     } else {

chrome/android/webapk/shell_apk/BUILD.gn

@@ -75,7 +75,6 @@ template("webapk_java") {
       "//chrome/android/webapk/libs/common:common_java",
       "//chrome/android/webapk/libs/common:splash_java",
     ]
-    min_sdk_version = 16
   }
 }
 
@@ -92,7 +91,6 @@ template("webapk_with_service_java") {
       "src/org/chromium/webapk/shell_apk/WebApkServiceImplWrapper.java",
     ]
     deps += [ ":compiled_in_runtime_library_java" ]
-    min_sdk_version = 16
   }
 }
 

remoting/android/BUILD.gn

@@ -82,7 +82,7 @@ remoting_localize("remoting_test_apk_manifest") {
 instrumentation_test_apk("remoting_test_apk") {
   android_manifest = "$root_gen_dir/remoting/android_test/AndroidManifest.xml"
   android_manifest_dep = ":remoting_test_apk_manifest"
-  target_sdk_version = 23
+  target_sdk_version = 28
   apk_name = "ChromotingTest"
   apk_under_test = ":remoting_apk"
   java_files = [

remoting/android/remoting_apk_tmpl.gni

@@ -14,7 +14,6 @@ template("remoting_apk_tmpl") {
 
     android_manifest = "$root_gen_dir/remoting/android/AndroidManifest.xml"
     android_manifest_dep = "//remoting/android:remoting_apk_manifest"
-    target_sdk_version = 28
     shared_libraries = [ "//remoting/client/jni:remoting_client_jni" ]
     version_name = chrome_version_name
     version_code = chrome_version_code

third_party/custom_tabs_client/BUILD.gn

@@ -27,8 +27,6 @@ android_apk("custom_tabs_client_example_apk") {
     "src/Application/src/main/java/org/chromium/customtabsclient/SessionHelper.java",
   ]
   android_manifest = "src/Application/src/main/AndroidManifest.xml"
-  min_sdk_version = 16
-  target_sdk_version = 21
   apk_name = "CustomTabsClientExample"
   deps = [
     ":chrome_tabs_client_example_apk_resources",