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

Commit a83d7be6 authored by Yi Jiang's avatar Yi Jiang
Browse files

Removes circular dependency in inner classes. 

There is a dependency loop in AttentionManagerService:
mUserStates->mAttentionCheck->mIAttentionCallback->mUserStates. This may
leads to unsafe states.

Test: atest AttentionManagerServiceTest
Bug: 149931757

Change-Id: I775c18f45a764c5bbe71bd0d34d03888d91bab3b
parent ba0f563d

services/core/java/com/android/server/attention/AttentionManagerService.java

+47 −46
Original line number Diff line number Diff line
@@ -90,10 +90,12 @@ public class AttentionManagerService extends SystemService { 
     * DeviceConfig flag name, describes how much time we consider a result fresh; if the check

     * attention called within that period - cached value will be returned.

     */

    @VisibleForTesting static final String KEY_STALE_AFTER_MILLIS = "stale_after_millis";

    @VisibleForTesting

    static final String KEY_STALE_AFTER_MILLIS = "stale_after_millis";



    /** Default value in absence of {@link DeviceConfig} override. */

    @VisibleForTesting static final long DEFAULT_STALE_AFTER_MILLIS = 1_000;

    @VisibleForTesting

    static final long DEFAULT_STALE_AFTER_MILLIS = 1_000;



    /** The size of the buffer that stores recent attention check results. */

    @VisibleForTesting
@@ -237,7 +239,7 @@ public class AttentionManagerService extends SystemService { 
                }

            }



            userState.mCurrentAttentionCheck = createAttentionCheck(callbackInternal, userState);

            userState.mCurrentAttentionCheck = new AttentionCheck(callbackInternal, userState);



            if (userState.mService != null) {

                try {
@@ -255,46 +257,6 @@ public class AttentionManagerService extends SystemService { 
        }

    }



    private AttentionCheck createAttentionCheck(AttentionCallbackInternal callbackInternal,

            UserState userState) {

        final IAttentionCallback iAttentionCallback = new IAttentionCallback.Stub() {

            @Override

            public void onSuccess(@AttentionSuccessCodes int result, long timestamp) {

                if (userState.mCurrentAttentionCheck.mIsFulfilled) {

                    return;

                }

                userState.mCurrentAttentionCheck.mIsFulfilled = true;

                callbackInternal.onSuccess(result, timestamp);

                logStats(result);

                synchronized (mLock) {

                    if (userState.mAttentionCheckCacheBuffer == null) {

                        userState.mAttentionCheckCacheBuffer = new AttentionCheckCacheBuffer();

                    }

                    userState.mAttentionCheckCacheBuffer.add(

                            new AttentionCheckCache(SystemClock.uptimeMillis(), result, timestamp));

                }

            }



            @Override

            public void onFailure(@AttentionFailureCodes int error) {

                if (userState.mCurrentAttentionCheck.mIsFulfilled) {

                    return;

                }

                userState.mCurrentAttentionCheck.mIsFulfilled = true;

                callbackInternal.onFailure(error);

                logStats(error);

            }



            private void logStats(int result) {

                FrameworkStatsLog.write(

                        FrameworkStatsLog.ATTENTION_MANAGER_SERVICE_RESULT_REPORTED,

                        result);

            }

        };



        return new AttentionCheck(callbackInternal, iAttentionCallback);

    }



    /** Cancels the specified attention check. */

    @VisibleForTesting

    void cancelAttentionCheck(AttentionCallbackInternal callbackInternal) {
@@ -505,13 +467,42 @@ public class AttentionManagerService extends SystemService { 
    static final class AttentionCheck {

        private final AttentionCallbackInternal mCallbackInternal;

        private final IAttentionCallback mIAttentionCallback;



        private boolean mIsDispatched;

        private boolean mIsFulfilled;



        AttentionCheck(AttentionCallbackInternal callbackInternal,

                IAttentionCallback iAttentionCallback) {

        AttentionCheck(AttentionCallbackInternal callbackInternal, UserState userState) {

            mCallbackInternal = callbackInternal;

            mIAttentionCallback = iAttentionCallback;

            mIAttentionCallback = new IAttentionCallback.Stub() {

                @Override

                public void onSuccess(@AttentionSuccessCodes int result, long timestamp) {

                    if (mIsFulfilled) {

                        return;

                    }

                    mIsFulfilled = true;

                    callbackInternal.onSuccess(result, timestamp);

                    logStats(result);

                    userState.appendResultToAttentionCacheBuffer(

                            new AttentionCheckCache(SystemClock.uptimeMillis(), result,

                                    timestamp));

                }



                @Override

                public void onFailure(@AttentionFailureCodes int error) {

                    if (mIsFulfilled) {

                        return;

                    }

                    mIsFulfilled = true;

                    callbackInternal.onFailure(error);

                    logStats(error);

                }



                private void logStats(int result) {

                    FrameworkStatsLog.write(

                            FrameworkStatsLog.ATTENTION_MANAGER_SERVICE_RESULT_REPORTED,

                            result);

                }

            };

        }



        void cancelInternal() {
@@ -611,6 +602,16 @@ public class AttentionManagerService extends SystemService { 
            }

        }



        private void appendResultToAttentionCacheBuffer(AttentionCheckCache cache) {

            synchronized (mLock) {

                if (mAttentionCheckCacheBuffer == null) {

                    mAttentionCheckCacheBuffer = new AttentionCheckCacheBuffer();

                }

                mAttentionCheckCacheBuffer.add(cache);

            }

        }





        private class AttentionServiceConnection implements ServiceConnection {

            @Override

            public void onServiceConnected(ComponentName name, IBinder service) {

services/tests/servicestests/src/com/android/server/attention/AttentionManagerServiceTest.java

+3 −3
Original line number Diff line number Diff line
@@ -70,7 +70,7 @@ public class AttentionManagerServiceTest { 
    @Mock

    private AttentionHandler mMockHandler;

    @Mock

    private IAttentionCallback mMockIAttentionCallback;

    private UserState mMockUserState;

    @Mock

    private IPowerManager mMockIPowerManager;

    @Mock
@@ -114,7 +114,7 @@ public class AttentionManagerServiceTest { 
    @Test

    public void testCancelAttentionCheck_noCrashWhenCallbackMismatched() {

        mSpyUserState.mCurrentAttentionCheck =

                new AttentionCheck(mMockAttentionCallbackInternal, mMockIAttentionCallback);

                new AttentionCheck(mMockAttentionCallbackInternal, mMockUserState);

        doReturn(mSpyUserState).when(mSpyAttentionManager).peekCurrentUserStateLocked();

        mSpyAttentionManager.cancelAttentionCheck(null);

    }
@@ -122,7 +122,7 @@ public class AttentionManagerServiceTest { 
    @Test

    public void testCancelAttentionCheck_cancelCallbackWhenMatched() {

        mSpyUserState.mCurrentAttentionCheck =

                new AttentionCheck(mMockAttentionCallbackInternal, mMockIAttentionCallback);

                new AttentionCheck(mMockAttentionCallbackInternal, mMockUserState);

        doReturn(mSpyUserState).when(mSpyAttentionManager).peekCurrentUserStateLocked();

        mSpyAttentionManager.cancelAttentionCheck(mMockAttentionCallbackInternal);

        verify(mSpyAttentionManager).cancel(any());