PawletOS/cromite
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity
Files
create-pull-request/patch-b8df90d
cromite/build/patches/Restore-adaptive-button-in-top-toolbar-customization.patch
T
Carmelo Messina b937cb60ec v123 patches
2024-03-15 16:24:42 +01:00

324 lines
17 KiB
Diff
Raw Permalink Blame History

From: csagan5 <32685696+csagan5@users.noreply.github.com>
Date: Wed, 7 Dec 2022 20:32:15 +0100
Subject: Restore adaptive-button-in-top-toolbar-customization
This reverts commit 18d03b9cca4e90d2a446ea28266876d8c5fdc4f0.
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 +-
.../browser/share/ShareButtonController.java | 37 +++++++++++
chrome/browser/about_flags.cc | 6 --
chrome/browser/flag-metadata.json | 3 +-
.../flags/android/chrome_feature_list.cc | 5 ++
.../browser/flags/ChromeFeatureList.java | 1 +
.../AdaptiveToolbarButtonController.java | 6 +-
.../adaptive/AdaptiveToolbarFeatures.java | 65 ++++++++++++++++++-
.../AdaptiveToolbarStatePredictor.java | 9 ++-
.../AdaptiveToolbarStatePredictorTest.java | 15 +++++
10 files changed, 138 insertions(+), 12 deletions(-)
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
@@ -46,6 +46,7 @@ import org.chromium.chrome.browser.sync.settings.ManageSyncSettings;
import org.chromium.chrome.browser.sync.settings.SignInPreference;
import org.chromium.chrome.browser.sync.settings.SyncPromoPreference;
import org.chromium.chrome.browser.sync.settings.SyncSettingsUtils;
+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.signin.SyncPromoController;
@@ -247,7 +248,7 @@ public class MainSettings extends ChromeBaseSettingsFragment
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/android/java/src/org/chromium/chrome/browser/share/ShareButtonController.java b/chrome/android/java/src/org/chromium/chrome/browser/share/ShareButtonController.java
--- a/chrome/android/java/src/org/chromium/chrome/browser/share/ShareButtonController.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/share/ShareButtonController.java
@@ -111,6 +111,43 @@ public class ShareButtonController extends BaseButtonDataProvider {
return mShareUtils.shouldEnableShare(tab);
}
+/* @Override
+ public ButtonData get(Tab tab) {
+ updateButtonVisibility(tab);
+ maybeSetIphCommandBuilder(tab);
+ return mButtonData;
+ }
+
+ private void updateButtonVisibility(Tab tab) {
+ if (tab == null || tab.getWebContents() == null || mTabProvider == null
+ || mTabProvider.get() == null || !isFeatureEnabled()) {
+ mButtonData.setCanShow(false);
+ return;
+ }
+
+ final boolean isDeviceWideEnough =
+ mScreenWidthDp >= AdaptiveToolbarFeatures.getDeviceMinimumWidthForShowingButton();
+ if (mShareDelegateSupplier.get() == null || !isDeviceWideEnough) {
+ mButtonData.setCanShow(false);
+ return;
+ }
+
+ mButtonData.setCanShow(mShareUtils.shouldEnableShare(tab));
+ }
+
+ private static boolean isFeatureEnabled() {
+ return (AdaptiveToolbarFeatures.isSingleVariantModeEnabled()
+ && AdaptiveToolbarFeatures.getSingleVariantMode()
+ == AdaptiveToolbarButtonVariant.SHARE)
+ || AdaptiveToolbarFeatures.isCustomizationEnabled();
+ }
+
+ private void notifyObservers(boolean hint) {
+ for (ButtonDataObserver observer : mObservers) {
+ observer.buttonDataChanged(hint);
+ }
+ } */
+
/**
* Returns an IPH for this button. Only called once native is initialized and when {@code
* AdaptiveToolbarFeatures.isCustomizationEnabled()} is true.
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -658,18 +658,12 @@ const FeatureEntry::FeatureParam kAdaptiveButtonCustomization_Share[] = {
{"default_segment", "share"},
{"show_ui_only_after_ready", "false"},
{"ignore_segmentation_results", "true"}};
-const FeatureEntry::FeatureParam kAdaptiveButtonCustomization_Voice[] = {
- {"default_segment", "voice"},
- {"show_ui_only_after_ready", "false"},
- {"ignore_segmentation_results", "true"}};
const FeatureEntry::FeatureVariation
kAdaptiveButtonInTopToolbarCustomizationVariations[] = {
{"New Tab", kAdaptiveButtonCustomization_NewTab,
std::size(kAdaptiveButtonCustomization_NewTab), nullptr},
{"Share", kAdaptiveButtonCustomization_Share,
std::size(kAdaptiveButtonCustomization_Share), nullptr},
- {"Voice", kAdaptiveButtonCustomization_Voice,
- std::size(kAdaptiveButtonCustomization_Voice), nullptr},
};
const FeatureEntry::FeatureParam kContextualPageActionsUiParams_Quiet[] = {
diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
--- a/chrome/browser/flag-metadata.json
+++ b/chrome/browser/flag-metadata.json
@@ -42,9 +42,10 @@
"expiry_milestone": 125
},
{
+ // restored in Bromite
"name": "adaptive-button-in-top-toolbar-customization",
"owners": [ "salg@google.com", "chrome-segmentation-platform@google.com" ],
- "expiry_milestone": 125
+ "expiry_milestone": -1
},
{
"name": "adaptive-button-in-top-toolbar-translate",
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
@@ -150,6 +150,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&history_clusters::internal::kJourneys,
&history_clusters::internal::kOmniboxAction,
&history_clusters::internal::kOmniboxHistoryClusterProvider,
+ &kAdaptiveButtonInTopToolbar,
&kAdaptiveButtonInTopToolbarTranslate,
&kAdaptiveButtonInTopToolbarAddToBookmarks,
&kAdaptiveButtonInTopToolbarCustomizationV2,
@@ -388,6 +389,10 @@ static jlong JNI_ChromeFeatureMap_GetNativeMap(JNIEnv* env) {
// Alphabetical:
+CROMITE_FEATURE(kAdaptiveButtonInTopToolbar,
+ "AdaptiveButtonInTopToolbar",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+
BASE_FEATURE(kAdaptiveButtonInTopToolbarTranslate,
"AdaptiveButtonInTopToolbarTranslate",
base::FEATURE_DISABLED_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
@@ -142,6 +142,7 @@ public abstract class ChromeFeatureList {
}
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_TRANSLATE =
"AdaptiveButtonInTopToolbarTranslate";
public static final String ADAPTIVE_BUTTON_IN_TOP_TOOLBAR_ADD_TO_BOOKMARKS =
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
@@ -253,7 +253,11 @@ public class AdaptiveToolbarButtonController
@Override
public void onFinishNativeInitialization() {
- if (AdaptiveToolbarFeatures.isCustomizationEnabled()) {
+ if (AdaptiveToolbarFeatures.isSingleVariantModeEnabled()) {
+ @AdaptiveToolbarButtonVariant
+ int variant = AdaptiveToolbarFeatures.getSingleVariantMode();
+ setSingleProvider(variant);
+ } else if (AdaptiveToolbarFeatures.isCustomizationEnabled()) {
mAdaptiveToolbarStatePredictor.recomputeUiState(
uiState -> {
mSessionButtonVariant =
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
@@ -17,8 +17,18 @@ import org.chromium.chrome.browser.readaloud.ReadAloudFeatures;
import java.util.HashMap;
-/** A utility class for handling feature flags used by {@link AdaptiveToolbarButtonController}. */
+/** A utility class for handling feature flags used by {@link AdaptiveToolbarButtonController}. *
+ * <p>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";
@@ -34,6 +44,7 @@ public class AdaptiveToolbarFeatures {
private static final String VARIATION_PARAM_DISABLE_UI = "disable_ui";
private static final String VARIATION_PARAM_IGNORE_SEGMENTATION_RESULTS =
"ignore_segmentation_results";
+ private static final String VARIATION_PARAM_SINGLE_VARIANT_MODE = "mode";
private static final String VARIATION_PARAM_SHOW_UI_ONLY_AFTER_READY =
"show_ui_only_after_ready";
@VisibleForTesting static final String VARIATION_PARAM_MIN_VERSION = "min_version_adaptive";
@@ -102,6 +113,21 @@ public class AdaptiveToolbarFeatures {
}
}
+ /**
+ * 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}.
+ *
+ * <p>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.
*
@@ -242,11 +268,48 @@ public class AdaptiveToolbarFeatures {
ChromeFeatureList.CONTEXTUAL_PAGE_ACTIONS, "enable_ui", true);
}
+ /**
+ * When the adaptive toolbar is configured in a single button variant mode, returns the {@link
+ * AdaptiveToolbarButtonVariant} being used.
+ *
+ * <p>This methods avoids parsing param strings more than once. Tests need to call {@link
+ * #clearParsedParamsForTesting()} to clear the cached values.
+ *
+ * <p>Must be called with the {@link FeatureList} initialized.
+ *
+ * <p>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.
*/
static @AdaptiveToolbarButtonVariant int getSegmentationDefault() {
+ assert !isSingleVariantModeEnabled();
assert isCustomizationEnabled();
if (sButtonVariant != null) return sButtonVariant;
String defaultSegment = getDefaultSegment();
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
@@ -87,10 +87,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));
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
@@ -72,6 +72,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() {
--
Reference in New Issue View Git Blame Copy Permalink