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

Commit e08c1a5f authored by jasonwshsu's avatar jasonwshsu
Browse files

Flash Notifications color dialog crashs if configuration changes 

Root Cause:
When configuration changes, it will recreate fragment then call no args constructor which lead to null pointer exception for error passing parameters in getInstance().

Solution:
* Use setArguments() to pass parameters.
* Move settings key update action into fragment.
* Obsever settings key changes to update summary accordingly.

Bug: 349049233
Test: atest ScreenFlashNotificationColorDialogFragmentTest ScreenFlashNotificationPreferenceControllerTest
Flag: EXEMPT bugfix
Change-Id: Ided0853cf9a989e2f288a85c1aa709f71f7e1cd6
parent 93b09303

src/com/android/settings/accessibility/ScreenFlashNotificationColorDialogFragment.java

+14 −9
Original line number Diff line number Diff line
@@ -28,11 +28,13 @@ import android.content.Intent; 
import android.graphics.Color;

import android.os.Bundle;

import android.os.UserHandle;

import android.provider.Settings;

import android.view.View;



import androidx.annotation.ColorInt;

import androidx.annotation.NonNull;

import androidx.annotation.Nullable;

import androidx.annotation.VisibleForTesting;

import androidx.appcompat.app.AlertDialog;

import androidx.fragment.app.DialogFragment;
@@ -40,8 +42,6 @@ import com.android.settings.R; 


import java.util.Timer;

import java.util.TimerTask;

import java.util.function.Consumer;





/**

 * DialogFragment for Screen flash notification color picker.
@@ -49,29 +49,33 @@ import java.util.function.Consumer; 
public class ScreenFlashNotificationColorDialogFragment extends DialogFragment implements

        ColorSelectorLayout.OnCheckedChangeListener {



    private static final int DEFAULT_COLOR = Color.TRANSPARENT;

    private static final int PREVIEW_LONG_TIME_MS = 5000;

    private static final int BETWEEN_STOP_AND_START_DELAY_MS = 250;

    private static final int MARGIN_FOR_STOP_DELAY_MS = 100;



    @VisibleForTesting

    static final String EXTRA_COLOR = "extra_color";

    @ColorInt

    private int mCurrentColor = Color.TRANSPARENT;

    private Consumer<Integer> mConsumer;

    private int mCurrentColor = DEFAULT_COLOR;



    private Timer mTimer = null;

    private Boolean mIsPreview = false;



    static ScreenFlashNotificationColorDialogFragment getInstance(int initialColor,

            Consumer<Integer> colorConsumer) {

    static ScreenFlashNotificationColorDialogFragment getInstance(int initialColor) {

        final ScreenFlashNotificationColorDialogFragment result =

                new ScreenFlashNotificationColorDialogFragment();

        result.mCurrentColor = initialColor;

        result.mConsumer = colorConsumer != null ? colorConsumer : i -> {};

        Bundle bundle = new Bundle();

        bundle.putInt(EXTRA_COLOR, initialColor);

        result.setArguments(bundle);

        return result;

    }



    @NonNull

    @Override

    public Dialog onCreateDialog(@Nullable Bundle savedInstanceState) {

        mCurrentColor = getArguments().getInt(EXTRA_COLOR, DEFAULT_COLOR);



        final View dialogView = getLayoutInflater().inflate(R.layout.layout_color_selector_dialog,

                null);
@@ -90,7 +94,8 @@ public class ScreenFlashNotificationColorDialogFragment extends DialogFragment i 
                })

                .setPositiveButton(R.string.color_selector_dialog_save, (dialog, which) -> {

                    mCurrentColor = colorSelectorLayout.getCheckedColor(DEFAULT_SCREEN_FLASH_COLOR);

                    mConsumer.accept(mCurrentColor);

                    Settings.System.putInt(getContext().getContentResolver(),

                            Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR, mCurrentColor);

                })

                .create();

        createdDialog.setOnShowListener(

src/com/android/settings/accessibility/ScreenFlashNotificationPreferenceController.java

+57 −10
Original line number Diff line number Diff line
@@ -21,10 +21,17 @@ import static com.android.settings.accessibility.AccessibilityUtil.State.ON; 
import static com.android.settings.accessibility.FlashNotificationsUtil.DEFAULT_SCREEN_FLASH_COLOR;



import android.content.Context;

import android.database.ContentObserver;

import android.graphics.Color;

import android.net.Uri;

import android.os.Handler;

import android.provider.Settings;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;

import androidx.fragment.app.Fragment;

import androidx.lifecycle.DefaultLifecycleObserver;

import androidx.lifecycle.LifecycleOwner;

import androidx.preference.Preference;

import androidx.preference.PreferenceScreen;
@@ -32,24 +39,37 @@ import com.android.settings.R; 
import com.android.settings.core.TogglePreferenceController;

import com.android.settings.overlay.FeatureFactory;



import java.util.function.Consumer;



/**

 * Controller for Screen flash notification.

 */

