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

Commit 66fdd6f3 authored by Fabián Kozynski's avatar Fabián Kozynski
Browse files

Use an Optional wrapper in TileLifecycleManager

This way, we can prevent NPE, as the variable is never null.
Aditionally, make sure that we always perform operations directly on
Optional.

It's hard to test the NPE because of thread issues, but with this fix we
can guarantee:

* No NPE thanks to Optional
* Behavior is maintained thanks to CTS

Test: atest TileLifecycleManagerTest
Test: atest CtsTileServiceTestCases
Test: atest CtsSystemUiHostTestCases
Fixes: 317014639
Flag: NONE
Change-Id: I27a5e5b4ced1fe5da9b0e4b686cb5e58ca820c8c
parent 77c5fef8
Loading
Loading
Loading
Loading
+6 −1
Original line number Original line Diff line number Diff line
@@ -19,16 +19,21 @@ import android.os.IBinder;
import android.service.quicksettings.IQSTileService;
import android.service.quicksettings.IQSTileService;
import android.util.Log;
import android.util.Log;


import androidx.annotation.NonNull;



public class QSTileServiceWrapper {
public class QSTileServiceWrapper {
    private static final String TAG = "IQSTileServiceWrapper";
    private static final String TAG = "IQSTileServiceWrapper";


    @NonNull
    private final IQSTileService mService;
    private final IQSTileService mService;


    public QSTileServiceWrapper(IQSTileService service) {
    public QSTileServiceWrapper(@NonNull IQSTileService service) {
        mService = service;
        mService = service;
    }
    }


