Privacy Guide: Fix crash on Activity recreation

The initial fragment state is not set in PrivacyGuideMetricsDelegate when te Fragment/Activity is recreated. This causes a crash when the user tries to navigate away from the current step by clicking the next button. This CL fixes that by correctly saving and restoring the initial fragment state in PrivacyGuideMetricsDelegate. (cherry picked from commit 5625802) Bug: 1446880, 1238896 Change-Id: I7b505548f106cf14515685053109364d26b9ac1e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583513 Commit-Queue: Rubin Deliallisi <rubindl@chromium.org> Reviewed-by: Rohit Agarwal <roagarwal@chromium.org> Reviewed-by: Filipa Senra <fsenra@google.com> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com> Cr-Original-Commit-Position: refs/heads/main@{#1152523} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4591190 Auto-Submit: Rubin Deliallisi <rubindl@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/branch-heads/5790@{#366} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · Jun 5, 2023 · 32d561c · 32d561c
1 parent 0f5fd41
commit 32d561c
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 0 deletions.
diff --git a/...uide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragment.java b/...uide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragment.java
@@ -71,6 +71,9 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         setHasOptionsMenu(true);
         mPrivacyGuideMetricsDelegate = new PrivacyGuideMetricsDelegate();
+        if (savedInstanceState != null) {
+            mPrivacyGuideMetricsDelegate.restoreState(savedInstanceState);
+        }
     }
 
     @Nullable
@@ -206,6 +209,12 @@ public boolean onOptionsItemSelected(@NonNull MenuItem item) {
         return false;
     }
 
+    @Override
+    public void onSaveInstanceState(@NonNull Bundle outState) {
+        mPrivacyGuideMetricsDelegate.saveState(outState);
+        super.onSaveInstanceState(outState);
+    }
+
     public void setBottomSheetController(BottomSheetController bottomSheetController) {
         mBottomSheetController = bottomSheetController;
     }

diff --git a/...droid/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideMetricsDelegate.java b/...droid/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideMetricsDelegate.java
@@ -4,6 +4,9 @@
 
 package org.chromium.chrome.browser.privacy_guide;
 
+import android.os.Bundle;
+
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
 import org.chromium.base.metrics.RecordHistogram;
