Skip to content

Commit

Permalink
[Signin] Remove interface state in SignInPreference.java
Browse files Browse the repository at this point in the history
The state interface in SignInPreference is not used in production code
except one test. So, this patch removes the interface and then update
the test.

Bug: 1245603
Change-Id: I5a09ae22b03ae1f6f714405e9eaab82431960f65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3138313
Commit-Queue: Jinho Bang <zino@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918117}
  • Loading branch information
romandev authored and Chromium LUCI CQ committed Sep 3, 2021
1 parent 128a76c commit bb0993e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import android.content.Context;
import android.util.AttributeSet;

import androidx.annotation.IntDef;
import androidx.appcompat.content.res.AppCompatResources;
import androidx.preference.Preference;
import androidx.preference.PreferenceViewHolder;
Expand Down Expand Up @@ -35,9 +34,6 @@
import org.chromium.components.user_prefs.UserPrefs;
import org.chromium.ui.base.ViewUtils;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

/**
* A preference that displays "Sign in to Chrome" when the user is not sign in, and displays
* the user's name, email, profile image and sync error icon if necessary when the user is signed
Expand All @@ -46,24 +42,12 @@
public class SignInPreference
extends Preference implements SignInStateObserver, ProfileDataCache.Observer,
SyncStateChangedListener, AccountsChangeObserver {
// TODO(https://crbug.com/1245603): Remove state
@IntDef({State.SIGNIN_DISABLED_BY_POLICY, State.SIGNIN_DISALLOWED, State.GENERIC_PROMO,
State.SIGNED_IN})
@Retention(RetentionPolicy.SOURCE)
public @interface State {
int SIGNIN_DISABLED_BY_POLICY = 0;
int SIGNIN_DISALLOWED = 1;
int GENERIC_PROMO = 2;
int SIGNED_IN = 3;
}

private final PrefService mPrefService;
private boolean mWasGenericSigninPromoDisplayed;
private boolean mViewEnabled;
private boolean mIsShowingSigninPromo;
private final ProfileDataCache mProfileDataCache;
private final AccountManagerFacade mAccountManagerFacade;
private @State int mState;

/**
* Constructor for inflating from XML.
Expand All @@ -75,9 +59,6 @@ public SignInPreference(Context context, AttributeSet attrs) {
mPrefService = UserPrefs.get(Profile.getLastUsedRegularProfile());
mProfileDataCache = ProfileDataCache.createWithDefaultImageSizeAndNoBadge(context);
mAccountManagerFacade = AccountManagerFacadeProvider.getInstance();

// State will be updated in registerForUpdates.
mState = State.SIGNED_IN;
mIsShowingSigninPromo = false;
}

Expand Down Expand Up @@ -114,12 +95,6 @@ public void onDetached() {
}
}

/** Returns the state of the preference. Not valid until registerForUpdates is called. */
@State
public int getState() {
return mState;
}

/**
* Sets whether Personalized Signin Promo is being shown in {@link
* org.chromium.chrome.browser.settings.MainSettings} page
Expand Down Expand Up @@ -160,7 +135,6 @@ private void update() {
}

private void setupSigninDisabledByPolicy() {
mState = State.SIGNIN_DISABLED_BY_POLICY;
setTitle(R.string.sync_promo_turn_on_sync);
setSummary(R.string.sign_in_to_chrome_disabled_summary);
setFragment(null);
Expand All @@ -175,12 +149,10 @@ private void setupSigninDisabledByPolicy() {
}

private void setupSigninDisallowed() {
mState = State.SIGNIN_DISALLOWED;
mWasGenericSigninPromoDisplayed = false;
}

private void setupGenericPromo() {
mState = State.GENERIC_PROMO;
setTitle(R.string.sync_promo_turn_on_sync);
setSummary(R.string.signin_pref_summary);

Expand All @@ -200,7 +172,6 @@ private void setupGenericPromo() {
}

private void setupSignedIn(String accountName) {
mState = State.SIGNED_IN;
DisplayableProfileData profileData = mProfileDataCache.getProfileDataOrDefault(accountName);

setTitle(profileData.getFullNameOrEmail());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
import org.chromium.chrome.browser.signin.ui.SyncConsentActivityLauncher;
import org.chromium.chrome.browser.sync.SyncService;
import org.chromium.chrome.browser.sync.SyncTestRule;
import org.chromium.chrome.browser.sync.settings.SignInPreference;
import org.chromium.chrome.browser.sync.settings.AccountManagementFragment;
import org.chromium.chrome.browser.sync.settings.SyncPromoPreference;
import org.chromium.chrome.browser.sync.settings.SyncPromoPreference.State;
import org.chromium.chrome.browser.tracing.settings.DeveloperSettings;
Expand Down Expand Up @@ -317,12 +317,14 @@ public void testAccountSignIn() throws InterruptedException {
mMainSettings.findPreference(MainSettings.PREF_MANAGE_SYNC).isVisible());

// SignIn to see the changes
mSyncTestRule.setUpAccountAndEnableSyncForTesting();
CoreAccountInfo account = mSyncTestRule.setUpAccountAndEnableSyncForTesting();
SyncTestUtil.waitForSyncFeatureActive();
SignInPreference signInPreference =
(SignInPreference) assertSettingsExists(MainSettings.PREF_SIGN_IN, null);
Assert.assertEquals("SignInPreference should be at the signed in state. ",
signInPreference.getState(), SignInPreference.State.SIGNED_IN);
// SignInPreference should be at the signed in state
Assert.assertEquals("SignInPreference should be at the signed in state.",
account.getEmail(),
mMainSettings.findPreference(MainSettings.PREF_SIGN_IN).getSummary().toString());
assertSettingsExists(MainSettings.PREF_SIGN_IN, AccountManagementFragment.class);

Assert.assertTrue("Account section header should appear when user signed in.",
mMainSettings.findPreference(MainSettings.PREF_ACCOUNT_AND_GOOGLE_SERVICES_SECTION)
.isVisible());
Expand Down

0 comments on commit bb0993e

Please sign in to comment.