    // This can never be null, as it's the constructor parameter and it's final
    @NonNull
    public IBinder asBinder() {
    public IBinder asBinder() {
        return mService.asBinder();
        return mService.asBinder();
    }
    }
+45 −13
Original line number Original line Diff line number Diff line
@@ -40,6 +40,7 @@ import android.text.format.DateUtils;
import android.util.ArraySet;
import android.util.ArraySet;
import android.util.Log;
import android.util.Log;


import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.Nullable;
import androidx.annotation.WorkerThread;
import androidx.annotation.WorkerThread;


@@ -54,8 +55,10 @@ import dagger.assisted.AssistedInject;


import java.util.NoSuchElementException;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Predicate;


/**
/**
 * Manages the lifecycle of a TileService.
 * Manages the lifecycle of a TileService.
@@ -101,8 +104,8 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    private final ActivityManager mActivityManager;
    private final ActivityManager mActivityManager;


    private Set<Integer> mQueuedMessages = new ArraySet<>();
    private Set<Integer> mQueuedMessages = new ArraySet<>();
    @Nullable
    @NonNull
    private volatile QSTileServiceWrapper mWrapper;
    private volatile Optional<QSTileServiceWrapper> mOptionalWrapper = Optional.empty();
    private boolean mListening;
    private boolean mListening;
    private IBinder mClickBinder;
    private IBinder mClickBinder;


@@ -222,6 +225,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                // Only try a new binding if we are not currently bound.
                // Only try a new binding if we are not currently bound.
                mIsBound.compareAndSet(false, bindServices());
                mIsBound.compareAndSet(false, bindServices());
                if (!mIsBound.get()) {
                if (!mIsBound.get()) {
                    Log.d(TAG, "Failed to bind to service");
                    mContext.unbindService(this);
                    mContext.unbindService(this);
                }
                }
            } catch (SecurityException e) {
            } catch (SecurityException e) {
@@ -281,7 +285,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
            service.linkToDeath(this, 0);
            service.linkToDeath(this, 0);
        } catch (RemoteException e) {
        } catch (RemoteException e) {
        }
        }
        mWrapper = wrapper;
        mOptionalWrapper = Optional.of(wrapper);
        handlePendingMessages();
        handlePendingMessages();
    }
    }


@@ -368,6 +372,10 @@ public class TileLifecycleManager extends BroadcastReceiver implements
     * are supposed to be bound, we will try to bind after some amount of time.
     * are supposed to be bound, we will try to bind after some amount of time.
     */
     */
    private void handleDeath() {
    private void handleDeath() {
        if (!mIsBound.get()) {
            // If we are already not bound, don't do anything else.
            return;
        }
        mExecutor.execute(() -> {
        mExecutor.execute(() -> {
            if (!mIsBound.get()) {
            if (!mIsBound.get()) {
                // If we are already not bound, don't do anything else.
                // If we are already not bound, don't do anything else.
@@ -522,7 +530,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    @Override
    @Override
    public void onTileAdded() {
    public void onTileAdded() {
        if (mDebug) Log.d(TAG, "onTileAdded " + getComponent());
        if (mDebug) Log.d(TAG, "onTileAdded " + getComponent());
        if (mWrapper == null || !mWrapper.onTileAdded()) {
        if (isNullOrFailedAction(mOptionalWrapper, QSTileServiceWrapper::onTileAdded)) {
            queueMessage(MSG_ON_ADDED);
            queueMessage(MSG_ON_ADDED);
            handleDeath();
            handleDeath();
        }
        }
@@ -531,7 +539,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    @Override
    @Override
    public void onTileRemoved() {
    public void onTileRemoved() {
        if (mDebug) Log.d(TAG, "onTileRemoved " + getComponent());
        if (mDebug) Log.d(TAG, "onTileRemoved " + getComponent());
        if (mWrapper == null || !mWrapper.onTileRemoved()) {
        if (isNullOrFailedAction(mOptionalWrapper, QSTileServiceWrapper::onTileRemoved)) {
            queueMessage(MSG_ON_REMOVED);
            queueMessage(MSG_ON_REMOVED);
            handleDeath();
            handleDeath();
        }
        }
@@ -541,7 +549,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    public void onStartListening() {
    public void onStartListening() {
        if (mDebug) Log.d(TAG, "onStartListening " + getComponent());
        if (mDebug) Log.d(TAG, "onStartListening " + getComponent());
        mListening = true;
        mListening = true;
        if (mWrapper != null && !mWrapper.onStartListening()) {
        if (isNotNullAndFailedAction(mOptionalWrapper, QSTileServiceWrapper::onStartListening)) {
            handleDeath();
            handleDeath();
        }
        }
    }
    }
@@ -550,7 +558,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    public void onStopListening() {
    public void onStopListening() {
        if (mDebug) Log.d(TAG, "onStopListening " + getComponent());
        if (mDebug) Log.d(TAG, "onStopListening " + getComponent());
        mListening = false;
        mListening = false;
        if (mWrapper != null && !mWrapper.onStopListening()) {
        if (isNotNullAndFailedAction(mOptionalWrapper, QSTileServiceWrapper::onStopListening)) {
            handleDeath();
            handleDeath();
        }
        }
    }
    }
@@ -558,7 +566,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    @Override
    @Override
    public void onClick(IBinder iBinder) {
    public void onClick(IBinder iBinder) {
        if (mDebug) Log.d(TAG, "onClick " + iBinder + " " + getComponent() + " " + mUser);
        if (mDebug) Log.d(TAG, "onClick " + iBinder + " " + getComponent() + " " + mUser);
        if (mWrapper == null || !mWrapper.onClick(iBinder)) {
        if (isNullOrFailedAction(mOptionalWrapper, (wrapper) -> wrapper.onClick(iBinder))) {
            mClickBinder = iBinder;
            mClickBinder = iBinder;
            queueMessage(MSG_ON_CLICK);
            queueMessage(MSG_ON_CLICK);
            handleDeath();
            handleDeath();
@@ -568,7 +576,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    @Override
    @Override
    public void onUnlockComplete() {
    public void onUnlockComplete() {
        if (mDebug) Log.d(TAG, "onUnlockComplete " + getComponent());
        if (mDebug) Log.d(TAG, "onUnlockComplete " + getComponent());
        if (mWrapper == null || !mWrapper.onUnlockComplete()) {
        if (isNullOrFailedAction(mOptionalWrapper, QSTileServiceWrapper::onUnlockComplete)) {
            queueMessage(MSG_ON_UNLOCK_COMPLETE);
            queueMessage(MSG_ON_UNLOCK_COMPLETE);
            handleDeath();
            handleDeath();
        }
        }
@@ -577,7 +585,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    @Nullable
    @Nullable
    @Override
    @Override
    public IBinder asBinder() {
    public IBinder asBinder() {
        return mWrapper != null ? mWrapper.asBinder() : null;
        return mOptionalWrapper.map(QSTileServiceWrapper::asBinder).orElse(null);
    }
    }


    @Override
    @Override
@@ -591,18 +599,42 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    }
    }


    private void freeWrapper() {
    private void freeWrapper() {
        if (mWrapper != null) {
        if (mOptionalWrapper.isPresent()) {
            try {
            try {
                mWrapper.asBinder().unlinkToDeath(this, 0);
                mOptionalWrapper.ifPresent(
                        (wrapper) -> wrapper.asBinder().unlinkToDeath(this, 0)
                );
            } catch (NoSuchElementException e) {
            } catch (NoSuchElementException e) {
                Log.w(TAG, "Trying to unlink not linked recipient for component"
                Log.w(TAG, "Trying to unlink not linked recipient for component"
                        + mIntent.getComponent().flattenToShortString());
                        + mIntent.getComponent().flattenToShortString());
            }
            }
            mWrapper = null;
            mOptionalWrapper = Optional.empty();
        }
        }
    }
    }


    public interface TileChangeListener {
    public interface TileChangeListener {
        void onTileChanged(ComponentName tile);
        void onTileChanged(ComponentName tile);
    }
    }

    /**
     * Returns true if the Optional is empty OR performing the action on the content of the Optional
     * (when not empty) fails.
     */
    private static boolean isNullOrFailedAction(
            Optional<QSTileServiceWrapper> optionalWrapper,
            Predicate<QSTileServiceWrapper> action
    ) {
        return !optionalWrapper.map(action::test).orElse(false);
    }

    /**
     * Returns true if the Optional is not empty AND performing the action on the content of
     * the Optional fails.
     */
    private static boolean isNotNullAndFailedAction(
            Optional<QSTileServiceWrapper> optionalWrapper,
            Predicate<QSTileServiceWrapper> action
    ) {
        return  !optionalWrapper.map(action::test).orElse(true);
    }
}
}