From 32d561c7e7b3f5bc40a1beebdfb6757e17cb47f3 Mon Sep 17 00:00:00 2001 From: Rubin Deliallisi Date: Mon, 5 Jun 2023 19:57:45 +0000 Subject: [PATCH] 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 562580226b9683f78da279b0ed52a9b4ea83bbc0) Bug: 1446880, 1238896 Change-Id: I7b505548f106cf14515685053109364d26b9ac1e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583513 Commit-Queue: Rubin Deliallisi Reviewed-by: Rohit Agarwal Reviewed-by: Filipa Senra Code-Coverage: Findit Cr-Original-Commit-Position: refs/heads/main@{#1152523} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4591190 Auto-Submit: Rubin Deliallisi Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5790@{#366} Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114} --- .../privacy_guide/PrivacyGuideFragment.java | 9 +++ .../PrivacyGuideMetricsDelegate.java | 44 ++++++++++++ .../PrivacyGuideFragmentTest.java | 72 +++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/chrome/browser/privacy_guide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragment.java b/chrome/browser/privacy_guide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragment.java index e1217609840f8..b89c8adee6500 100644 --- a/chrome/browser/privacy_guide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragment.java +++ b/chrome/browser/privacy_guide/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/chrome/browser/privacy_guide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideMetricsDelegate.java b/chrome/browser/privacy_guide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideMetricsDelegate.java index a5865f28cb220..6429a201e9c82 100644 --- a/chrome/browser/privacy_guide/android/java/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideMetricsDelegate.java +++ b/chrome/browser/privacy_guide/android/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/chrome/browser/privacy_guide/android/javatests/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragmentTest.java b/chrome/browser/privacy_guide/android/javatests/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragmentTest.java index 4dd4352b53a65..f77ed2159dfa5 100644 --- a/chrome/browser/privacy_guide/android/javatests/src/org/chromium/chrome/browser/privacy_guide/PrivacyGuideFragmentTest.java +++ b/chrome/browser/privacy_guide/android/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"})