From 53c4f5366445ec01ea4f8293436780f14493065f Mon Sep 17 00:00:00 2001 From: Lijin Shen Date: Wed, 7 Jun 2023 20:33:36 +0000 Subject: [PATCH] [M115] Add metrics recording for secondary activity back press handling This CL: 1. adds "Android.BackPress.SecondaryActivity" for recording the activity which handles the back press 2. moves BackPressHelper* to c/b/back_press 3. polishes tests 4. fixes typos in some tests (cherry picked from commit 46fe40a74991befff74e713ce0704f52918d58c9) Bug: 1406012 Change-Id: I2962d061dd9c9b419ee2aaf7b8038e83bbdc4748 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4519916 Reviewed-by: Andrew Grieve Code-Coverage: Findit Commit-Queue: Lijin Shen Cr-Original-Commit-Position: refs/heads/main@{#1152020} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599034 Reviewed-by: Theresa Sullivan Reviewed-by: Jinsuk Kim Cr-Commit-Position: refs/branch-heads/5790@{#461} Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114} --- .../base/library_loader/LibraryLoader.java | 6 ++- chrome/android/chrome_java_sources.gni | 1 - .../chrome_junit_test_java_sources.gni | 1 - .../app/bookmarks/BookmarkActivity.java | 10 +++-- .../app/download/home/DownloadActivity.java | 10 ++--- .../browser/firstrun/FirstRunActivity.java | 6 +++ .../firstrun/FirstRunActivityBase.java | 10 +++-- .../firstrun/LightweightFirstRunActivity.java | 6 +++ .../browser/history/HistoryActivity.java | 10 +++-- .../browser/history/HistoryManager.java | 1 - .../browser/settings/SettingsActivity.java | 9 ++-- .../sync/settings/ManageSyncSettings.java | 2 +- .../chrome/browser/history/HistoryUITest.java | 18 ++++++-- chrome/browser/back_press/android/BUILD.gn | 11 ++++- .../browser/back_press}/BackPressHelper.java | 44 ++++++++++++------- .../back_press}/BackPressHelperUnitTest.java | 33 +++++++++++--- .../SecondaryActivityBackPressUma.java | 37 ++++++++++++++++ .../download/home/DownloadActivityV2Test.java | 16 +++++-- tools/metrics/histograms/enums.xml | 9 ++++ .../metadata/android/histograms.xml | 10 +++++ .../org/chromium/ui/widget/ToastManager.java | 2 +- 21 files changed, 195 insertions(+), 57 deletions(-) rename chrome/{android/java/src/org/chromium/chrome/browser => browser/back_press/android/java/src/org/chromium/chrome/browser/back_press}/BackPressHelper.java (74%) rename chrome/{android/junit/src/org/chromium/chrome/browser => browser/back_press/android/java/src/org/chromium/chrome/browser/back_press}/BackPressHelperUnitTest.java (71%) create mode 100644 chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/SecondaryActivityBackPressUma.java diff --git a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java index 2f0a584834dbd..5d5815f666bf9 100644 --- a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java +++ b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java @@ -580,9 +580,13 @@ private void preloadAlreadyLocked(String packageName, boolean inZygote) { * * @deprecated: please avoid using in new code: * https://crsrc.org/c/base/android/jni_generator/README.md#testing-for-readiness-use-get + * + * TODO(crbug.com/1406012): adding back {@link VisibleForTesting} after crbug.com/1442347 is + * fixed. This method is exposed in order to unblock the test failures because of isInitialized + * loading .so file before native flags are ready. Ideally, it should be fixed by migrating + * the feature flag to CachedFlag. */ @Deprecated - @VisibleForTesting public boolean isLoaded() { return mLoadState == LoadState.LOADED && (!sEnableStateForTesting || mLoadStateForTesting == LoadState.LOADED); diff --git a/chrome/android/chrome_java_sources.gni b/chrome/android/chrome_java_sources.gni index d1c578409b6b3..9860c9bfdd83b 100644 --- a/chrome/android/chrome_java_sources.gni +++ b/chrome/android/chrome_java_sources.gni @@ -10,7 +10,6 @@ chrome_java_sources = [ "java/src/org/chromium/chrome/browser/AppHooksModule.java", "java/src/org/chromium/chrome/browser/AppIndexingUtil.java", "java/src/org/chromium/chrome/browser/ApplicationLifetime.java", - "java/src/org/chromium/chrome/browser/BackPressHelper.java", "java/src/org/chromium/chrome/browser/BackupSigninProcessor.java", "java/src/org/chromium/chrome/browser/BrowserRestartActivity.java", "java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java", diff --git a/chrome/android/chrome_junit_test_java_sources.gni b/chrome/android/chrome_junit_test_java_sources.gni index 290bbbfaedb91..25fedce27f0ea 100644 --- a/chrome/android/chrome_junit_test_java_sources.gni +++ b/chrome/android/chrome_junit_test_java_sources.gni @@ -6,7 +6,6 @@ chrome_junit_test_java_sources = [ "java/src/org/chromium/chrome/browser/segmentation_platform/SignalAccumulatorTest.java", "java/src/org/chromium/chrome/browser/tab/TabFaviconTest.java", "junit/src/org/chromium/chrome/browser/AppIndexingUtilTest.java", - "junit/src/org/chromium/chrome/browser/BackPressHelperUnitTest.java", "junit/src/org/chromium/chrome/browser/ChromeActionModeHandlerUnitTest.java", "junit/src/org/chromium/chrome/browser/ChromeActivityUnitTest.java", "junit/src/org/chromium/chrome/browser/ChromeBackgroundServiceTest.java", diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/bookmarks/BookmarkActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/app/bookmarks/BookmarkActivity.java index 3d08f3466fbef..f1c645990fbce 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/app/bookmarks/BookmarkActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/app/bookmarks/BookmarkActivity.java @@ -9,10 +9,11 @@ import android.text.TextUtils; import org.chromium.base.IntentUtils; -import org.chromium.chrome.browser.BackPressHelper; import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.SnackbarActivity; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.back_press.BackPressManager; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.bookmarks.BookmarkManagerCoordinator; import org.chromium.chrome.browser.bookmarks.BookmarkPage; import org.chromium.chrome.browser.bookmarks.BookmarkUiPrefs; @@ -47,10 +48,11 @@ true, isIncognito, getSnackbarManager(), Profile.getLastUsedRegularProfile(), mBookmarkManagerCoordinator.updateForUrl(url); setContentView(mBookmarkManagerCoordinator.getView()); if (BackPressManager.isSecondaryActivityEnabled()) { - BackPressHelper.create(this, getOnBackPressedDispatcher(), mBookmarkManagerCoordinator); + BackPressHelper.create(this, getOnBackPressedDispatcher(), mBookmarkManagerCoordinator, + SecondaryActivity.BOOKMARK); } else { - BackPressHelper.create( - this, getOnBackPressedDispatcher(), mBookmarkManagerCoordinator::onBackPressed); + BackPressHelper.create(this, getOnBackPressedDispatcher(), + mBookmarkManagerCoordinator::onBackPressed, SecondaryActivity.BOOKMARK); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/download/home/DownloadActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/app/download/home/DownloadActivity.java index e5a5ded47ece1..5b653f1c582b8 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/app/download/home/DownloadActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/app/download/home/DownloadActivity.java @@ -7,9 +7,10 @@ import android.app.Activity; import android.os.Bundle; -import org.chromium.chrome.browser.BackPressHelper; import org.chromium.chrome.browser.SnackbarActivity; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.back_press.BackPressManager; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.download.DownloadUtils; import org.chromium.chrome.browser.download.home.DownloadManagerCoordinator; import org.chromium.chrome.browser.download.home.DownloadManagerUiConfig; @@ -26,7 +27,6 @@ import org.chromium.ui.permissions.AndroidPermissionDelegate; import java.lang.ref.WeakReference; - /** * Activity for managing downloads handled through Chrome. */ @@ -81,10 +81,10 @@ public void onCreate(Bundle savedInstanceState) { mDownloadCoordinator.addObserver(mUiObserver); if (BackPressManager.isSecondaryActivityEnabled()) { BackPressHelper.create(this, getOnBackPressedDispatcher(), - mDownloadCoordinator.getBackPressHandlers()); + mDownloadCoordinator.getBackPressHandlers(), SecondaryActivity.DOWNLOAD); } else { - BackPressHelper.create( - this, getOnBackPressedDispatcher(), mDownloadCoordinator::onBackPressed); + BackPressHelper.create(this, getOnBackPressedDispatcher(), + mDownloadCoordinator::onBackPressed, SecondaryActivity.DOWNLOAD); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java index f37831e3de258..f2e72b22bf53c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java @@ -23,6 +23,7 @@ import org.chromium.base.Promise; import org.chromium.base.metrics.RecordHistogram; import org.chromium.chrome.R; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.customtabs.CustomTabActivity; import org.chromium.chrome.browser.fonts.FontPreloader; import org.chromium.chrome.browser.metrics.UmaUtils; @@ -398,6 +399,11 @@ public void onStart() { return BackPressResult.SUCCESS; } + @Override + public int getSecondaryActivity() { + return SecondaryActivity.FIRST_RUN; + } + // FirstRunPageDelegate: @Override public Bundle getProperties() { diff --git a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivityBase.java b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivityBase.java index db91b908462b1..dba5e82eb6903 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivityBase.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivityBase.java @@ -21,8 +21,9 @@ import org.chromium.base.supplier.ObservableSupplierImpl; import org.chromium.base.supplier.OneshotSupplier; import org.chromium.base.supplier.OneshotSupplierImpl; -import org.chromium.chrome.browser.BackPressHelper; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.back_press.BackPressManager; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.customtabs.CustomTabsConnection; import org.chromium.chrome.browser.init.AsyncInitializationActivity; import org.chromium.chrome.browser.metrics.SimpleStartupForegroundSessionDetector; @@ -109,12 +110,13 @@ public void triggerLayoutInflation() { protected void onPreCreate() { super.onPreCreate(); if (BackPressManager.isSecondaryActivityEnabled()) { - BackPressHelper.create(this, getOnBackPressedDispatcher(), this); + BackPressHelper.create( + this, getOnBackPressedDispatcher(), this, getSecondaryActivity()); } else { BackPressHelper.create(this, getOnBackPressedDispatcher(), () -> { handleBackPress(); return true; - }); + }, getSecondaryActivity()); } } @@ -169,6 +171,8 @@ public ObservableSupplier getHandleBackPressChangedSupplier() { @Override public abstract @BackPressResult int handleBackPress(); + public abstract @SecondaryActivity int getSecondaryActivity(); + protected void flushPersistentData() { if (mNativeInitialized) { ProfileManagerUtils.flushPersistentDataForAllProfiles(); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java index ca2f5fa146b66..fe5e73366210e 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java @@ -22,6 +22,7 @@ import org.chromium.base.ThreadUtils; import org.chromium.base.metrics.RecordHistogram; import org.chromium.chrome.R; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.customtabs.CustomTabActivity; import org.chromium.chrome.browser.enterprise.util.EnterpriseInfo; import org.chromium.ui.base.LocalizationUtils; @@ -220,6 +221,11 @@ public void onDestroy() { return BackPressResult.SUCCESS; } + @Override + public int getSecondaryActivity() { + return SecondaryActivity.LIGHTWEIGHT_FIRST_RUN; + } + private void abortFirstRunExperience() { finish(); notifyCustomTabCallbackFirstRunIfNecessary(getIntent(), false); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryActivity.java index 80a93e645c4a0..88e1ab8380b3f 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryActivity.java @@ -9,10 +9,11 @@ import androidx.annotation.VisibleForTesting; import org.chromium.base.IntentUtils; -import org.chromium.chrome.browser.BackPressHelper; import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.SnackbarActivity; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.back_press.BackPressManager; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.history_clusters.HistoryClustersConstants; import org.chromium.chrome.browser.profiles.Profile; @@ -37,10 +38,11 @@ public void onCreate(Bundle savedInstanceState) { new BrowsingHistoryBridge(Profile.getLastUsedRegularProfile())); setContentView(mHistoryManager.getView()); if (BackPressManager.isSecondaryActivityEnabled()) { - BackPressHelper.create(this, getOnBackPressedDispatcher(), mHistoryManager); - } else { BackPressHelper.create( - this, getOnBackPressedDispatcher(), mHistoryManager::onBackPressed); + this, getOnBackPressedDispatcher(), mHistoryManager, SecondaryActivity.HISTORY); + } else { + BackPressHelper.create(this, getOnBackPressedDispatcher(), + mHistoryManager::onBackPressed, SecondaryActivity.HISTORY); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java index 2fb16d39a0588..1683841350809 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java @@ -690,7 +690,6 @@ public boolean onBackPressed() { } else if (isHistoryClustersUIShowing()) { return mHistoryClustersCoordinator.onBackPressed(); } - return mSelectableListLayout.onBackPressed(); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java index 9cad0039a8587..7783471cc6799 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java @@ -34,12 +34,13 @@ import org.chromium.base.IntentUtils; import org.chromium.chrome.R; import org.chromium.chrome.browser.ApplicationLifetime; -import org.chromium.chrome.browser.BackPressHelper; import org.chromium.chrome.browser.ChromeBaseAppCompatActivity; import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.LaunchIntentDispatcher; import org.chromium.chrome.browser.accessibility.settings.ChromeAccessibilitySettingsDelegate; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.back_press.BackPressManager; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.browsing_data.ClearBrowsingDataFragmentBasic; import org.chromium.chrome.browser.feedback.FragmentHelpAndFeedbackLauncher; import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncherImpl; @@ -398,12 +399,14 @@ private void initBackPressHandler() { if (BackPressManager.isSecondaryActivityEnabled()) { if (activeFragment instanceof BackPressHandler) { BackPressHelper.create(activeFragment.getViewLifecycleOwner(), - getOnBackPressedDispatcher(), (BackPressHandler) activeFragment); + getOnBackPressedDispatcher(), (BackPressHandler) activeFragment, + SecondaryActivity.SETTINGS); } } else if (activeFragment instanceof BackPressHelper.ObsoleteBackPressedHandler) { BackPressHelper.create(activeFragment.getViewLifecycleOwner(), getOnBackPressedDispatcher(), - (BackPressHelper.ObsoleteBackPressedHandler) activeFragment); + (BackPressHelper.ObsoleteBackPressedHandler) activeFragment, + SecondaryActivity.SETTINGS); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/sync/settings/ManageSyncSettings.java b/chrome/android/java/src/org/chromium/chrome/browser/sync/settings/ManageSyncSettings.java index 3b229431fc69c..72bf104741894 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/sync/settings/ManageSyncSettings.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/sync/settings/ManageSyncSettings.java @@ -35,9 +35,9 @@ import org.chromium.base.task.TaskTraits; import org.chromium.chrome.R; import org.chromium.chrome.browser.AppHooks; -import org.chromium.chrome.browser.BackPressHelper; import org.chromium.chrome.browser.SyncFirstSetupCompleteSource; import org.chromium.chrome.browser.autofill.PersonalDataManager; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.feedback.FragmentHelpAndFeedbackLauncher; import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncher; import org.chromium.chrome.browser.profiles.Profile; diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/history/HistoryUITest.java b/chrome/android/junit/src/org/chromium/chrome/browser/history/HistoryUITest.java index 0315ab4144f2f..bcaf5b5c8aba1 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/history/HistoryUITest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/history/HistoryUITest.java @@ -52,10 +52,12 @@ import org.chromium.base.Promise; import org.chromium.base.test.BaseRobolectricTestRunner; +import org.chromium.base.test.util.HistogramWatcher; import org.chromium.base.test.util.JniMocker; import org.chromium.chrome.R; -import org.chromium.chrome.browser.BackPressHelper; import org.chromium.chrome.browser.IntentHandler; +import org.chromium.chrome.browser.back_press.BackPressHelper; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.history_clusters.HistoryClustersBridge; import org.chromium.chrome.browser.history_clusters.HistoryClustersCoordinator; @@ -203,10 +205,11 @@ public void setUp() throws Exception { // Some individual tests may override to enable this feature which is disabled by // the class by default. if (ChromeFeatureList.isEnabled(ChromeFeatureList.BACK_GESTURE_REFACTOR_ACTIVITY)) { - BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mHistoryManager); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mHistoryManager, + SecondaryActivity.HISTORY); } else { - BackPressHelper.create( - mLifecycleOwner, mOnBackPressedDispatcher, mHistoryManager::onBackPressed); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, + mHistoryManager::onBackPressed, SecondaryActivity.HISTORY); } } @@ -478,6 +481,7 @@ public void testSearchView() throws Exception { @Test @SmallTest + @DisableFeatures({ChromeFeatureList.BACK_GESTURE_REFACTOR_ACTIVITY}) public void testSearchViewDismissedByBackPress() { final HistoryManagerToolbar toolbar = mHistoryManager.getToolbarForTests(); View toolbarShadow = mHistoryManager.getSelectableListLayout().getToolbarShadowForTests(); @@ -494,17 +498,23 @@ public void testSearchViewDismissedByBackPress() { Assert.assertEquals(View.GONE, toolbarSearchView.getVisibility()); // Press back press to unselect item and the search view is showing again. + var backPressRecorder = HistogramWatcher.newSingleRecordWatcher( + "Android.BackPress.SecondaryActivity", SecondaryActivity.HISTORY); Assert.assertTrue(mHistoryManager.getHandleBackPressChangedSupplier().get()); TestThreadUtils.runOnUiThreadBlocking(mOnBackPressedDispatcher::onBackPressed); Assert.assertFalse(mHistoryManager.getSelectionDelegateForTests().isSelectionEnabled()); Assert.assertEquals(View.GONE, toolbarShadow.getVisibility()); Assert.assertEquals(View.VISIBLE, toolbarSearchView.getVisibility()); + backPressRecorder.assertExpected(); // Press back to close the search view. + var backPressRecorder2 = HistogramWatcher.newSingleRecordWatcher( + "Android.BackPress.SecondaryActivity", SecondaryActivity.HISTORY); Assert.assertTrue(mHistoryManager.getHandleBackPressChangedSupplier().get()); TestThreadUtils.runOnUiThreadBlocking(mOnBackPressedDispatcher::onBackPressed); Assert.assertEquals(View.GONE, toolbarShadow.getVisibility()); Assert.assertEquals(View.GONE, toolbarSearchView.getVisibility()); + backPressRecorder2.assertExpected(); } @Test diff --git a/chrome/browser/back_press/android/BUILD.gn b/chrome/browser/back_press/android/BUILD.gn index a1959572eed82..b799e5628980f 100644 --- a/chrome/browser/back_press/android/BUILD.gn +++ b/chrome/browser/back_press/android/BUILD.gn @@ -6,8 +6,10 @@ import("//build/config/android/rules.gni") android_library("java") { sources = [ + "java/src/org/chromium/chrome/browser/back_press/BackPressHelper.java", "java/src/org/chromium/chrome/browser/back_press/BackPressManager.java", "java/src/org/chromium/chrome/browser/back_press/MinimizeAppAndCloseTabBackPressHandler.java", + "java/src/org/chromium/chrome/browser/back_press/SecondaryActivityBackPressUma.java", ] deps = [ @@ -21,11 +23,15 @@ android_library("java") { "//content/public/android:content_full_java", "//third_party/androidx:androidx_activity_activity_java", "//third_party/androidx:androidx_annotation_annotation_java", + "//third_party/androidx:androidx_lifecycle_lifecycle_common_java", ] } robolectric_library("junit") { - sources = [ "java/src/org/chromium/chrome/browser/back_press/BackPressManagerUnitTest.java" ] + sources = [ + "java/src/org/chromium/chrome/browser/back_press/BackPressHelperUnitTest.java", + "java/src/org/chromium/chrome/browser/back_press/BackPressManagerUnitTest.java", + ] deps = [ ":java", @@ -36,7 +42,10 @@ robolectric_library("junit") { "//chrome/browser/flags:java", "//components/browser_ui/widget/android:java", "//third_party/androidx:androidx_activity_activity_java", + "//third_party/androidx:androidx_lifecycle_lifecycle_common_java", + "//third_party/androidx:androidx_lifecycle_lifecycle_runtime_java", "//third_party/junit:junit", + "//third_party/mockito:mockito_java", ] } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/BackPressHelper.java b/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/BackPressHelper.java similarity index 74% rename from chrome/android/java/src/org/chromium/chrome/browser/BackPressHelper.java rename to chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/BackPressHelper.java index 5f9a74a857702..10be7740a7863 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/BackPressHelper.java +++ b/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/BackPressHelper.java @@ -2,12 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -package org.chromium.chrome.browser; +package org.chromium.chrome.browser.back_press; import androidx.activity.OnBackPressedCallback; import androidx.activity.OnBackPressedDispatcher; import androidx.lifecycle.LifecycleOwner; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.components.browser_ui.widget.gesture.BackPressHandler; /** @@ -28,31 +29,35 @@ public interface ObsoleteBackPressedHandler { } /** - * @deprecated Create a {@link BackPressHelper} that can handle a chain of handlers. - * Prefer {@link #create(LifecycleOwner, OnBackPressedDispatcher, BackPressHandler)} - * whenever possible. * @param lifecycleOwner {@link LifecycleOwner} managing the back press logic's lifecycle. * @param dispatcher {@link OnBackPressedDispatcher} that holds other callbacks. - * @param handler {@link ObsoleteBackPressedHandler} implementing the caller's back press - * handler. + * @param handler {@link ObsoleteBackPressedHandler} implementing the caller's back press. + * @param activity {@link SecondaryActivity} handling the back press. + * handler. + * @deprecated Create a {@link BackPressHelper} that can handle a chain of handlers. Prefer + * {@link #create(LifecycleOwner, OnBackPressedDispatcher, BackPressHandler, int)} whenever + * possible. */ @Deprecated public static void create(LifecycleOwner lifecycleOwner, OnBackPressedDispatcher dispatcher, - ObsoleteBackPressedHandler handler) { - new BackPressHelper(lifecycleOwner, dispatcher, handler); + ObsoleteBackPressedHandler handler, @SecondaryActivity int activity) { + new BackPressHelper(lifecycleOwner, dispatcher, handler, activity); } /** * Register a {@link BackPressHandler} on a given {@link OnBackPressedDispatcher}. + * * @param lifecycleOwner {@link LifecycleOwner} managing the back press logic's lifecycle. * @param dispatcher {@link OnBackPressedDispatcher} that holds other callbacks. * @param handler {@link BackPressHandler} observing back press state and consuming back press. + * @param activity {@link SecondaryActivity} handling the back press. */ public static void create(LifecycleOwner lifecycleOwner, OnBackPressedDispatcher dispatcher, - BackPressHandler handler) { + BackPressHandler handler, @SecondaryActivity int activity) { var callback = new OnBackPressedCallback(/* enabled */ false) { @Override public void handleOnBackPressed() { + SecondaryActivityBackPressUma.record(activity); handler.handleBackPress(); } }; @@ -61,20 +66,23 @@ public void handleOnBackPressed() { } /** - * Register a list of {@link BackPressHandler} on a given {@link OnBackPressedDispatcher}. - * The first handler has the top priority and the last one has the least. + * Register a list of {@link BackPressHandler} on a given {@link OnBackPressedDispatcher}. The + * first handler has the top priority and the last one has the least. * TODO(https://crbug.com/1406012): consider introducing a lightweight * {@link org.chromium.chrome.browser.back_press.BackPressManager} if too many handlers should * be registered. + * * @param lifecycleOwner {@link LifecycleOwner} managing the back press logic's lifecycle. * @param dispatcher {@link OnBackPressedDispatcher} that holds other callbacks. - * @param handlers {@link BackPressHandler} observing back press state and consuming back press. + * @param handlers {@link BackPressHandler} observing back press state and consuming back + * press. + * @param activity {@link SecondaryActivity} handling the back press. */ public static void create(LifecycleOwner lifecycleOwner, OnBackPressedDispatcher dispatcher, - BackPressHandler[] handlers) { + BackPressHandler[] handlers, @SecondaryActivity int activity) { // OnBackPressedDispatcher triggers handlers in a reversed order. for (int i = handlers.length - 1; i >= 0; i--) { - create(lifecycleOwner, dispatcher, handlers[i]); + create(lifecycleOwner, dispatcher, handlers[i], activity); } } @@ -95,11 +103,15 @@ public static void onBackPressed( } private BackPressHelper(LifecycleOwner lifecycleOwner, OnBackPressedDispatcher dispatcher, - ObsoleteBackPressedHandler handler) { + ObsoleteBackPressedHandler handler, @SecondaryActivity int activity) { dispatcher.addCallback(lifecycleOwner, new OnBackPressedCallback(true) { @Override public void handleOnBackPressed() { - if (!handler.onBackPressed()) onBackPressed(dispatcher, this); + if (handler.onBackPressed()) { + SecondaryActivityBackPressUma.record(activity); + } else { + onBackPressed(dispatcher, this); + } } }); } diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/BackPressHelperUnitTest.java b/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/BackPressHelperUnitTest.java similarity index 71% rename from chrome/android/junit/src/org/chromium/chrome/browser/BackPressHelperUnitTest.java rename to chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/BackPressHelperUnitTest.java index 7fbed96df7103..eacbfce462e3c 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/BackPressHelperUnitTest.java +++ b/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/BackPressHelperUnitTest.java @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -package org.chromium.chrome.browser; +package org.chromium.chrome.browser.back_press; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; @@ -21,9 +21,11 @@ import org.robolectric.annotation.Config; import org.chromium.base.test.BaseRobolectricTestRunner; +import org.chromium.base.test.util.HistogramWatcher; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; /** - * Unit tests for {@link org.chromium.chrome.browser.BackPressHelper}. + * Unit tests for {@link BackPressHelper}. */ @RunWith(BaseRobolectricTestRunner.class) @Config(manifest = Config.NONE) @@ -53,37 +55,54 @@ public void testNextCallbackNotInvokedIfAlreadyConsumed() { doReturn(true).when(mBackPressedHandler).onBackPressed(); // The last-added one is invoked first: mBackPressedHandler -> mBackPressedHandler2 - BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler2); - BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler2, + SecondaryActivity.DOWNLOAD); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler, + SecondaryActivity.HISTORY); + + var histogramWatcher = HistogramWatcher.newSingleRecordWatcher( + "Android.BackPress.SecondaryActivity", SecondaryActivity.HISTORY); mLifecycle.setCurrentState(Lifecycle.State.STARTED); mOnBackPressedDispatcher.onBackPressed(); verify(mBackPressedHandler).onBackPressed(); verify(mBackPressedHandler2, never()).onBackPressed(); verify(mFallbackRunnable, never()).run(); + histogramWatcher.assertExpected(); } @Test public void testInvokeNextCallbackIfNotConsumed() { doReturn(false).when(mBackPressedHandler).onBackPressed(); doReturn(true).when(mBackPressedHandler2).onBackPressed(); - BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler2); - BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler2, + SecondaryActivity.DOWNLOAD); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler, + SecondaryActivity.HISTORY); + + var histogramWatcher = HistogramWatcher.newSingleRecordWatcher( + "Android.BackPress.SecondaryActivity", SecondaryActivity.DOWNLOAD); mLifecycle.setCurrentState(Lifecycle.State.STARTED); mOnBackPressedDispatcher.onBackPressed(); verify(mBackPressedHandler).onBackPressed(); verify(mBackPressedHandler2).onBackPressed(); verify(mFallbackRunnable, never()).run(); + histogramWatcher.assertExpected(); } @Test public void testInvokeFallbackRunnableIfNotHandled() { doReturn(false).when(mBackPressedHandler).onBackPressed(); - BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler); + BackPressHelper.create(mLifecycleOwner, mOnBackPressedDispatcher, mBackPressedHandler, + SecondaryActivity.HISTORY); mLifecycle.setCurrentState(Lifecycle.State.STARTED); mOnBackPressedDispatcher.onBackPressed(); + var histogramWatcher = HistogramWatcher.newBuilder() + .expectNoRecords("Android.BackPress.SecondaryActivity") + .build(); verify(mFallbackRunnable).run(); + histogramWatcher.assertExpected(); } } diff --git a/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/SecondaryActivityBackPressUma.java b/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/SecondaryActivityBackPressUma.java new file mode 100644 index 0000000000000..038a3de970efa --- /dev/null +++ b/chrome/browser/back_press/android/java/src/org/chromium/chrome/browser/back_press/SecondaryActivityBackPressUma.java @@ -0,0 +1,37 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.back_press; + +import androidx.annotation.IntDef; + +import org.chromium.base.metrics.RecordHistogram; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Uma recorder for secondary activity back press handling. + */ +public class SecondaryActivityBackPressUma { + @IntDef({SecondaryActivity.DOWNLOAD, SecondaryActivity.BOOKMARK, SecondaryActivity.FIRST_RUN, + SecondaryActivity.LIGHTWEIGHT_FIRST_RUN, SecondaryActivity.HISTORY, + SecondaryActivity.SETTINGS, SecondaryActivity.NUM_TYPES}) + @Retention(RetentionPolicy.SOURCE) + public @interface SecondaryActivity { + int DOWNLOAD = 0; + int BOOKMARK = 1; + int FIRST_RUN = 2; + int LIGHTWEIGHT_FIRST_RUN = 3; + int HISTORY = 4; + int SETTINGS = 5; + + int NUM_TYPES = 6; + } + + public static void record(@SecondaryActivity int activity) { + RecordHistogram.recordEnumeratedHistogram( + "Android.BackPress.SecondaryActivity", activity, SecondaryActivity.NUM_TYPES); + } +} diff --git a/chrome/browser/download/internal/android/java/src/org/chromium/chrome/browser/download/home/DownloadActivityV2Test.java b/chrome/browser/download/internal/android/java/src/org/chromium/chrome/browser/download/home/DownloadActivityV2Test.java index 9cf7c21aece72..8b9e6b536ce33 100644 --- a/chrome/browser/download/internal/android/java/src/org/chromium/chrome/browser/download/home/DownloadActivityV2Test.java +++ b/chrome/browser/download/internal/android/java/src/org/chromium/chrome/browser/download/home/DownloadActivityV2Test.java @@ -54,9 +54,11 @@ import org.chromium.base.test.util.Batch; import org.chromium.base.test.util.CriteriaHelper; import org.chromium.base.test.util.DisabledTest; +import org.chromium.base.test.util.HistogramWatcher; import org.chromium.base.test.util.JniMocker; -import org.chromium.chrome.browser.BackPressHelper; +import org.chromium.chrome.browser.back_press.BackPressHelper; import org.chromium.chrome.browser.back_press.BackPressManager; +import org.chromium.chrome.browser.back_press.SecondaryActivityBackPressUma.SecondaryActivity; import org.chromium.chrome.browser.download.home.list.ListUtils; import org.chromium.chrome.browser.download.home.list.holder.ListItemViewHolder; import org.chromium.chrome.browser.download.home.rename.RenameUtils; @@ -202,10 +204,10 @@ private void setUpUi() { getActivity().setContentView(mDownloadCoordinator.getView()); if (BackPressManager.isSecondaryActivityEnabled()) { BackPressHelper.create(getActivity(), getActivity().getOnBackPressedDispatcher(), - mDownloadCoordinator.getBackPressHandlers()); + mDownloadCoordinator.getBackPressHandlers(), SecondaryActivity.DOWNLOAD); } else { BackPressHelper.create(getActivity(), getActivity().getOnBackPressedDispatcher(), - mDownloadCoordinator::onBackPressed); + mDownloadCoordinator::onBackPressed, SecondaryActivity.DOWNLOAD); } mDownloadCoordinator.updateForUrl(UrlConstants.DOWNLOADS_URL); @@ -668,20 +670,26 @@ public void testDismissSearchViewByBackPress() { onView(withId(R.id.search_text)).check(matches(not(isDisplayed()))); // Clear the selection by back press and assert that the search view is showing again. + var backPressRecorder = HistogramWatcher.newSingleRecordWatcher( + "Android.BackPress.SecondaryActivity", SecondaryActivity.DOWNLOAD); TestThreadUtils.runOnUiThreadBlocking( getActivity().getOnBackPressedDispatcher()::onBackPressed); + backPressRecorder.assertExpected(); onView(withId(R.id.search_text)).check(matches(isDisplayed())); // Close the search view, by performing a back press. + var backPressRecorder2 = HistogramWatcher.newSingleRecordWatcher( + "Android.BackPress.SecondaryActivity", SecondaryActivity.DOWNLOAD); TestThreadUtils.runOnUiThreadBlocking( getActivity().getOnBackPressedDispatcher()::onBackPressed); + backPressRecorder2.assertExpected(); CriteriaHelper.pollInstrumentationThread( () -> { onView(withId(R.id.search_text)).check(matches(not(isDisplayed()))); }); } @Test @MediumTest - @Features.DisableFeatures({ChromeFeatureList.BACK_GESTURE_REFACTOR_ACTIVITY}) + @Features.EnableFeatures({ChromeFeatureList.BACK_GESTURE_REFACTOR_ACTIVITY}) public void testDismissSearchViewByBackPress_BackPressRefactor() { testDismissSearchViewByBackPress(); } diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index 112acc04f2a61..702d848e6d6e3 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -93894,6 +93894,15 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf + + + + + + + + + diff --git a/tools/metrics/histograms/metadata/android/histograms.xml b/tools/metrics/histograms/metadata/android/histograms.xml index fc54d110f7c06..7abfe31f8712a 100644 --- a/tools/metrics/histograms/metadata/android/histograms.xml +++ b/tools/metrics/histograms/metadata/android/histograms.xml @@ -585,6 +585,16 @@ chromium-metrics-reviews@google.com. + + lazzzis@chromium.org + src/chrome/browser/back_press/android/OWNERS + + Records the secondary activity which intercepts the back press when the user + does the Backpress gesture and it's not handled by the system. + + + ckitagawa@chromium.org diff --git a/ui/android/java/src/org/chromium/ui/widget/ToastManager.java b/ui/android/java/src/org/chromium/ui/widget/ToastManager.java index 0513cdb15382f..ca26ef8ec4649 100644 --- a/ui/android/java/src/org/chromium/ui/widget/ToastManager.java +++ b/ui/android/java/src/org/chromium/ui/widget/ToastManager.java @@ -52,7 +52,7 @@ private interface ToastEvent { private Toast mToast; static boolean isEnabled() { - if (Boolean.FALSE.equals(sIsEnabled) || !LibraryLoader.getInstance().isInitialized()) { + if (Boolean.FALSE.equals(sIsEnabled) || !LibraryLoader.getInstance().isLoaded()) { return false; } if (sIsEnabled == null) {