public class ScreenFlashNotificationPreferenceController extends TogglePreferenceController {

public class ScreenFlashNotificationPreferenceController extends

        TogglePreferenceController implements DefaultLifecycleObserver {



    private final FlashNotificationColorContentObserver mFlashNotificationColorContentObserver;



    private Fragment mParentFragment;

    private Preference mPreference;



    public ScreenFlashNotificationPreferenceController(Context context, String preferenceKey) {

        super(context, preferenceKey);

        mFlashNotificationColorContentObserver = new FlashNotificationColorContentObserver(

                new Handler(mContext.getMainLooper()));

    }



    public void setParentFragment(Fragment parentFragment) {

        this.mParentFragment = parentFragment;

    }



    @Override

    public void onStart(@NonNull LifecycleOwner owner) {

        mFlashNotificationColorContentObserver.register(mContext);

    }



    @Override

    public void onStop(@NonNull LifecycleOwner owner) {

        mFlashNotificationColorContentObserver.unregister(mContext);

    }



    @Override

    public int getAvailabilityStatus() {

        return AVAILABLE;
@@ -100,14 +120,8 @@ public class ScreenFlashNotificationPreferenceController extends TogglePreferenc 
                    Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR,

                    DEFAULT_SCREEN_FLASH_COLOR);



            final Consumer<Integer> consumer = color -> {

                Settings.System.putInt(mContext.getContentResolver(),

                        Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR, color);

                refreshColorSummary();

            };



            ScreenFlashNotificationColorDialogFragment

                    .getInstance(initialColor, consumer)

                    .getInstance(initialColor)

                    .show(mParentFragment.getParentFragmentManager(),

                            ScreenFlashNotificationColorDialogFragment.class.getSimpleName());

            return true;
@@ -128,4 +142,37 @@ public class ScreenFlashNotificationPreferenceController extends TogglePreferenc 
    private void refreshColorSummary() {

        if (mPreference != null) mPreference.setSummary(getSummary());

    }



    private final class FlashNotificationColorContentObserver extends ContentObserver {

        private final Uri mColorUri = Settings.System.getUriFor(

                Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR);



        FlashNotificationColorContentObserver(Handler handler) {

            super(handler);

        }



        /**

         * Register this observer to given {@link Context}, to be called from lifecycle

         * {@code onStart} method.

         */

        public void register(@NonNull Context context) {

            context.getContentResolver().registerContentObserver(

                    mColorUri, /* notifyForDescendants= */ false, this);

        }



        /**

         * Unregister this observer from given {@link Context}, to be called from lifecycle

         * {@code onStop} method.

         */

        public void unregister(@NonNull Context context) {

            context.getContentResolver().unregisterContentObserver(this);

        }



        @Override

        public void onChange(boolean selfChange, @Nullable Uri uri) {

            if (mColorUri.equals(uri)) {

                refreshColorSummary();

            }

        }

    }

}

tests/robotests/src/com/android/settings/accessibility/ScreenFlashNotificationColorDialogFragmentTest.java

+21 −14
Original line number Diff line number Diff line
@@ -37,10 +37,13 @@ import static org.robolectric.Shadows.shadowOf; 


import android.content.Intent;

import android.graphics.Color;

import android.os.Bundle;

import android.provider.Settings;



import androidx.appcompat.app.AlertDialog;

import androidx.fragment.app.testing.FragmentScenario;

import androidx.lifecycle.Lifecycle;

import androidx.test.core.app.ApplicationProvider;



import com.android.settings.R;

import com.android.settings.testutils.FakeTimer;
@@ -56,23 +59,26 @@ import org.robolectric.util.ReflectionHelpers; 


import java.util.List;

import java.util.Timer;

import java.util.function.Consumer;



@RunWith(RobolectricTestRunner.class)

public class ScreenFlashNotificationColorDialogFragmentTest {



    private static final int DEFAULT_COLOR = ROSE.mColorInt;



    private FragmentScenario<TestScreenFlashNotificationColorDialogFragment> mFragmentScenario;

    private ScreenFlashNotificationColorDialogFragment mDialogFragment;

    private AlertDialog mAlertDialog;

    private ColorSelectorLayout mColorSelectorLayout;

    private int mCurrentColor;



    @Before