@@ -16,6 +19,11 @@
  * Privacy Guide {@link PrivacyGuideFragment}.
  */
 class PrivacyGuideMetricsDelegate {
+    private static final String INITIAL_MSBB_STATE = "INITIAL_MSBB_STATE";
+    private static final String INITIAL_HISTORY_SYNC_STATE = "INITIAL_HISTORY_SYNC_STATE";
+    private static final String INITIAL_SAFE_BROWSING_STATE = "INITIAL_SAFE_BROWSING_STATE";
+    private static final String INITIAL_COOKIES_CONTROL_MODE = "INITIAL_COOKIES_CONTROL_MODE";
+
     /**
      * Initial state of the MSBB when {@link MSBBFragment} is created.
      */
@@ -33,6 +41,42 @@ class PrivacyGuideMetricsDelegate {
      */
     private @Nullable @CookieControlsMode Integer mInitialCookiesControlMode;
 
+    /**
+     * A method to persist the initial state of all Fragments on Activity destruction.
+     */
+    void saveState(@NonNull Bundle bundle) {
+        if (mInitialMsbbState != null) {
+            bundle.putBoolean(INITIAL_MSBB_STATE, mInitialMsbbState);
+        }
+        if (mInitialHistorySyncState != null) {
+            bundle.putBoolean(INITIAL_HISTORY_SYNC_STATE, mInitialHistorySyncState);
+        }
+        if (mInitialSafeBrowsingState != null) {
+            bundle.putInt(INITIAL_SAFE_BROWSING_STATE, mInitialSafeBrowsingState);
+        }
+        if (mInitialCookiesControlMode != null) {
+            bundle.putInt(INITIAL_COOKIES_CONTROL_MODE, mInitialCookiesControlMode);
+        }
+    }
+
+    /**
+     * A method to restore the initial state of all Fragments on Activity recreation.
+     */
+    void restoreState(@NonNull Bundle bundle) {
+        if (bundle.containsKey(INITIAL_MSBB_STATE)) {
+            mInitialMsbbState = bundle.getBoolean(INITIAL_MSBB_STATE);
+        }
+        if (bundle.containsKey(INITIAL_HISTORY_SYNC_STATE)) {
+            mInitialHistorySyncState = bundle.getBoolean(INITIAL_HISTORY_SYNC_STATE);
+        }
+        if (bundle.containsKey(INITIAL_SAFE_BROWSING_STATE)) {
+            mInitialSafeBrowsingState = bundle.getInt(INITIAL_SAFE_BROWSING_STATE);
+        }
+        if (bundle.containsKey(INITIAL_COOKIES_CONTROL_MODE)) {
+            mInitialCookiesControlMode = bundle.getInt(INITIAL_COOKIES_CONTROL_MODE);
+        }
+    }
+
     /**
      * A method to record metrics on the next click of {@link MSBBFragment}
      */

diff --git a/...oid/javatests/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragmentTest.java b/...oid/javatests/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragmentTest.java
@@ -601,6 +601,24 @@ public void testMSBBCard_nextButtonInitialMSBBStateIsSet() {
         mSettingsActivityTestRule.getFragment().setPrivacyGuideMetricsDelegateForTesting(null);
     }
 
+    @Test
+    @LargeTest
+    @Feature({"PrivacyGuide"})
+    public void testMSBBCard_nextButtonAfterActivityRecreation() {
+        setMSBBState(false);
+        launchPrivacyGuide();
+        navigateFromWelcomeToMSBBCard();
+
+        var histogram = HistogramWatcher.newSingleRecordWatcher(
+                SETTINGS_STATES_HISTOGRAM, PrivacyGuideSettingsStates.MSBB_OFF_TO_ON);
+
+        onView(withId(R.id.msbb_switch)).perform(click());
+        mSettingsActivityTestRule.recreateActivity();
+        navigateFromMSBBToHistorySyncCard();
+
+        histogram.assertExpected();
+    }
+
     @Test
     @LargeTest
     @Feature({"PrivacyGuide"})
@@ -711,6 +729,24 @@ public void testHistorySyncCard_onToOnSettingsStatesHistogram() {
         histogram.assertExpected();
     }
 
+    @Test
+    @LargeTest
+    @Feature({"PrivacyGuide"})
+    public void testHistorySyncCard_nextButtonAfterActivityRecreation() {
+        setHistorySyncState(false);
+        launchPrivacyGuide();
+        goToHistorySyncCard();
+
+        var histogram = HistogramWatcher.newSingleRecordWatcher(
+                SETTINGS_STATES_HISTOGRAM, PrivacyGuideSettingsStates.HISTORY_SYNC_OFF_TO_ON);
+
+        onView(withId(R.id.history_sync_switch)).perform(click());
+        mSettingsActivityTestRule.recreateActivity();
+        navigateFromHistorySyncToSBCard();
+
+        histogram.assertExpected();
+    }
+
     @Test
     @LargeTest
     @Feature({"PrivacyGuide"})
@@ -835,6 +871,24 @@ public void testSafeBrowsingCard_enhancedToStandardSettingsStatesHistogram() {
         histogram.assertExpected();
     }
 
+    @Test
+    @LargeTest
+    @Feature({"PrivacyGuide"})
+    public void testSafeBrowsingCard_nextButtonAfterActivityRecreation() {
+        setSafeBrowsingState(SafeBrowsingState.STANDARD_PROTECTION);
+        launchPrivacyGuide();
+        goToSafeBrowsingCard();
+
+        var histogram = HistogramWatcher.newSingleRecordWatcher(SETTINGS_STATES_HISTOGRAM,
+                PrivacyGuideSettingsStates.SAFE_BROWSING_STANDARD_TO_ENHANCED);
+
+        onView(withId(R.id.enhanced_option)).perform(click());
+        mSettingsActivityTestRule.recreateActivity();
+        navigateFromSBToCookiesCard();
+
+        histogram.assertExpected();
+    }
+
     @Test
     @LargeTest
     @Feature({"PrivacyGuide"})
@@ -958,6 +1012,24 @@ public void testCookiesCard_block3PTo3PSettingsStatesHistogram() {
         histogram.assertExpected();
     }
 
+    @Test
+    @LargeTest
+    @Feature({"PrivacyGuide"})
+    public void testCookiesCard_nextButtonAfterActivityRecreation() {
+        setCookieControlsMode(CookieControlsMode.INCOGNITO_ONLY);
+        launchPrivacyGuide();
+        goToCookiesCard();
+
+        var histogram = HistogramWatcher.newSingleRecordWatcher(
+                SETTINGS_STATES_HISTOGRAM, PrivacyGuideSettingsStates.BLOCK3P_INCOGNITO_TO3P);
+
+        onView(withId(R.id.block_third_party)).perform(click());
+        mSettingsActivityTestRule.recreateActivity();
+        navigateFromCookiesToCompletionCard();
+
+        histogram.assertExpected();
+    }
+
     @Test
     @LargeTest
     @Feature({"PrivacyGuide"})