From 249dbfea33a4b49cde700cecec7bb43003e2be03 Mon Sep 17 00:00:00 2001 From: Carmelo Messina Date: Wed, 22 Jan 2025 14:24:41 +0100 Subject: [PATCH] Restore adaptive-button-in-top-toolbar-customization: patch simplified --- ...-button-in-top-toolbar-customization.patch | 280 +++++------------- 1 file changed, 72 insertions(+), 208 deletions(-) diff --git a/build/patches/Restore-adaptive-button-in-top-toolbar-customization.patch b/build/patches/Restore-adaptive-button-in-top-toolbar-customization.patch index 3b64820f..64751b2b 100644 --- a/build/patches/Restore-adaptive-button-in-top-toolbar-customization.patch +++ b/build/patches/Restore-adaptive-button-in-top-toolbar-customization.patch @@ -7,187 +7,59 @@ Voice button and legacy share/voice functionality is not restored. License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html --- - .../chrome/browser/settings/MainSettings.java | 3 +- - .../flags/android/chrome_feature_list.cc | 4 ++ - .../browser/flags/ChromeFeatureList.java | 1 + - .../AdaptiveToolbarButtonController.java | 9 +++ - .../adaptive/AdaptiveToolbarFeatures.java | 65 ++++++++++++++++++- - .../adaptive/AdaptiveToolbarPrefs.java | 2 +- - .../AdaptiveToolbarStatePredictor.java | 9 ++- - .../AdaptiveToolbarStatePredictorTest.java | 15 +++++ - ...oButtonGroupAdaptiveToolbarPreference.java | 1 + - ...ve-button-in-top-toolbar-customization.inc | 1 + - 10 files changed, 105 insertions(+), 5 deletions(-) + .../chrome/browser/settings/MainSettings.java | 10 +--------- + .../segmentation_platform_config.cc | 1 + + .../adaptive/AdaptiveToolbarButtonController.java | 2 +- + .../toolbar/adaptive/AdaptiveToolbarPrefs.java | 2 +- + .../adaptive/AdaptiveToolbarStatePredictor.java | 4 ++++ + .../RadioButtonGroupAdaptiveToolbarPreference.java | 11 ++--------- + ...e-adaptive-button-in-top-toolbar-customization.inc | 1 + + 7 files changed, 11 insertions(+), 20 deletions(-) create mode 100644 cromite_flags/chrome/browser/flags/android/chrome_feature_list_cc/Restore-adaptive-button-in-top-toolbar-customization.inc diff --git a/chrome/android/java/src/org/chromium/chrome/browser/settings/MainSettings.java b/chrome/android/java/src/org/chromium/chrome/browser/settings/MainSettings.java --- a/chrome/android/java/src/org/chromium/chrome/browser/settings/MainSettings.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/settings/MainSettings.java -@@ -59,6 +59,7 @@ import org.chromium.chrome.browser.sync.settings.SyncSettingsUtils; - import org.chromium.chrome.browser.tab_group_sync.TabGroupSyncFeatures; - import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities; - import org.chromium.chrome.browser.toolbar.ToolbarPositionController; -+import org.chromium.chrome.browser.toolbar.adaptive.AdaptiveToolbarFeatures; - import org.chromium.chrome.browser.toolbar.adaptive.AdaptiveToolbarStatePredictor; - import org.chromium.chrome.browser.tracing.settings.DeveloperSettings; - import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager; -@@ -306,7 +307,7 @@ public class MainSettings extends ChromeBaseSettingsFragment - uiState -> { - // We don't show the toolbar shortcut settings page if disabled from - // finch. +@@ -301,15 +301,7 @@ public class MainSettings extends ChromeBaseSettingsFragment + templateUrlService.load(); + } + +- new AdaptiveToolbarStatePredictor(getContext(), getProfile(), null) +- .recomputeUiState( +- uiState -> { +- // We don't show the toolbar shortcut settings page if disabled from +- // finch. - if (uiState.canShowUi) return; -+ if (uiState.canShowUi && !AdaptiveToolbarFeatures.isSingleVariantModeEnabled()) return; - getPreferenceScreen() - .removePreference(findPreference(PREF_TOOLBAR_SHORTCUT)); - }); -diff --git a/chrome/browser/flags/android/chrome_feature_list.cc b/chrome/browser/flags/android/chrome_feature_list.cc ---- a/chrome/browser/flags/android/chrome_feature_list.cc -+++ b/chrome/browser/flags/android/chrome_feature_list.cc -@@ -429,6 +429,10 @@ static jlong JNI_ChromeFeatureMap_GetNativeMap(JNIEnv* env) { +- getPreferenceScreen() +- .removePreference(findPreference(PREF_TOOLBAR_SHORTCUT)); +- }); ++ new AdaptiveToolbarStatePredictor(getContext(), getProfile(), null); - // Alphabetical: + if (BuildInfo.getInstance().isAutomotive) { + getPreferenceScreen().removePreference(findPreference(PREF_SAFETY_CHECK)); +diff --git a/chrome/browser/segmentation_platform/segmentation_platform_config.cc b/chrome/browser/segmentation_platform/segmentation_platform_config.cc +--- a/chrome/browser/segmentation_platform/segmentation_platform_config.cc ++++ b/chrome/browser/segmentation_platform/segmentation_platform_config.cc +@@ -71,6 +71,7 @@ constexpr int kAdaptiveToolbarDefaultSelectionTTLDays = 56; -+CROMITE_FEATURE(kAdaptiveButtonInTopToolbar, -+ "AdaptiveButtonInTopToolbar", -+ base::FEATURE_ENABLED_BY_DEFAULT); -+ - BASE_FEATURE(kAdaptiveButtonInTopToolbarCustomizationV2, - "AdaptiveButtonInTopToolbarCustomizationV2", - base::FEATURE_ENABLED_BY_DEFAULT); -diff --git a/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java b/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java ---- a/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java -+++ b/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java -@@ -156,6 +156,7 @@ public abstract class ChromeFeatureList { - public static final String ACCOUNT_REAUTHENTICATION_RECENT_TIME_WINDOW = - "AccountReauthenticationRecentTimeWindow"; - public static final String ALLOW_USER_CERTIFICATES = "AllowUserCertificates"; -+ public static final String ADAPTIVE_BUTTON_IN_TOP_TOOLBAR = "AdaptiveButtonInTopToolbar"; - public static final String ADAPTIVE_BUTTON_IN_TOP_TOOLBAR_PAGE_SUMMARY = - "AdaptiveButtonInTopToolbarPageSummary"; - public static final String ADAPTIVE_BUTTON_IN_TOP_TOOLBAR_CUSTOMIZATION_V2 = + #if BUILDFLAG(IS_ANDROID) + std::unique_ptr GetConfigForAdaptiveToolbar() { ++ if ((true)) return nullptr; + if (!base::FeatureList::IsEnabled( + chrome::android::kAdaptiveButtonInTopToolbarCustomizationV2)) { + return nullptr; diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarButtonController.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarButtonController.java --- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarButtonController.java +++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarButtonController.java -@@ -276,6 +276,15 @@ public class AdaptiveToolbarButtonController - new AdaptiveToolbarStatePredictor(mContext, profile, mAndroidPermissionDelegate); - ContextUtils.getAppSharedPreferences().registerOnSharedPreferenceChangeListener(this); - -+ if (AdaptiveToolbarFeatures.isSingleVariantModeEnabled()) { -+ @AdaptiveToolbarButtonVariant -+ int variant = AdaptiveToolbarFeatures.getSingleVariantMode(); -+ setSingleProvider(variant); -+ -+ mOriginalButtonSpec = null; -+ notifyObservers(mButtonData.canShow()); -+ return; -+ } - if (!AdaptiveToolbarFeatures.isCustomizationEnabled()) return; - mAdaptiveToolbarStatePredictor.recomputeUiState(mUiStateCallback); - AdaptiveToolbarStats.recordSelectedSegmentFromSegmentationPlatformAsync( -diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarFeatures.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarFeatures.java ---- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarFeatures.java -+++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarFeatures.java -@@ -18,8 +18,18 @@ import org.chromium.ui.base.DeviceFormFactor; - import java.util.HashMap; - import java.util.List; - --/** A utility class for handling feature flags used by {@link AdaptiveToolbarButtonController}. */ -+/** A utility class for handling feature flags used by {@link AdaptiveToolbarButtonController}. * -+ *

TODO(shaktisahu): This class supports both the data collection and the customization -+ * experiment. Cleanup once the former is no longer needed. -+ */ - public class AdaptiveToolbarFeatures { -+ /** Adaptive toolbar button is always empty. */ -+ public static final String ALWAYS_NONE = "always-none"; -+ /** Adaptive toolbar button opens a new tab. */ -+ public static final String ALWAYS_NEW_TAB = "always-new-tab"; -+ /** Adaptive toolbar button shares the current tab. */ -+ public static final String ALWAYS_SHARE = "always-share"; -+ - /** Finch default group for new tab variation. */ - static final String NEW_TAB = "new-tab"; - -@@ -51,6 +61,7 @@ public class AdaptiveToolbarFeatures { - @AdaptiveToolbarButtonVariant private static Integer sButtonVariant; - - /** For testing only. */ -+ private static final String VARIATION_PARAM_SINGLE_VARIANT_MODE = "mode"; - private static String sDefaultSegmentForTesting; - - private static HashMap sActionChipOverridesForTesting; -@@ -83,6 +94,21 @@ public class AdaptiveToolbarFeatures { - return false; +@@ -297,7 +297,7 @@ public class AdaptiveToolbarButtonController } -+ /** -+ * Returns whether the adaptive toolbar is enabled in single variant mode. Returns true also to -+ * provide legacy support for feature flags {@code ShareButtonInTopToolbar} and {@code -+ * VoiceButtonInTopToolbar}. -+ * -+ *

Must be called with the {@link FeatureList} initialized. -+ */ -+ public static boolean isSingleVariantModeEnabled() { -+ if (isCustomizationEnabled()) return false; -+ if (ChromeFeatureList.isEnabled(ChromeFeatureList.ADAPTIVE_BUTTON_IN_TOP_TOOLBAR)) { -+ return true; -+ } -+ return false; -+ } -+ - /** - * Returns whether the adaptive toolbar is enabled with segmentation and customization. - * -@@ -214,6 +240,42 @@ public class AdaptiveToolbarFeatures { - return segmentationResults.get(0); + private boolean isScreenWideEnoughForButton() { +- return mScreenWidthDp >= AdaptiveToolbarFeatures.getDeviceMinimumWidthForShowingButton(); ++ return true; } -+ /** -+ * When the adaptive toolbar is configured in a single button variant mode, returns the {@link -+ * AdaptiveToolbarButtonVariant} being used. -+ * -+ *

This methods avoids parsing param strings more than once. Tests need to call {@link -+ * #clearParsedParamsForTesting()} to clear the cached values. -+ * -+ *

Must be called with the {@link FeatureList} initialized. -+ * -+ *

TODO(shaktisahu): Have a similar method for segmentation. -+ */ -+ @AdaptiveToolbarButtonVariant -+ public static int getSingleVariantMode() { -+ assert isSingleVariantModeEnabled(); -+ if (sButtonVariant != null) return sButtonVariant; -+ -+ String mode = ChromeFeatureList.getFieldTrialParamByFeature( -+ ChromeFeatureList.ADAPTIVE_BUTTON_IN_TOP_TOOLBAR, -+ VARIATION_PARAM_SINGLE_VARIANT_MODE); -+ switch (mode) { -+ case ALWAYS_NONE: -+ sButtonVariant = AdaptiveToolbarButtonVariant.NONE; -+ break; -+ case ALWAYS_NEW_TAB: -+ sButtonVariant = AdaptiveToolbarButtonVariant.NEW_TAB; -+ break; -+ case ALWAYS_SHARE: -+ sButtonVariant = AdaptiveToolbarButtonVariant.SHARE; -+ break; -+ default: -+ sButtonVariant = AdaptiveToolbarButtonVariant.UNKNOWN; -+ break; -+ } -+ return sButtonVariant; -+ } -+ - /** - * Returns the default variant to be shown in segmentation experiment when the backend results - * are unavailable or not configured. -@@ -221,6 +283,7 @@ public class AdaptiveToolbarFeatures { - * @param context Context to determine form-factor. - */ - static @AdaptiveToolbarButtonVariant int getSegmentationDefault(Context context) { -+ assert !isSingleVariantModeEnabled(); - assert isCustomizationEnabled(); - if (sButtonVariant != null) return sButtonVariant; - String defaultSegment = getDefaultSegment(context); + /** Returns the {@link ButtonDataProvider} used in a single-variant mode. */ diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarPrefs.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarPrefs.java --- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarPrefs.java +++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarPrefs.java @@ -203,49 +75,17 @@ diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/brow diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictor.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictor.java --- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictor.java +++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictor.java -@@ -99,10 +99,15 @@ public class AdaptiveToolbarStatePredictor { - - // Early return if the feature isn't enabled. - if (!AdaptiveToolbarFeatures.isCustomizationEnabled()) { -+ boolean canShowUi = AdaptiveToolbarFeatures.isSingleVariantModeEnabled(); -+ @AdaptiveToolbarButtonVariant -+ int toolbarButtonState = AdaptiveToolbarFeatures.isSingleVariantModeEnabled() -+ ? AdaptiveToolbarFeatures.getSingleVariantMode() -+ : AdaptiveToolbarButtonVariant.UNKNOWN; - callback.onResult( - new UiState( -- false, -- AdaptiveToolbarButtonVariant.UNKNOWN, -+ canShowUi, -+ toolbarButtonState, - AdaptiveToolbarButtonVariant.UNKNOWN, - AdaptiveToolbarButtonVariant.UNKNOWN)); +@@ -201,6 +201,10 @@ public class AdaptiveToolbarStatePredictor { + * @param callback A callback for results. + */ + public void readFromSegmentationPlatform(Callback> callback) { ++ if ((true)) { ++ callback.onResult(List.of(AdaptiveToolbarButtonVariant.UNKNOWN)); ++ return; ++ } + if (sSegmentationResultsForTesting != null) { + callback.onResult(sSegmentationResultsForTesting); return; -diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictorTest.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictorTest.java ---- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictorTest.java -+++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/AdaptiveToolbarStatePredictorTest.java -@@ -73,6 +73,21 @@ public class AdaptiveToolbarStatePredictorTest { - statePredictor.recomputeUiState(verifyResultCallback(expected)); - } - -+ @Test -+ @SmallTest -+ @EnableFeatures({ChromeFeatureList.ADAPTIVE_BUTTON_IN_TOP_TOOLBAR, -+ ChromeFeatureList.VOICE_SEARCH_AUDIO_CAPTURE_POLICY}) -+ @DisableFeatures({ChromeFeatureList.ADAPTIVE_BUTTON_IN_TOP_TOOLBAR_CUSTOMIZATION_V2}) -+ public void -+ testWorksWithDataCollectionFeatureFlag() { -+ ShadowChromeFeatureList.sParamValues.put("mode", "always-voice"); -+ AdaptiveToolbarStatePredictor statePredictor = buildStatePredictor( -+ true, AdaptiveToolbarButtonVariant.VOICE, true, AdaptiveToolbarButtonVariant.SHARE); -+ UiState expected = new UiState(true, AdaptiveToolbarButtonVariant.VOICE, -+ AdaptiveToolbarButtonVariant.UNKNOWN, AdaptiveToolbarButtonVariant.UNKNOWN); -+ statePredictor.recomputeUiState(verifyResultCallback(expected)); -+ } -+ - @Test - @SmallTest - public void testManualOverride() { diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/settings/RadioButtonGroupAdaptiveToolbarPreference.java b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/settings/RadioButtonGroupAdaptiveToolbarPreference.java --- a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/settings/RadioButtonGroupAdaptiveToolbarPreference.java +++ b/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/adaptive/settings/RadioButtonGroupAdaptiveToolbarPreference.java @@ -257,6 +97,30 @@ diff --git a/chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/brow mNewTabButton = (RadioButtonWithDescription) holder.findViewById(R.id.adaptive_option_new_tab); mShareButton = (RadioButtonWithDescription) holder.findViewById(R.id.adaptive_option_share); +@@ -64,6 +65,7 @@ public class RadioButtonGroupAdaptiveToolbarPreference extends Preference + (RadioButtonWithDescription) holder.findViewById(R.id.adaptive_option_voice_search); + mTranslateButton = + (RadioButtonWithDescription) holder.findViewById(R.id.adaptive_option_translate); ++ updateButtonVisibility(mTranslateButton, false); + mAddToBookmarksButton = + (RadioButtonWithDescription) + holder.findViewById(R.id.adaptive_option_add_to_bookmarks); +@@ -101,15 +103,6 @@ public class RadioButtonGroupAdaptiveToolbarPreference extends Preference + R.string + .adaptive_toolbar_button_preference_based_on_your_usage_description, + getButtonString(uiState.autoButtonCaption))); +- // Description to indicate these buttons only appear on small windows, +- // as large windows (tablets) show them elsewhere on UI (strip, omnibox). +- String basedOnWindowDesc = +- getContext() +- .getString( +- R.string +- .adaptive_toolbar_button_preference_based_on_window_width_description); +- mNewTabButton.setDescriptionText(basedOnWindowDesc); +- mAddToBookmarksButton.setDescriptionText(basedOnWindowDesc); + updateVoiceButtonVisibility(); + updateReadAloudButtonVisibility(); + updatePageSummaryButtonVisibility(); diff --git a/cromite_flags/chrome/browser/flags/android/chrome_feature_list_cc/Restore-adaptive-button-in-top-toolbar-customization.inc b/cromite_flags/chrome/browser/flags/android/chrome_feature_list_cc/Restore-adaptive-button-in-top-toolbar-customization.inc new file mode 100644 --- /dev/null