chromium/src
0
Fork 0
Code Activity

android: Fix lint for testonly apks

Browse Source 
Previously when chrome_public_test_apk enabled lint, some testonly lint
warnings were disabled for all testonly targets because they did not
apply to chrome_public_test_apk. It turns out that Cronet and WebView
shell both were testonly targets but needed test checks.

This CL turns these checks back on by removing all the ones previously
disabled for testonly targets. This requires re-baselining with the
script:
$ third_party/android_build_tools/lint/rebuild_baselines.py

Bug: 333130242
Change-Id: Ie9c6ab5a7dbe51b8b4905c8e83487cf8c05db023
Fixed: 333130242
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5582415
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1309391}
This commit is contained in:
Peter Wen
2024-06-03 16:50:59 +00:00
committed by Chromium LUCI CQ
parent 7c55febea8
commit 4e52fc5cf3
7 changed files with 1468 additions and 105 deletions
android_webview/tools/system_webview_shell
lint-baseline.xmllint-suppressions.xml
build
android
gyp
lint.py
config
android
internal_rules.gni
chrome/android/expectations
lint-baseline.xml
chromecast/android
lint-baseline.xml
components/cronet/android
lint-baseline.xml

68
android_webview/tools/system_webview_shell/lint-baseline.xml

@ -1,5 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<issues format="6" by="lint 8.6.0-alpha02" type="baseline" client="" dependencies="true" name="" variant="all" version="8.6.0-alpha02">
<issues format="6" by="lint 8.6.0-alpha03" type="baseline" client="" dependencies="true" name="" variant="all" version="8.6.0-alpha03">
<issue
id="NewApi"
message="Call requires API level 23 (current min is 21): `canGrant`"
errorLine1=" if (!canGrant(webkitPermission)) {"
errorLine2=" ~~~~~~~~">
<location
file="../../android_webview/tools/system_webview_shell/apk/src/org/chromium/webview_shell/WebViewBrowserFragment.java"
line="512"
column="18"/>
</issue>
<issue
id="RequiresFeature"
@ -56,50 +67,6 @@
column="53"/>
</issue>
<issue
id="AppBundleLocaleChanges"
message="Found dynamic locale changes, but did not find corresponding Play Core library calls for downloading languages and splitting by language is not disabled in the `bundle` configuration"
errorLine1=" config.setLocale(Locale.forLanguageTag(languageTag));"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../base/android/java/src/org/chromium/base/LocaleUtils.java"
line="237"
column="13"/>
</issue>
<issue
id="QueryPermissionsNeeded"
message="Consider adding a `&lt;queries>` declaration to your manifest when calling this method; see https://g.co/dev/packagevisibility for details"
errorLine1=" return pm.queryIntentActivities(intent, flags);"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../base/android/java/src/org/chromium/base/PackageManagerUtils.java"
line="64"
column="23"/>
</issue>
<issue
id="UnspecifiedRegisterReceiverFlag"
message="`receiver` is missing `RECEIVER_EXPORTED` or `RECEIVER_NOT_EXPORTED` flag for unprotected broadcasts registered for an IntentFilter that cannot be inspected by lint"
errorLine1=" return context.registerReceiver(receiver, filter, permission, scheduler);"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../base/android/java/src/org/chromium/base/ContextUtils.java"
line="319"
column="20"/>
</issue>
<issue
id="UnclosedTrace"
message="The `beginSection()` call is not always closed with a matching `endSection()` because the code in between may return early"
errorLine1=" Trace.beginSection(name);"
errorLine2=" ~~~~~~~~~~~~">
<location
file="../../base/android/java/src/org/chromium/base/metrics/ScopedSysTraceEvent.java"
line="33"
column="15"/>
</issue>
<issue
id="UnclosedTrace"
message="The `beginSection()` call is not always closed with a matching `endSection()` because the code in between may return early"
@ -133,17 +100,6 @@
column="15"/>
</issue>
<issue
id="UseCompatLoadingForDrawables"
message="Use `ResourcesCompat.getDrawable()`"
errorLine1=" return res.getDrawable(id, null);"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java"
line="193"
column="24"/>
</issue>
<issue
id="HardcodedDebugMode"
message="Avoid hardcoding the debug mode; leaving it out allows debug and release builds to automatically assign one"

7
android_webview/tools/system_webview_shell/lint-suppressions.xml

@ -4,9 +4,12 @@ Please refer to the lint doc for how to use this file and what should go in it:
https://chromium.googlesource.com/chromium/src/+/main/build/android/docs/lint.md
-->
<lint>
<issue id="SetJavaScriptEnabled" severity="ignore"/>
<issue id="all">
<ignore regexp="../../../base/"/>
</issue>
<issue id="AcceptsUserCertificates" severity="ignore"/>
<issue id="ForegroundServiceType" severity="ignore" />
<issue id="InsecureBaseConfiguration" severity="ignore"/>
<issue id="ScopedStorage" severity="ignore"/>
<issue id="ForegroundServiceType" severity="ignore" />
<issue id="SetJavaScriptEnabled" severity="ignore"/>
</lint>

