Add an Android Lint check to prevent sendBroadcast calls on Main thread

This lint check verifies that BroadcastSender is always used in SystemUI for sending broadcasts. It should prevent main thread jank due to broadcast calls.

Bug: 223606115
Test: Included unit test for the linter.
Change-Id: I16b8b3471389ff2c59053c686c94cb9cb694ec42
This commit is contained in:
Jernej Virag
2022-03-10 17:52:11 +00:00
parent 923eb82c53
commit bb4ff3f645
5 changed files with 332 additions and 0 deletions

View File

@@ -128,6 +128,10 @@ android_library {
kotlincflags: ["-Xjvm-default=enable"],
plugins: ["dagger2-compiler"],
lint: {
extra_check_modules: ["SystemUILintChecker"],
},
}
filegroup {

View 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,
},
}

View File

@@ -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)
)
}
}

View File

@@ -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"
)
}

View File

@@ -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)
}