    public void setUp() {

        mCurrentColor = ROSE.mColorInt;

        Settings.System.putInt(ApplicationProvider.getApplicationContext().getContentResolver(),

                Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR, DEFAULT_COLOR);

        Bundle fragmentArgs = new Bundle();

        fragmentArgs.putInt(ScreenFlashNotificationColorDialogFragment.EXTRA_COLOR, DEFAULT_COLOR);

        mFragmentScenario = FragmentScenario.launch(

                TestScreenFlashNotificationColorDialogFragment.class,

                /* fragmentArgs= */ null,

                fragmentArgs,

                R.style.Theme_AlertDialog_SettingsLib,

                Lifecycle.State.INITIALIZED);

        setupFragment();
@@ -99,7 +105,7 @@ public class ScreenFlashNotificationColorDialogFragmentTest { 
        performClickOnDialog(BUTTON_NEUTRAL);

        getTimerFromFragment().runOneTask();



        assertStartPreview(ROSE.mColorInt);

        assertStartPreview(DEFAULT_COLOR);

    }



    @Test
@@ -168,20 +174,26 @@ public class ScreenFlashNotificationColorDialogFragmentTest { 
    }



    @Test

    public void clickColorAndClickNegative_assertColor() {

    public void clickColorAndClickNegative_assertDefaultColor() {

        checkColorButton(AZURE);

        performClickOnDialog(BUTTON_NEGATIVE);



        assertThat(getTimerFromFragment()).isNull();

        assertThat(mCurrentColor).isEqualTo(ROSE.mColorInt);

        assertThat(Settings.System.getInt(

                ApplicationProvider.getApplicationContext().getContentResolver(),

                Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR, AZURE.mColorInt)).isEqualTo(

                DEFAULT_COLOR);

    }



    @Test

    public void clickColorAndClickPositive_assertColor() {

    public void clickColorAndClickPositive_assertSameColor() {

        checkColorButton(BLUE);

        performClickOnDialog(BUTTON_POSITIVE);



        assertThat(mCurrentColor).isEqualTo(BLUE.mColorInt);

        assertThat(Settings.System.getInt(

                ApplicationProvider.getApplicationContext().getContentResolver(),

                Settings.System.SCREEN_FLASH_NOTIFICATION_COLOR, DEFAULT_COLOR)).isEqualTo(

                BLUE.mColorInt);

    }



    private void checkColorButton(ScreenFlashNotificationColor color) {
@@ -201,11 +213,6 @@ public class ScreenFlashNotificationColorDialogFragmentTest { 
    }



    private void setupFragment() {

        mFragmentScenario.onFragment(fragment -> {

            ReflectionHelpers.setField(fragment, "mCurrentColor", mCurrentColor);

            ReflectionHelpers.setField(fragment, "mConsumer",

                    (Consumer<Integer>) selectedColor -> mCurrentColor = selectedColor);

        });

        mFragmentScenario.moveToState(Lifecycle.State.RESUMED);



        mFragmentScenario.onFragment(fragment -> {

tests/robotests/src/com/android/settings/accessibility/ScreenFlashNotificationPreferenceControllerTest.java

+5 −5
Original line number Diff line number Diff line
@@ -57,8 +57,6 @@ import org.robolectric.annotation.Implementation; 
import org.robolectric.annotation.Implements;

import org.robolectric.annotation.Resetter;



import java.util.function.Consumer;



@RunWith(RobolectricTestRunner.class)

@Config(shadows = {

        ScreenFlashNotificationPreferenceControllerTest
@@ -83,7 +81,6 @@ public class ScreenFlashNotificationPreferenceControllerTest { 
    private FragmentManager mFragmentManager;

    @Mock

    private ScreenFlashNotificationColorDialogFragment mDialogFragment;



    private ScreenFlashNotificationPreferenceController mController;

    private ContentResolver mContentResolver;
@@ -92,6 +89,7 @@ public class ScreenFlashNotificationPreferenceControllerTest { 
        MockitoAnnotations.initMocks(this);

        FragmentActivity fragmentActivity = Robolectric.setupActivity(FragmentActivity.class);

        Context context = fragmentActivity.getApplicationContext();



        ShadowScreenFlashNotificationColorDialogFragment.setInstance(mDialogFragment);

        ShadowFlashNotificationsUtils.setColorDescriptionText(COLOR_DESCRIPTION_TEXT);
@@ -99,8 +97,9 @@ public class ScreenFlashNotificationPreferenceControllerTest { 
        mController = new ScreenFlashNotificationPreferenceController(context, PREFERENCE_KEY);

        when(mPreferenceScreen.findPreference(PREFERENCE_KEY)).thenReturn(mPreference);

        when(mPreference.getKey()).thenReturn(PREFERENCE_KEY);

        mController.setParentFragment(mParentFragment);

        when(mParentFragment.getParentFragmentManager()).thenReturn(mFragmentManager);



        mController.setParentFragment(mParentFragment);

    }



    @After
@@ -181,6 +180,7 @@ public class ScreenFlashNotificationPreferenceControllerTest { 
    @Test

    public void handlePreferenceTreeClick() {

        mController.handlePreferenceTreeClick(mPreference);



        verify(mDialogFragment).show(any(FragmentManager.class), anyString());

    }
@@ -194,7 +194,7 @@ public class ScreenFlashNotificationPreferenceControllerTest { 


        @Implementation

        protected static ScreenFlashNotificationColorDialogFragment getInstance(

                int initialColor, Consumer<Integer> colorConsumer) {

                int initialColor) {

            return sInstance;

        }