32
build/android/gyp/lint.py

@ -42,28 +42,6 @@ _DISABLED_ALWAYS = [
"UnusedAttribute", # Chromium apks have various minSdkVersion values.
]
# These checks are not useful for test targets and adds an unnecessary burden
# to suppress them.
_DISABLED_FOR_TESTS = [
# We should not require test strings.xml files to explicitly add
# translatable=false since they are not translated and not used in
# production.
"MissingTranslation",
# Test strings.xml files often have simple names and are not translatable,
# so it may conflict with a production string and cause this error.
"Untranslatable",
# Test targets often use the same strings target and resources target as the
# production targets but may not use all of them.
"UnusedResources",
# TODO(wnwen): Turn this back on since to crash it would require running on
# a device with all the various minSdkVersions.
# Real NewApi violations crash the app, so the only ones that lint catches
# but tests still succeed are false positives.
"NewApi",
# Tests should be allowed to access these methods/classes.
"VisibleForTests",
]
_RES_ZIP_DIR = 'RESZIPS'
_SRCJAR_DIR = 'SRCJARS'
_AAR_DIR = 'AARS'
@ -220,7 +198,6 @@ def _RunLint(custom_lint_jar_path,
android_sdk_root,
lint_gen_dir,
baseline,
testonly_target=False,
warnings_as_errors=False):
logging.info('Lint starting')
if not cache_dir:
@ -271,9 +248,6 @@ def _RunLint(custom_lint_jar_path,
','.join(_DISABLED_ALWAYS),
]
if testonly_target:
cmd.extend(['--disable', ','.join(_DISABLED_FOR_TESTS)])
if not manifest_path:
manifest_path = os.path.join(build_utils.DIR_SOURCE_ROOT, 'build',
'android', 'AndroidManifest.xml')
@ -437,11 +411,6 @@ def _ParseArgs(argv):
parser.add_argument('--android-sdk-root',
required=True,
help='Lint needs an explicit path to the android sdk.')
parser.add_argument('--testonly',
action='store_true',
help='If set, some checks like UnusedResources will be '
'disabled since they are not helpful for test '
'targets.')
parser.add_argument('--create-cache',
action='store_true',
help='Whether this invocation is just warming the cache.')
@ -541,7 +510,6 @@ def main():
args.android_sdk_root,
args.lint_gen_dir,
args.baseline,
testonly_target=args.testonly,
warnings_as_errors=args.warnings_as_errors)
logging.info('Creating stamp file')
build_utils.Touch(args.stamp)

5
build/config/android/internal_rules.gni

@ -1147,11 +1147,6 @@ if (enable_java_templates) {
]
}
if (defined(testonly) && testonly) {
# Allows us to ignore unnecessary checks when linting test targets.
args += [ "--testonly" ]
}
if (defined(invoker.manifest_package)) {
args += [ "--manifest-package=${invoker.manifest_package}" ]
}

16
chrome/android/expectations/lint-baseline.xml

@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<issues format="6" by="lint 8.6.0-alpha02" type="baseline" client="" dependencies="true" name="" variant="all" version="8.6.0-alpha02">
<issues format="6" by="lint 8.6.0-alpha03" type="baseline" client="" dependencies="true" name="" variant="all" version="8.6.0-alpha03">
<issue
id="WrongCommentType"
@ -217,7 +217,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../media/base/android/java/src/org/chromium/media/MediaDrmBridge.java"
line="1506"
line="1519"
column="8"/>
</issue>
@ -250,7 +250,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java"
line="1277"
line="1256"
column="43"/>
</issue>
@ -338,7 +338,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~">
<location
file="../../content/public/android/java/src/org/chromium/content_public/browser/WebContents.java"
line="123"
line="120"
column="27"/>
</issue>
@ -1009,7 +1009,7 @@
errorLine2=" ~~~~~~~~~~~">
<location
file="../../chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java"
line="432"
line="532"
column="72"/>
</issue>
@ -1603,7 +1603,7 @@
errorLine2=" ^">
<location
file="gen/chrome/android/monochrome_public_bundle__lint/AndroidManifest.xml"
line="615"
line="624"
column="5"/>
</issue>
@ -1647,7 +1647,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="gen/chrome/android/monochrome_public_bundle__lint/AndroidManifest.xml"
line="2317"
line="2325"
column="399"/>
</issue>
@ -1658,7 +1658,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="gen/chrome/android/monochrome_public_bundle__lint/AndroidManifest.xml"
line="2317"
line="2325"
column="434"/>
</issue>

2
chromecast/android/lint-baseline.xml

@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<issues format="6" by="lint 8.6.0-alpha02" type="baseline" client="" dependencies="true" name="" variant="all" version="8.6.0-alpha02">
<issues format="6" by="lint 8.6.0-alpha03" type="baseline" client="" dependencies="true" name="" variant="all" version="8.6.0-alpha03">
<issue
id="PictureInPictureIssue"

1443
components/cronet/android/lint-baseline.xml

File diff suppressed because it is too large Load Diff