Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8012e665 authored by Lucas Dupin's avatar Lucas Dupin
Browse files

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
parent ef8f854a
Loading
Loading
Loading
Loading
+67 −0
Original line number Diff line number Diff line
/*
 * 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<String> {
        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)
            )
    }
}
+2 −1
Original line number Diff line number Diff line
@@ -27,7 +27,8 @@ import com.google.auto.service.AutoService
class SystemUIIssueRegistry : IssueRegistry() {

    override val issues: List<Issue>
        get() = listOf(BroadcastSentViaContextDetector.ISSUE)
        get() = listOf(BindServiceViaContextDetector.ISSUE,
                BroadcastSentViaContextDetector.ISSUE)

    override val api: Int
        get() = CURRENT_API
+140 −0
Original line number Diff line number Diff line
/*
 * 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<Issue> = 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)
}