Skip to content

Commit

Permalink
Remove expired flags in flag-metadata.json
Browse files Browse the repository at this point in the history
Fixed: 1489874, 1490000, 1490020, 1450821
Change-Id: Ia498c60b0534bf3c61acecc5e8dceb69587a5da7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4931528
Reviewed-by: Samar Chehade-Lepleux <samarchehade@google.com>
Commit-Queue: Tanmoy Mollik <triploblastic@google.com>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1211415}
  • Loading branch information
Tanmoy Mollik authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent cc755b4 commit 0e16cec
Show file tree
Hide file tree
Showing 13 changed files with 15 additions and 176 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,6 @@ public void test_DoNotLoadLastSelectedTabOnStartupV2() {
@Test
@SmallTest
@Feature({"StartSurface"})
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
public void testRecordHistogramProfileButtonClick_StartSurface() {
Assume.assumeTrue(mImmediateReturn);
ChromeTabbedActivity cta = mActivityTestRule.getActivity();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.layouts.LayoutType;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
Expand Down Expand Up @@ -69,9 +68,6 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
// ProfileDataCache facilitates retrieving profile picture.
private ProfileDataCache mProfileDataCache;

// Whether the identity disc is visible.
private boolean mIsIdentityDiscShown;

private ButtonDataImpl mButtonData;
private ObserverList<ButtonDataObserver> mObservers = new ObserverList<>();
private boolean mNativeIsInitialized;
Expand Down Expand Up @@ -165,27 +161,17 @@ private void calculateButtonData() {
}

String email = CoreAccountInfo.getEmailFrom(getSignedInAccountInfo());
if (ChromeFeatureList.isEnabled(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)) {
mIsIdentityDiscShown = true;
} else {
mIsIdentityDiscShown = email != null;
}
ensureProfileDataCache(mIsIdentityDiscShown);
ensureProfileDataCache();

if (mIsIdentityDiscShown) {
mButtonData.setButtonSpec(
buttonSpecWithDrawableAndDescription(mButtonData.getButtonSpec(), email));
mButtonData.setCanShow(true);
} else {
mButtonData.setCanShow(false);
}
mButtonData.setButtonSpec(
buttonSpecWithDrawableAndDescription(mButtonData.getButtonSpec(), email));
mButtonData.setCanShow(true);
}

private ButtonSpec buttonSpecWithDrawableAndDescription(
ButtonSpec buttonSpec, @Nullable String email) {
Drawable drawable = getProfileImage(email);
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
&& (buttonSpec.getDrawable() == drawable)) {
if (buttonSpec.getDrawable() == drawable) {
return buttonSpec;
}

Expand All @@ -201,8 +187,8 @@ private ButtonSpec buttonSpecWithDrawableAndDescription(
* Creates and initializes ProfileDataCache if it wasn't created previously. Subscribes
* IdentityDiscController for profile data updates.
*/
private void ensureProfileDataCache(boolean isIdentityDiscShown) {
if (!isIdentityDiscShown || mProfileDataCache != null) return;
private void ensureProfileDataCache() {
if (mProfileDataCache != null) return;

mProfileDataCache =
ProfileDataCache.createWithoutBadge(mContext, R.dimen.toolbar_identity_disc_size);
Expand All @@ -213,8 +199,7 @@ private void ensureProfileDataCache(boolean isIdentityDiscShown) {
* Returns Profile picture Drawable. The size of the image corresponds to current visual state.
*/
private Drawable getProfileImage(@Nullable String email) {
assert mIsIdentityDiscShown;
if (shouldUseSignedOutAvatar(email)) {
if (email == null) {
return AppCompatResources.getDrawable(mContext, R.drawable.account_circle);
}
return mProfileDataCache.getProfileDataOrDefault(email).getImage();
Expand All @@ -226,7 +211,6 @@ private Drawable getProfileImage(@Nullable String email) {
*/
private void resetIdentityDiscCache() {
if (mProfileDataCache != null) {
assert mIsIdentityDiscShown;
mProfileDataCache.removeObserver(this);
mProfileDataCache = null;
}
Expand All @@ -243,7 +227,6 @@ private void notifyObservers(boolean hint) {
*/
@Override
public void onProfileDataUpdated(String accountEmail) {
if (!mIsIdentityDiscShown) return;
assert mProfileDataCache != null;

if (accountEmail.equals(CoreAccountInfo.getEmailFrom(getSignedInAccountInfo()))) {
Expand All @@ -261,8 +244,8 @@ public void onProfileDataUpdated(String accountEmail) {
/**
* Implements {@link IdentityManager.Observer}.
*
* IdentityDisc should be shown as long as the user is signed in or IDENTITY_STATUS_CONSISTENCY
* is enabled. Whether the user is syncing or not should not matter.
* <p>IdentityDisc should be always shown regardless of whether the user is signed out, signed
* in or syncing.
*/
@Override
public void onPrimaryAccountChanged(PrimaryAccountChangeEvent eventDetails) {
Expand Down Expand Up @@ -348,10 +331,6 @@ private void setProfile(Profile profile) {
}

private String getContentDescription(@Nullable String email) {
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)) {
return mContext.getString(R.string.accessibility_toolbar_btn_identity_disc);
}

if (email == null) {
return mContext.getString(R.string.accessibility_toolbar_btn_signed_out_identity_disc);
}
Expand Down Expand Up @@ -394,11 +373,4 @@ void onClick() {
boolean isProfileDataCacheEmpty() {
return mProfileDataCache == null;
}

private static boolean shouldUseSignedOutAvatar(@Nullable String email) {
boolean isIdentityStatusConsistencyEnabled =
ChromeFeatureList.isEnabled(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY);
boolean isUserSignedOut = email == null;
return isIdentityStatusConsistencyEnabled && isUserSignedOut;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import android.view.View;

import androidx.test.espresso.matcher.ViewMatchers;
import androidx.test.filters.MediumTest;
import androidx.test.filters.SmallTest;
import androidx.test.platform.app.InstrumentationRegistry;

import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -42,7 +39,6 @@
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.profiles.Profile;
Expand All @@ -59,8 +55,6 @@
import org.chromium.chrome.test.util.ActivityTestUtils;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.NewTabPageTestUtils;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.chrome.test.util.browser.signin.SigninTestRule;
import org.chromium.chrome.test.util.browser.signin.SigninTestUtil;
import org.chromium.components.embedder_support.util.UrlConstants;
Expand Down Expand Up @@ -149,46 +143,7 @@ public void testIdentityDiscWithNavigation() {

@Test
@MediumTest
@DisableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
@SuppressWarnings("CheckReturnValue")
public void testIdentityDiscWithSignin() {
// When user is signed out and IdentityStatusConsistency is disabled, Identity Disc should
// not be visible on the NTP.
onView(withId(R.id.optional_toolbar_button))
.check(
(view, noViewException) -> {
if (view != null) {
ViewMatchers.assertThat(
"IdentityDisc view should be gone if it exists",
view.getVisibility(),
Matchers.is(View.GONE));
}
});

// Identity Disc should be shown on sign-in state change with a NTP refresh.
mSigninTestRule.addTestAccountThenSignin();
// TODO(https://crbug.com/1132291): Remove the reload once the sign-in without sync observer
// is implemented.
TestThreadUtils.runOnUiThreadBlocking(mTab::reload);
// TODO(crbug.com/1469988): This is a no-op, replace with ViewUtils.waitForVisibleView().
ViewUtils.isEventuallyVisible(
allOf(
withId(R.id.optional_toolbar_button),
isDisplayed(),
withContentDescription(R.string.accessibility_toolbar_btn_identity_disc)));

mSigninTestRule.signOut();
// TODO(crbug.com/1469988): This is a no-op, replace with ViewUtils.waitForVisibleView().
ViewUtils.isEventuallyVisible(
allOf(
withId(R.id.optional_toolbar_button),
withEffectiveVisibility(ViewMatchers.Visibility.GONE)));
}

@Test
@MediumTest
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
public void testIdentityDiscSignedOut_identityStatusConsistencyEnabled() {
public void testIdentityDiscSignedOut() {
// When user is signed out, a signed-out avatar should be visible on the NTP.
ViewUtils.waitForVisibleView(
allOf(
Expand All @@ -206,9 +161,7 @@ public void testIdentityDiscSignedOut_identityStatusConsistencyEnabled() {

@Test
@MediumTest
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
public void
testIdentityDiscSignedOut_signinDisabledByPolicy_identityStatusConsistencyEnabled() {
public void testIdentityDiscSignedOut_signinDisabledByPolicy() {
IdentityServicesProvider.setInstanceForTests(mIdentityServicesProviderMock);
TestThreadUtils.runOnUiThreadBlocking(
() -> {
Expand Down Expand Up @@ -240,8 +193,7 @@ public void testIdentityDiscSignedOut_identityStatusConsistencyEnabled() {

@Test
@MediumTest
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
public void testIdentityDiscWithSignin_identityStatusConsistencyEnabled() {
public void testIdentityDiscWithSignin() {
// Identity Disc should be shown on sign-in state change with a NTP refresh.
mSigninTestRule.addAccountThenSignin(EMAIL, NAME);
// TODO(https://crbug.com/1132291): Remove the reload once the sign-in without sync observer
Expand Down Expand Up @@ -271,8 +223,7 @@ public void testIdentityDiscWithSignin_identityStatusConsistencyEnabled() {

@Test
@MediumTest
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
public void testIdentityDiscWithSignin_nonDisplayableEmail_identityStatusConsistencyEnabled() {
public void testIdentityDiscWithSignin_nonDisplayableEmail() {
// Identity Disc should be shown on sign-in state change with a NTP refresh.
CoreAccountInfo coreAccountInfo = addAccountWithNonDisplayableEmail(NAME);
SigninTestUtil.signin(coreAccountInfo);
Expand Down Expand Up @@ -302,44 +253,8 @@ public void testIdentityDiscWithSignin_nonDisplayableEmail_identityStatusConsist

@Test
@MediumTest
@DisableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
@SuppressWarnings("CheckReturnValue")
public void testIdentityDiscWithSigninAndEnableSync() {
// When user is signed out and IdentityStatusConsistency is disabled, Identity Disc should
// not be visible on the NTP.
onView(withId(R.id.optional_toolbar_button))
.check(
(view, noViewException) -> {
if (view != null) {
ViewMatchers.assertThat(
"IdentityDisc view should be gone if it exists",
view.getVisibility(),
Matchers.is(View.GONE));
}
});

// Identity Disc should be shown on sign-in state change without NTP refresh.
mSigninTestRule.addTestAccountThenSigninAndEnableSync();
// TODO(crbug.com/1469988): This is a no-op, replace with ViewUtils.waitForVisibleView().
ViewUtils.isEventuallyVisible(
allOf(
withId(R.id.optional_toolbar_button),
withContentDescription(R.string.accessibility_toolbar_btn_identity_disc),
isDisplayed()));

mSigninTestRule.signOut();
// TODO(crbug.com/1469988): This is a no-op, replace with ViewUtils.waitForVisibleView().
ViewUtils.isEventuallyVisible(
allOf(
withId(R.id.optional_toolbar_button),
withEffectiveVisibility(ViewMatchers.Visibility.GONE)));
}

@Test
@MediumTest
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
@SuppressWarnings("CheckReturnValue")
public void testIdentityDiscWithSigninAndEnableSync_identityStatusConsistencyEnabled() {
// Identity Disc should be shown on sign-in state change without NTP refresh.
mSigninTestRule.addAccountThenSigninAndEnableSync(EMAIL, NAME);
String expectedContentDescription =
Expand Down Expand Up @@ -369,9 +284,7 @@ public void testIdentityDiscWithSigninAndEnableSync_identityStatusConsistencyEna

@Test
@MediumTest
@EnableFeatures(ChromeFeatureList.IDENTITY_STATUS_CONSISTENCY)
public void
testIdentityDiscWithSigninAndEnableSync_nonDisplayableEmail_identityStatusConsistencyEnabled() {
public void testIdentityDiscWithSigninAndEnableSync_nonDisplayableEmail() {
// Identity Disc should be shown on sign-in state change without NTP refresh.
CoreAccountInfo coreAccountInfo = addAccountWithNonDisplayableEmail(NAME);
SigninTestUtil.signinAndEnableSync(
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7502,11 +7502,6 @@ const FeatureEntry kFeatureEntries[] = {
FEATURE_WITH_PARAMS_VALUE_TYPE(switches::kTangibleSync,
kTangibleSyncVariations,
"TangibleSyncVariations")},

{"identity-status-consistency",
flag_descriptions::kIdentityStatusConsistencyName,
flag_descriptions::kIdentityStatusConsistencyDescription, kOsAndroid,
FEATURE_VALUE_TYPE(switches::kIdentityStatusConsistency)},
#endif // BUILDFLAG(IS_ANDROID)

{"gainmap-hdr-images", flag_descriptions::kGainmapHdrImagesName,
Expand Down
15 changes: 0 additions & 15 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -2230,11 +2230,6 @@
"owners": [ "jophba", "openscreen-eng" ],
"expiry_milestone": 122
},
{
"name": "enable-cbd-sign-out",
"owners": [ "triploblastic", "chrome-signin-team", "arthurmilchior"],
"expiry_milestone":120
},
{
"name": "enable-cct-text-fragment-lookup-api",
"owners": [ "tbansal", "chrome-intelligence-core"],
Expand Down Expand Up @@ -4653,11 +4648,6 @@
"owners": [ "nigeltao", "simmonsjosh@google.com" ],
"expiry_milestone": 126
},
{
"name": "gaia-id-in-amf",
"owners": [ "triploblastic", "chrome-signin-team" ],
"expiry_milestone":120
},
{
"name": "gainmap-hdr-images",
"owners": [ "ccameron", "chrome-color@google.com" ],
Expand Down Expand Up @@ -4847,11 +4837,6 @@
],
"expiry_milestone": 122
},
{
"name": "identity-status-consistency",
"owners": [ "triploblastic", "chrome-signin-team" ],
"expiry_milestone":120
},
{
"name": "ignore-gpu-blocklist",
"owners": [ "kbr", "zmo" ],
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,6 @@ const char kAvifGainmapHdrImagesDescription[] =
"Chrome uses the gainmap (if present) in AVIF images to render the HDR "
"version on HDR displays and the SDR version on SDR displays.";

const char kIdentityStatusConsistencyName[] = "Identity Status Consistency";
const char kIdentityStatusConsistencyDescription[] =
"If enabled, always show identity status - even for signed-out users";

const char kTangibleSyncName[] = "Tangible Sync";
const char kTangibleSyncDescription[] =
"Enables the tangible sync when a user starts the sync consent flow";
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,6 @@ extern const char kGainmapHdrImagesDescription[];
extern const char kAvifGainmapHdrImagesName[];
extern const char kAvifGainmapHdrImagesDescription[];

extern const char kIdentityStatusConsistencyName[];
extern const char kIdentityStatusConsistencyDescription[];

extern const char kTangibleSyncName[];
extern const char kTangibleSyncDescription[];

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,6 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&supervised_user::kEnableProtoApiForClassifyUrl,
&supervised_user::kKidFriendlyContentFeed,
&switches::kForceStartupSigninPromo,
&switches::kIdentityStatusConsistency,
&switches::kForceDisableExtendedSyncPromos,
&switches::kSearchEngineChoice,
&switches::kSeedAccountsRevamp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ public static Map<String, String> getFieldTrialParamsForFeature(String featureNa
"SafeBrowsingHashPrefixRealTimeLookups";
public static final String HIDE_TAB_ON_TAB_SWITCHER = "HideTabOnTabSwitcher";
public static final String HISTORY_JOURNEYS = "Journeys";
public static final String IDENTITY_STATUS_CONSISTENCY = "IdentityStatusConsistency";
public static final String INCOGNITO_DOWNLOADS_WARNING = "IncognitoDownloadsWarning";
public static final String INCOGNITO_NTP_REVAMP = "IncognitoNtpRevamp";
public static final String INCOGNITO_REAUTHENTICATION_FOR_ANDROID =
Expand Down
5 changes: 0 additions & 5 deletions components/signin/public/base/signin_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ namespace switches {
// All switches in alphabetical order.

#if BUILDFLAG(IS_ANDROID)
// Feature to add a signed-out avatar on the NTP.
BASE_FEATURE(kIdentityStatusConsistency,
"IdentityStatusConsistency",
base::FEATURE_ENABLED_BY_DEFAULT);

// Feature to refactor how and when accounts are seeded on Android.
BASE_FEATURE(kSeedAccountsRevamp,
"SeedAccountsRevamp",
Expand Down

0 comments on commit 0e16cec

Please sign in to comment.