Make sure receivers are using BroadcastDispatcher
Context#registerReceiver and friends make blocking IPCs and have been root cause of jank in the past. Let's avoid them. Test: atest RegisterReceiverViaContextDetectorTest Bug: 238923086 Change-Id: I5fe57cd81d49563d0b3ecb09de2f8d78b9aacf95
This commit is contained in:
@@ -0,0 +1,66 @@
|
||||
/*
|
||||
* 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
|
||||
|
||||
class RegisterReceiverViaContextDetector : Detector(), SourceCodeScanner {
|
||||
|
||||
override fun getApplicableMethodNames(): List<String> {
|
||||
return listOf("registerReceiver", "registerReceiverAsUser", "registerReceiverForAllUsers")
|
||||
}
|
||||
|
||||
override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
|
||||
if (context.evaluator.isMemberInSubClassOf(method, "android.content.Context")) {
|
||||
context.report(
|
||||
ISSUE,
|
||||
method,
|
||||
context.getNameLocation(node),
|
||||
"BroadcastReceivers should be registered via BroadcastDispatcher."
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
companion object {
|
||||
@JvmField
|
||||
val ISSUE: Issue =
|
||||
Issue.create(
|
||||
id = "RegisterReceiverViaContextDetector",
|
||||
briefDescription = "Broadcast registrations via Context are blocking " +
|
||||
"calls. Please use BroadcastDispatcher.",
|
||||
explanation =
|
||||
"Context#registerReceiver is a blocking call to the system server, " +
|
||||
"making it very likely that you'll drop a frame. Please use " +
|
||||
"BroadcastDispatcher instead (or move this call to a " +
|
||||
"@Background Executor.)",
|
||||
category = Category.PERFORMANCE,
|
||||
priority = 8,
|
||||
severity = Severity.WARNING,
|
||||
implementation = Implementation(RegisterReceiverViaContextDetector::class.java,
|
||||
Scope.JAVA_FILE_SCOPE)
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -28,7 +28,9 @@ class SystemUIIssueRegistry : IssueRegistry() {
|
||||
|
||||
override val issues: List<Issue>
|
||||
get() = listOf(BindServiceViaContextDetector.ISSUE,
|
||||
BroadcastSentViaContextDetector.ISSUE)
|
||||
BroadcastSentViaContextDetector.ISSUE,
|
||||
RegisterReceiverViaContextDetector.ISSUE
|
||||
)
|
||||
|
||||
override val api: Int
|
||||
get() = CURRENT_API
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
/*
|
||||
* 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.checks.infrastructure.LintDetectorTest
|
||||
import com.android.tools.lint.checks.infrastructure.TestFile
|
||||
import com.android.tools.lint.checks.infrastructure.TestFiles
|
||||
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 RegisterReceiverViaContextDetectorTest : LintDetectorTest() {
|
||||
|
||||
override fun getDetector(): Detector = RegisterReceiverViaContextDetector()
|
||||
override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
|
||||
|
||||
override fun getIssues(): List<Issue> = listOf(
|
||||
RegisterReceiverViaContextDetector.ISSUE)
|
||||
|
||||
private val explanation = "BroadcastReceivers should be registered via BroadcastDispatcher."
|
||||
|
||||
@Test
|
||||
fun testRegisterReceiver() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.content.BroadcastReceiver;
|
||||
import android.content.Context;
|
||||
import android.content.IntentFilter;
|
||||
|
||||
public class TestClass1 {
|
||||
public void bind(Context context, BroadcastReceiver receiver,
|
||||
IntentFilter filter) {
|
||||
context.registerReceiver(receiver, filter, 0);
|
||||
}
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs)
|
||||
.issues(RegisterReceiverViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectWarningCount(1)
|
||||
.expectContains(explanation)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testRegisterReceiverAsUser() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.content.BroadcastReceiver;
|
||||
import android.content.Context;
|
||||
import android.content.IntentFilter;
|
||||
import android.os.Handler;
|
||||
import android.os.UserHandle;
|
||||
|
||||
public class TestClass1 {
|
||||
public void bind(Context context, BroadcastReceiver receiver,
|
||||
IntentFilter filter, Handler handler) {
|
||||
context.registerReceiverAsUser(receiver, UserHandle.ALL, filter,
|
||||
"permission", handler);
|
||||
}
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs)
|
||||
.issues(RegisterReceiverViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectWarningCount(1)
|
||||
.expectContains(explanation)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testRegisterReceiverForAllUsers() {
|
||||
lint().files(
|
||||
TestFiles.java(
|
||||
"""
|
||||
package test.pkg;
|
||||
import android.content.BroadcastReceiver;
|
||||
import android.content.Context;
|
||||
import android.content.IntentFilter;
|
||||
import android.os.Handler;
|
||||
import android.os.UserHandle;
|
||||
|
||||
public class TestClass1 {
|
||||
public void bind(Context context, BroadcastReceiver receiver,
|
||||
IntentFilter filter, Handler handler) {
|
||||
context.registerReceiverForAllUsers(receiver, filter, "permission",
|
||||
handler);
|
||||
}
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs)
|
||||
.issues(RegisterReceiverViaContextDetector.ISSUE)
|
||||
.run()
|
||||
.expectWarningCount(1)
|
||||
.expectContains(explanation)
|
||||
}
|
||||
|
||||
private val contextStub: TestFile = java(
|
||||
"""
|
||||
package android.content;
|
||||
import android.os.Handler;
|
||||
import android.os.UserHandle;
|
||||
|
||||
public class Context {
|
||||
public void registerReceiver(BroadcastReceiver receiver, IntentFilter filter,
|
||||
int flags) {};
|
||||
public void registerReceiverAsUser(BroadcastReceiver receiver, UserHandle user,
|
||||
IntentFilter filter, String broadcastPermission, Handler scheduler) {};
|
||||
public void registerReceiverForAllUsers(BroadcastReceiver receiver, IntentFilter filter,
|
||||
String broadcastPermission, Handler scheduler) {};
|
||||
}
|
||||
"""
|
||||
)
|
||||
|
||||
private val broadcastReceiverStub: TestFile = java(
|
||||
"""
|
||||
package android.content;
|
||||
|
||||
public class BroadcastReceiver {}
|
||||
"""
|
||||
)
|
||||
|
||||
private val intentFilterStub: TestFile = java(
|
||||
"""
|
||||
package android.content;
|
||||
|
||||
public class IntentFilter {}
|
||||
"""
|
||||
)
|
||||
|
||||
private val handlerStub: TestFile = java(
|
||||
"""
|
||||
package android.os;
|
||||
|
||||
public class Handler {}
|
||||
"""
|
||||
)
|
||||
|
||||
private val userHandleStub: TestFile = java(
|
||||
"""
|
||||
package android.os;
|
||||
|
||||
public enum UserHandle {
|
||||
ALL
|
||||
}
|
||||
"""
|
||||
)
|
||||
|
||||
private val stubs = arrayOf(contextStub, broadcastReceiverStub, intentFilterStub, handlerStub,
|
||||
userHandleStub)
|
||||
}
|
||||
Reference in New Issue
Block a user