Merge "Add an Android Lint check to prevent sendBroadcast calls on Main thread" into tm-dev
This commit is contained in:
@@ -128,6 +128,10 @@ android_library {
|
||||
kotlincflags: ["-Xjvm-default=enable"],
|
||||
|
||||
plugins: ["dagger2-compiler"],
|
||||
|
||||
lint: {
|
||||
extra_check_modules: ["SystemUILintChecker"],
|
||||
},
|
||||
}
|
||||
|
||||
filegroup {
|
||||
|
||||
52
packages/SystemUI/checks/Android.bp
Normal file
52
packages/SystemUI/checks/Android.bp
Normal file
@@ -0,0 +1,52 @@
|
||||
// Copyright (C) 2021 The Android Open Source Project
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package {
|
||||
// See: http://go/android-license-faq
|
||||
// A large-scale-change added 'default_applicable_licenses' to import
|
||||
// all of the 'license_kinds' from "frameworks_base_license"
|
||||
// to get the below license kinds:
|
||||
// SPDX-license-identifier-Apache-2.0
|
||||
default_applicable_licenses: ["frameworks_base_license"],
|
||||
}
|
||||
|
||||
java_library_host {
|
||||
name: "SystemUILintChecker",
|
||||
srcs: [
|
||||
"src/**/*.kt",
|
||||
"src/**/*.java",
|
||||
],
|
||||
plugins: ["auto_service_plugin"],
|
||||
libs: [
|
||||
"auto_service_annotations",
|
||||
"lint_api",
|
||||
],
|
||||
}
|
||||
|
||||
java_test_host {
|
||||
name: "SystemUILintCheckerTest",
|
||||
srcs: [
|
||||
"tests/**/*.kt",
|
||||
"tests/**/*.java",
|
||||
],
|
||||
static_libs: [
|
||||
"SystemUILintChecker",
|
||||
"junit",
|
||||
"lint",
|
||||
"lint_tests",
|
||||
],
|
||||
test_options: {
|
||||
unit_test: true,
|
||||
},
|
||||
}
|
||||
@@ -0,0 +1,83 @@
|
||||
/*
|
||||
* Copyright (C) 2022 The Android Open Source Project
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package com.android.internal.systemui.lint
|
||||
|
||||
import com.android.tools.lint.detector.api.Category
|
||||
import com.android.tools.lint.detector.api.Detector
|
||||
import com.android.tools.lint.detector.api.Implementation
|
||||
import com.android.tools.lint.detector.api.Issue
|
||||
import com.android.tools.lint.detector.api.JavaContext
|
||||
import com.android.tools.lint.detector.api.Scope
|
||||
import com.android.tools.lint.detector.api.Severity
|
||||
import com.android.tools.lint.detector.api.SourceCodeScanner
|
||||
import com.intellij.psi.PsiMethod
|
||||
import org.jetbrains.uast.UCallExpression
|
||||
import org.jetbrains.uast.UClass
|
||||
import org.jetbrains.uast.getParentOfType
|
||||
|
||||
/**
|
||||
* Checks if anyone is calling sendBroadcast / sendBroadcastAsUser on a Context (or subclasses) and
|
||||
* directs them to using com.android.systemui.broadcast.BroadcastSender instead.
|
||||
*/
|
||||
@Suppress("UnstableApiUsage")
|
||||
class BroadcastSentViaContextDetector : Detector(), SourceCodeScanner {
|
||||
|
||||
override fun getApplicableMethodNames(): List<String> {
|
||||
return listOf("sendBroadcast", "sendBroadcastAsUser")
|
||||
}
|
||||
|
||||
override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
|
||||
if (node.getParentOfType(UClass::class.java)?.qualifiedName ==
|
||||
"com.android.systemui.broadcast.BroadcastSender"
|
||||
) {
|
||||
// Don't warn for class we want the developers to use.
|
||||
return
|
||||
}
|
||||
|
||||
val evaulator = context.evaluator
|
||||
if (evaulator.isMemberInSubClassOf(method, "android.content.Context")) {
|
||||
context.report(
|
||||
ISSUE,
|
||||
method,
|
||||
context.getNameLocation(node),
|
||||
"Please don't call sendBroadcast/sendBroadcastAsUser directly on " +
|
||||
"Context, use com.android.systemui.broadcast.BroadcastSender instead."
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
companion object {
|
||||
@JvmField
|
||||
val ISSUE: Issue =
|
||||
Issue.create(
|
||||
id = "BroadcastSentViaContext",
|
||||
briefDescription = "Broadcast sent via Context instead of BroadcastSender.",
|
||||
explanation =
|
||||
"Broadcast was sent via " +
|
||||
"Context.sendBroadcast/Context.sendBroadcastAsUser. Please use " +
|
||||
"BroadcastSender.sendBroadcast/BroadcastSender.sendBroadcastAsUser " +
|
||||
"which will schedule dispatch of broadcasts on background thread. " +
|
||||
"Sending broadcasts on main thread causes jank due to synchronous " +
|
||||
"Binder calls.",
|
||||
category = Category.PERFORMANCE,
|
||||
priority = 8,
|
||||
severity = Severity.WARNING,
|
||||
implementation =
|
||||
Implementation(BroadcastSentViaContextDetector::class.java, Scope.JAVA_FILE_SCOPE)
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,43 @@
|
||||
/*
|
||||
* Copyright (C) 2022 The Android Open Source Project
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package com.android.internal.systemui.lint
|
||||
|
||||
import com.android.tools.lint.client.api.IssueRegistry
|
||||
import com.android.tools.lint.client.api.Vendor
|
||||
import com.android.tools.lint.detector.api.CURRENT_API
|
||||
import com.android.tools.lint.detector.api.Issue
|
||||
import com.google.auto.service.AutoService
|
||||
|
||||
@AutoService(IssueRegistry::class)
|
||||
@Suppress("UnstableApiUsage")
|
||||
class SystemUIIssueRegistry : IssueRegistry() {
|
||||
|
||||
override val issues: List<Issue>
|
||||
get() = listOf(BroadcastSentViaContextDetector.ISSUE)
|
||||
|
||||
override val api: Int
|
||||
get() = CURRENT_API
|
||||
override val minApi: Int
|
||||
get() = 8
|
||||
|
||||
override val vendor: Vendor =
|
||||
Vendor(
|
||||
vendorName = "Android",
|
||||
feedbackUrl = "http://b/issues/new?component=78010",
|
||||
contact = "jernej@google.com"
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,150 @@
|
||||
package com.android.internal.systemui.lint
|
||||
|
||||
import com.android.tools.lint.checks.infrastructure.LintDetectorTest
|
||||
import com.android.tools.lint.checks.infrastructure.TestFiles
|
||||
import com.android.tools.lint.checks.infrastructure.TestFile
|
||||
import com.android.tools.lint.checks.infrastructure.TestLintTask
|
||||
import com.android.tools.lint.detector.api.Detector
|
||||
import com.android.tools.lint.detector.api.Issue
|
||||
import org.junit.Test
|
||||
|
||||
class BroadcastSentViaContextDetectorTest : LintDetectorTest() {
|
||||
|
||||
override fun getDetector(): Detector = BroadcastSentViaContextDetector()
|
||||
override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
|
||||
|
||||
override fun getIssues(): List<Issue> = listOf(
|
||||
BroadcastSentViaContextDetector.ISSUE)
|
||||
|
||||
@Test
|
||||
fun testSendBroadcast() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.content.Context;
|
||||
|
||||
public class TestClass1 {
|
||||
public void send(Context context) {
|
||||
Intent intent = new Intent(Intent.ACTION_VIEW);
|
||||
context.sendBroadcast(intent);
|
||||
}
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs)
|
||||
.issues(BroadcastSentViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectWarningCount(1)
|
||||
.expectContains(
|
||||
"Please don't call sendBroadcast/sendBroadcastAsUser directly on " +
|
||||
"Context, use com.android.systemui.broadcast.BroadcastSender instead.")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testSendBroadcastAsUser() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.content.Context;
|
||||
import android.os.UserHandle;
|
||||
|
||||
public class TestClass1 {
|
||||
public void send(Context context) {
|
||||
Intent intent = new Intent(Intent.ACTION_VIEW);
|
||||
context.sendBroadcastAsUser(intent, UserHandle.ALL, "permission");
|
||||
}
|
||||
}
|
||||
""").indented(),
|
||||
*stubs)
|
||||
.issues(BroadcastSentViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectWarningCount(1)
|
||||
.expectContains(
|
||||
"Please don't call sendBroadcast/sendBroadcastAsUser directly on " +
|
||||
"Context, use com.android.systemui.broadcast.BroadcastSender instead.")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testSendBroadcastInActivity() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.app.Activity;
|
||||
import android.os.UserHandle;
|
||||
|
||||
public class TestClass1 {
|
||||
public void send(Activity activity) {
|
||||
Intent intent = new Intent(Intent.ACTION_VIEW);
|
||||
activity.sendBroadcastAsUser(intent, UserHandle.ALL, "permission");
|
||||
}
|
||||
|
||||
}
|
||||
""").indented(),
|
||||
*stubs)
|
||||
.issues(BroadcastSentViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectWarningCount(1)
|
||||
.expectContains(
|
||||
"Please don't call sendBroadcast/sendBroadcastAsUser directly on " +
|
||||
"Context, use com.android.systemui.broadcast.BroadcastSender instead.")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testNoopIfNoCall() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.content.Context;
|
||||
|
||||
public class TestClass1 {
|
||||
public void sendBroadcast() {
|
||||
Intent intent = new Intent(Intent.ACTION_VIEW);
|
||||
context.startActivity(intent);
|
||||
}
|
||||
}
|
||||
""").indented(),
|
||||
*stubs)
|
||||
.issues(BroadcastSentViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectClean()
|
||||
}
|
||||
|
||||
private val contextStub: TestFile = java(
|
||||
"""
|
||||
package android.content;
|
||||
import android.os.UserHandle;
|
||||
|
||||
public class Context {
|
||||
public void sendBroadcast(Intent intent) {};
|
||||
public void sendBroadcast(Intent intent, String receiverPermission) {};
|
||||
public void sendBroadcastAsUser(Intent intent, UserHandle userHandle,
|
||||
String permission) {};
|
||||
}
|
||||
"""
|
||||
)
|
||||
|
||||
private val activityStub: TestFile = java(
|
||||
"""
|
||||
package android.app;
|
||||
import android.content.Context;
|
||||
|
||||
public class Activity extends Context {}
|
||||
"""
|
||||
)
|
||||
|
||||
private val userHandleStub: TestFile = java(
|
||||
"""
|
||||
package android.os;
|
||||
|
||||
public enum UserHandle {
|
||||
ALL
|
||||
}
|
||||
"""
|
||||
)
|
||||
|
||||
private val stubs = arrayOf(contextStub, activityStub, userHandleStub)
|
||||
}
|
||||
Reference in New Issue
Block a user