From 8012e665f591ab3b82c2da875c86ecb780541fe8 Mon Sep 17 00:00:00 2001 From: Lucas Dupin Date: Wed, 10 Aug 2022 15:54:39 -0700 Subject: [PATCH] Service binding linter Introduces a linter for services being bound from sysui, asking to make sure that calls are being done from a background thread. Test: atest BindServiceViaContextDetectorTest Bug: 238923086 Change-Id: Ie97b8541b4f67b4d63809c073dfef28e69c99161 --- .../lint/BindServiceViaContextDetector.kt | 67 +++++++++ .../systemui/lint/SystemUIIssueRegistry.kt | 3 +- .../lint/BindServiceViaContextDetectorTest.kt | 140 ++++++++++++++++++ 3 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 packages/SystemUI/checks/src/com/android/internal/systemui/lint/BindServiceViaContextDetector.kt create mode 100644 packages/SystemUI/checks/tests/com/android/systemui/lint/BindServiceViaContextDetectorTest.kt diff --git a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/BindServiceViaContextDetector.kt b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/BindServiceViaContextDetector.kt new file mode 100644 index 0000000000000..925fae0ebfb4d --- /dev/null +++ b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/BindServiceViaContextDetector.kt @@ -0,0 +1,67 @@ +/* + * 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 + +@Suppress("UnstableApiUsage") +class BindServiceViaContextDetector : Detector(), SourceCodeScanner { + + override fun getApplicableMethodNames(): List { + return listOf("bindService", "bindServiceAsUser", "unbindService") + } + + override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) { + if (context.evaluator.isMemberInSubClassOf(method, "android.content.Context")) { + context.report( + ISSUE, + method, + context.getNameLocation(node), + "Binding or unbinding services are synchronous calls, please make " + + "sure you're on a @Background Executor." + ) + } + } + + companion object { + @JvmField + val ISSUE: Issue = + Issue.create( + id = "BindServiceViaContextDetector", + briefDescription = "Service bound/unbound via Context, please make sure " + + "you're on a background thread.", + explanation = + "Binding or unbinding services are synchronous calls to ActivityManager, " + + "they usually take multiple milliseconds to complete and will make" + + "the caller drop frames. Make sure you're on a @Background Executor.", + category = Category.PERFORMANCE, + priority = 8, + severity = Severity.WARNING, + implementation = + Implementation(BindServiceViaContextDetector::class.java, Scope.JAVA_FILE_SCOPE) + ) + } +} diff --git a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt index 397a110f4bc7b..226aebbd04641 100644 --- a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt +++ b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt @@ -27,7 +27,8 @@ import com.google.auto.service.AutoService class SystemUIIssueRegistry : IssueRegistry() { override val issues: List - get() = listOf(BroadcastSentViaContextDetector.ISSUE) + get() = listOf(BindServiceViaContextDetector.ISSUE, + BroadcastSentViaContextDetector.ISSUE) override val api: Int get() = CURRENT_API diff --git a/packages/SystemUI/checks/tests/com/android/systemui/lint/BindServiceViaContextDetectorTest.kt b/packages/SystemUI/checks/tests/com/android/systemui/lint/BindServiceViaContextDetectorTest.kt new file mode 100644 index 0000000000000..bf685f7c178ec --- /dev/null +++ b/packages/SystemUI/checks/tests/com/android/systemui/lint/BindServiceViaContextDetectorTest.kt @@ -0,0 +1,140 @@ +/* + * 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 BindServiceViaContextDetectorTest : LintDetectorTest() { + + override fun getDetector(): Detector = BindServiceViaContextDetector() + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + override fun getIssues(): List = listOf( + BindServiceViaContextDetector.ISSUE) + + private val explanation = "Binding or unbinding services are synchronous calls" + + @Test + fun testBindService() { + lint().files( + TestFiles.java( + """ + package test.pkg; + import android.content.Context; + + public class TestClass1 { + public void bind(Context context) { + Intent intent = new Intent(Intent.ACTION_VIEW); + context.bindService(intent, null, 0); + } + } + """ + ).indented(), + *stubs) + .issues(BindServiceViaContextDetector.ISSUE) + .run() + .expectWarningCount(1) + .expectContains(explanation) + } + + @Test + fun testBindServiceAsUser() { + lint().files( + TestFiles.java( + """ + package test.pkg; + import android.content.Context; + import android.os.UserHandle; + + public class TestClass1 { + public void bind(Context context) { + Intent intent = new Intent(Intent.ACTION_VIEW); + context.bindServiceAsUser(intent, null, 0, UserHandle.ALL); + } + } + """ + ).indented(), + *stubs) + .issues(BindServiceViaContextDetector.ISSUE) + .run() + .expectWarningCount(1) + .expectContains(explanation) + } + + @Test + fun testUnbindService() { + lint().files( + TestFiles.java( + """ + package test.pkg; + import android.content.Context; + import android.content.ServiceConnection; + + public class TestClass1 { + public void unbind(Context context, ServiceConnection connection) { + context.unbindService(connection); + } + } + """ + ).indented(), + *stubs) + .issues(BindServiceViaContextDetector.ISSUE) + .run() + .expectWarningCount(1) + .expectContains(explanation) + } + + private val contextStub: TestFile = java( + """ + package android.content; + import android.os.UserHandle; + + public class Context { + public void bindService(Intent intent) {}; + public void bindServiceAsUser(Intent intent, ServiceConnection connection, int flags, + UserHandle userHandle) {}; + public void unbindService(ServiceConnection connection) {}; + } + """ + ) + + private val serviceConnectionStub: TestFile = java( + """ + package android.content; + + public class ServiceConnection {} + """ + ) + + private val userHandleStub: TestFile = java( + """ + package android.os; + + public enum UserHandle { + ALL + } + """ + ) + + private val stubs = arrayOf(contextStub, serviceConnectionStub, userHandleStub) +}