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

Commit 7bc38633 authored by Alex Salo's avatar Alex Salo
Browse files

Fix lock contention in AttentionDetector 

Remove the unnessary check

Bug: 169427927
Test: manually
Change-Id: Ie1b7c4744014a5d20dec89745986c7a1d65c9875
parent 82bc48af

services/core/java/com/android/server/power/AttentionDetector.java

+1 −22
Original line number Diff line number Diff line
@@ -18,14 +18,12 @@ package com.android.server.power; 


import static android.provider.DeviceConfig.NAMESPACE_ATTENTION_MANAGER_SERVICE;



import android.Manifest;

import android.app.ActivityManager;

import android.app.SynchronousUserSwitchObserver;

import android.attention.AttentionManagerInternal;

import android.attention.AttentionManagerInternal.AttentionCallbackInternal;

import android.content.ContentResolver;

import android.content.Context;

import android.content.pm.PackageManager;

import android.database.ContentObserver;

import android.os.Handler;

import android.os.PowerManager;
@@ -123,9 +121,6 @@ public class AttentionDetector { 
    @VisibleForTesting

    protected WindowManagerInternal mWindowManager;



    @VisibleForTesting

    protected PackageManager mPackageManager;



    @VisibleForTesting

    protected ContentResolver mContentResolver;
@@ -164,7 +159,6 @@ public class AttentionDetector { 
    public void systemReady(Context context) {

        mContext = context;

        updateEnabledFromSettings(context);

        mPackageManager = context.getPackageManager();

        mContentResolver = context.getContentResolver();

        mAttentionManager = LocalServices.getService(AttentionManagerInternal.class);

        mWindowManager = LocalServices.getService(WindowManagerInternal.class);
@@ -192,14 +186,11 @@ public class AttentionDetector { 
    public long updateUserActivity(long nextScreenDimming, long dimDurationMillis) {

        if (nextScreenDimming == mLastActedOnNextScreenDimming

                || !mIsSettingEnabled

                || !isAttentionServiceSupported()

                || mWindowManager.isKeyguardShowingAndNotOccluded()) {

            return nextScreenDimming;

        }



        if (!isAttentionServiceSupported() || !serviceHasSufficientPermissions()) {

            return nextScreenDimming;

        }



        final long now = SystemClock.uptimeMillis();

        final long whenToCheck = nextScreenDimming - getPreDimCheckDurationMillis();

        final long whenToStopExtending = mLastUserActivityTime + getMaxExtensionMillis();
@@ -300,18 +291,6 @@ public class AttentionDetector { 
        return mAttentionManager != null && mAttentionManager.isAttentionServiceSupported();

    }



    /**

     * Returns {@code true} if the attention service has sufficient permissions, disables the

     * depending features otherwise.

     */

    @VisibleForTesting

    boolean serviceHasSufficientPermissions() {

        final String attentionPackage = mPackageManager.getAttentionServicePackageName();

        return attentionPackage != null && mPackageManager.checkPermission(

                Manifest.permission.CAMERA, attentionPackage)

                == PackageManager.PERMISSION_GRANTED;

    }



    public void dump(PrintWriter pw) {

        pw.println("AttentionDetector:");

        pw.println(" mIsSettingEnabled=" + mIsSettingEnabled);

services/tests/servicestests/src/com/android/server/power/AttentionDetectorTest.java

+0 −17
Original line number Diff line number Diff line
@@ -39,7 +39,6 @@ import static org.mockito.Mockito.when; 


import android.attention.AttentionManagerInternal;

import android.attention.AttentionManagerInternal.AttentionCallbackInternal;

import android.content.pm.PackageManager;

import android.os.PowerManager;

import android.os.PowerManagerInternal;

import android.os.SystemClock;
@@ -63,8 +62,6 @@ import org.mockito.MockitoAnnotations; 
public class AttentionDetectorTest extends AndroidTestCase {

    private static final long DEFAULT_DIM_DURATION_MILLIS = 6_000L;



    @Mock

    private PackageManager mPackageManager;

    @Mock

    private AttentionManagerInternal mAttentionManagerInternal;

    @Mock
@@ -80,9 +77,6 @@ public class AttentionDetectorTest extends AndroidTestCase { 
    @Before

    public void setUp() {

        MockitoAnnotations.initMocks(this);

        when(mPackageManager.getAttentionServicePackageName()).thenReturn("com.google.android.as");

        when(mPackageManager.checkPermission(any(), any())).thenReturn(

                PackageManager.PERMISSION_GRANTED);

        when(mAttentionManagerInternal.checkAttention(anyLong(), any()))

                .thenReturn(true);

        when(mWindowManagerInternal.isKeyguardShowingAndNotOccluded()).thenReturn(false);
@@ -156,16 +150,6 @@ public class AttentionDetectorTest extends AndroidTestCase { 
        assertThat(mNextDimming).isEqualTo(when);

    }



    @Test

    public void testOnUserActivity_doesntCheckIfNotSufficientPermissions() {

        when(mPackageManager.checkPermission(any(), any())).thenReturn(

                PackageManager.PERMISSION_DENIED);



        long when = registerAttention();

        verify(mAttentionManagerInternal, never()).checkAttention(anyLong(), any());

        assertThat(mNextDimming).isEqualTo(when);

    }



    @Test

    public void testOnUserActivity_doesntCrashIfNoAttentionService() {

        mAttentionManagerInternal = null;
@@ -452,7 +436,6 @@ public class AttentionDetectorTest extends AndroidTestCase { 
            super(AttentionDetectorTest.this.mOnUserAttention, new Object());

            mAttentionManager = mAttentionManagerInternal;

            mWindowManager = mWindowManagerInternal;

            mPackageManager = AttentionDetectorTest.this.mPackageManager;

            mContentResolver = getContext().getContentResolver();

        }