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

Commit 999372ca authored by Derek Jedral's avatar Derek Jedral
Browse files

Fix NPE with registering content listener

1) Fix the condition to check either subscribed or null.
2) Don't register listeners unless the feature flag is enabled

Test: make RunSettingsRoboTest
Bug: 267357231
Change-Id: I0134812cbac60c394d96c5a5621a7c16d781b05d
parent 957a35bd
Loading
Loading
Loading
Loading
+10 −8
Original line number Diff line number Diff line
@@ -87,10 +87,10 @@ public class ActiveUnlockContentListener {

    }

    /** Starts listening for updates from the ContentProvider, and fetches the current value. */
    public synchronized void subscribe() {
        if (mSubscribed && mUri != null) {
            return;
    /** Returns true if start listening for updates from the ContentProvider, false otherwise. */
    public synchronized boolean subscribe() {
        if (mSubscribed || mUri == null) {
            return false;
        }
        mSubscribed = true;
        mContext.getContentResolver().registerContentObserver(
@@ -99,15 +99,17 @@ public class ActiveUnlockContentListener {
                () -> {
                    getContentFromUri();
                });
        return true;
    }

    /** Stops listening for updates from the ContentProvider. */
    public synchronized void unsubscribe() {
        if (!mSubscribed && mUri != null) {
            return;
    /** Returns true if stops listening for updates from the ContentProvider, false otherewise. */
    public synchronized boolean unsubscribe() {
        if (!mSubscribed || mUri == null) {
            return false;
        }
        mSubscribed = false;
        mContext.getContentResolver().unregisterContentObserver(mContentObserver);
        return true;
    }

    /** Retrieves the most recently fetched value from the ContentProvider. */
+11 −5
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ public class ActiveUnlockStatusPreferenceController
    private final CombinedBiometricStatusUtils mCombinedBiometricStatusUtils;
    private final ActiveUnlockSummaryListener mActiveUnlockSummaryListener;
    private final ActiveUnlockDeviceNameListener mActiveUnlockDeviceNameListener;
    private final boolean mIsAvailable;

    public ActiveUnlockStatusPreferenceController(@NonNull Context context) {
        this(context, KEY_ACTIVE_UNLOCK_SETTINGS);
@@ -59,6 +60,7 @@ public class ActiveUnlockStatusPreferenceController
            @NonNull Context context, @NonNull String key) {
        super(context, key);
        mActiveUnlockStatusUtils = new ActiveUnlockStatusUtils(context);
        mIsAvailable = mActiveUnlockStatusUtils.isAvailable();
        mCombinedBiometricStatusUtils = new CombinedBiometricStatusUtils(context, getUserId());
        OnContentChangedListener onSummaryChangedListener = new OnContentChangedListener() {
            @Override
@@ -90,9 +92,11 @@ public class ActiveUnlockStatusPreferenceController
    /** Subscribes to update preference summary dynamically. */
    @OnLifecycleEvent(Lifecycle.Event.ON_START)
    public void onStart() {
        if (mIsAvailable) {
            mActiveUnlockSummaryListener.subscribe();
            mActiveUnlockDeviceNameListener.subscribe();
        }
    }

    /** Resets the preference reference on resume. */
    @OnLifecycleEvent(Lifecycle.Event.ON_RESUME)
@@ -105,9 +109,11 @@ public class ActiveUnlockStatusPreferenceController
    /** Unsubscribes to prevent leaked listener. */
    @OnLifecycleEvent(Lifecycle.Event.ON_STOP)
    public void onStop() {
        if (mIsAvailable) {
            mActiveUnlockSummaryListener.unsubscribe();
            mActiveUnlockDeviceNameListener.unsubscribe();
        }
    }

    @Override
    public void displayPreference(PreferenceScreen screen) {
@@ -127,7 +133,7 @@ public class ActiveUnlockStatusPreferenceController
        // This should never be called, as getAvailabilityStatus() will return the exact value.
        // However, this is an abstract method in BiometricStatusPreferenceController, and so
        // needs to be overridden.
        return mActiveUnlockStatusUtils.isAvailable();
        return mIsAvailable;
    }

    @Override
+23 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.settings.biometrics.activeunlock;

import static com.google.common.truth.Truth.assertThat;

import static org.mockito.Mockito.any;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
import static org.robolectric.shadows.ShadowLooper.idleMainLooper;
@@ -43,6 +44,8 @@ import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;

import java.util.ArrayList;

@RunWith(RobolectricTestRunner.class)
@Config(shadows = {ShadowDeviceConfig.class})
public class ActiveUnlockContentListenerTest {
@@ -136,6 +139,26 @@ public class ActiveUnlockContentListenerTest {
        assertThat(mUpdateCount).isEqualTo(1);
    }

    @Test
    public void noProvider_subscribeDoesntRegisterObserver() {
        when(mPackageManager.getInstalledPackages(any()))
                .thenReturn(new ArrayList<>());
        OnContentChangedListener listener = new OnContentChangedListener() {
            @Override
            public void onContentChanged(String newValue) {}
        };

        ActiveUnlockContentListener contentListener =
                new ActiveUnlockContentListener(
                        mContext,
                        listener,
                        "logTag",
                        FakeContentProvider.METHOD_SUMMARY,
                        FakeContentProvider.KEY_SUMMARY);

        assertThat(contentListener.subscribe()).isFalse();
    }

    private void updateContent(String content) {
        FakeContentProvider.setTileSummary(content);
        mContext.getContentResolver().notifyChange(