From: csagan5 <32685696+csagan5@users.noreply.github.com> Date: Sat, 10 Nov 2018 17:20:21 +0100 Subject: Disable promos displayed in bookmarks manager Remove personalized signin promo view from bookmarks, never reach signin thresholds. --- chrome/android/chrome_java_resources.gni | 1 - ...rsonalized_signin_promo_view_bookmarks.xml | 30 ----- .../bookmarks/BookmarkItemsAdapter.java | 30 +---- .../bookmarks/BookmarkPromoHeader.java | 121 +----------------- .../browser/signin/SigninPromoController.java | 23 +--- 5 files changed, 5 insertions(+), 200 deletions(-) delete mode 100644 chrome/android/java/res/layout/personalized_signin_promo_view_bookmarks.xml diff --git a/chrome/android/chrome_java_resources.gni b/chrome/android/chrome_java_resources.gni --- a/chrome/android/chrome_java_resources.gni +++ b/chrome/android/chrome_java_resources.gni @@ -934,7 +934,6 @@ chrome_java_resources = [ "java/res/layout/passwords_error_dialog.xml", "java/res/layout/passwords_progress_dialog.xml", "java/res/layout/personalized_signin_promo_view_body.xml", - "java/res/layout/personalized_signin_promo_view_bookmarks.xml", "java/res/layout/personalized_signin_promo_view_header.xml", "java/res/layout/personalized_signin_promo_view_modern_content_suggestions.xml", "java/res/layout/personalized_signin_promo_view_recent_tabs.xml", diff --git a/chrome/android/java/res/layout/personalized_signin_promo_view_bookmarks.xml b/chrome/android/java/res/layout/personalized_signin_promo_view_bookmarks.xml deleted file mode 100644 --- a/chrome/android/java/res/layout/personalized_signin_promo_view_bookmarks.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - - - - - - - - - \ No newline at end of file diff --git a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java --- a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java @@ -25,7 +25,6 @@ import org.chromium.chrome.browser.bookmarks.BookmarkListEntry.ViewType; import org.chromium.chrome.browser.bookmarks.BookmarkRow.Location; import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.profiles.Profile; -import org.chromium.chrome.browser.signin.PersonalizedSigninPromoView; import org.chromium.chrome.browser.sync.ProfileSyncService; import org.chromium.components.bookmarks.BookmarkId; import org.chromium.components.bookmarks.BookmarkType; @@ -51,7 +50,6 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter @ViewType private int mPromoHeaderType = ViewType.INVALID; private BookmarkDelegate mDelegate; - private BookmarkPromoHeader mPromoHeaderManager; private String mSearchText; private BookmarkId mCurrentFolder; private ProfileSyncService mProfileSyncService; @@ -180,9 +178,9 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter case ViewType.PERSONALIZED_SIGNIN_PROMO: // fall through case ViewType.PERSONALIZED_SYNC_PROMO: - return mPromoHeaderManager.createPersonalizedSigninAndSyncPromoHolder(parent); + return null; case ViewType.SYNC_PROMO: - return mPromoHeaderManager.createSyncPromoHolder(parent); + return null; case ViewType.SECTION_HEADER: return createSectionHeaderViewHolder(parent, viewType); case ViewType.FOLDER: @@ -202,11 +200,7 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter @Override public void onBindViewHolder(ViewHolder holder, int position) { if (holder.getItemViewType() == ViewType.PERSONALIZED_SIGNIN_PROMO) { - PersonalizedSigninPromoView view = (PersonalizedSigninPromoView) holder.itemView; - mPromoHeaderManager.setupPersonalizedSigninPromo(view); } else if (holder.getItemViewType() == ViewType.PERSONALIZED_SYNC_PROMO) { - PersonalizedSigninPromoView view = (PersonalizedSigninPromoView) holder.itemView; - mPromoHeaderManager.setupPersonalizedSyncPromo(view); } else if (holder.getItemViewType() == ViewType.SECTION_HEADER) { bindSectionHeaderViewHolder(holder.itemView, getItemByPosition(position)); } else if (BookmarkListEntry.isBookmarkEntry(holder.getItemViewType())) { @@ -254,7 +248,6 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter case ViewType.PERSONALIZED_SIGNIN_PROMO: // fall through case ViewType.PERSONALIZED_SYNC_PROMO: - mPromoHeaderManager.detachPersonalizePromoView(); break; default: // Other view holders don't have special recycling code. @@ -280,7 +273,6 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter updateHeader(!topLevelFoldersShowing()); }; - mPromoHeaderManager = new BookmarkPromoHeader(mContext, promoHeaderChangeAction); populateTopLevelFoldersList(); mElements = new ArrayList<>(); @@ -295,7 +287,6 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter mDelegate.getModel().removeObserver(mBookmarkModelObserver); mDelegate.getSelectionDelegate().removeObserver(this); mDelegate = null; - mPromoHeaderManager.destroy(); mProfileSyncService.removeSyncStateChangedListener(this); } @@ -418,23 +409,6 @@ class BookmarkItemsAdapter extends DragReorderableListAdapter return; } else if (currentUIState == BookmarkUIState.STATE_SEARCHING) { mPromoHeaderType = ViewType.INVALID; - } else { - switch (mPromoHeaderManager.getPromoState()) { - case BookmarkPromoHeader.PromoState.PROMO_NONE: - mPromoHeaderType = ViewType.INVALID; - break; - case BookmarkPromoHeader.PromoState.PROMO_SIGNIN_PERSONALIZED: - mPromoHeaderType = ViewType.PERSONALIZED_SIGNIN_PROMO; - break; - case BookmarkPromoHeader.PromoState.PROMO_SYNC_PERSONALIZED: - mPromoHeaderType = ViewType.PERSONALIZED_SYNC_PROMO; - break; - case BookmarkPromoHeader.PromoState.PROMO_SYNC: - mPromoHeaderType = ViewType.SYNC_PROMO; - break; - default: - assert false : "Unexpected value for promo state!"; - } } boolean willShowPromo = hasPromoHeader(); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java --- a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java @@ -21,15 +21,9 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceKeys; import org.chromium.chrome.browser.preferences.SharedPreferencesManager; import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.signin.IdentityServicesProvider; -import org.chromium.chrome.browser.signin.PersonalizedSigninPromoView; import org.chromium.chrome.browser.signin.ProfileDataCache; -import org.chromium.chrome.browser.signin.SigninManager; -import org.chromium.chrome.browser.signin.SigninManager.SignInStateObserver; import org.chromium.chrome.browser.signin.SigninPromoController; -import org.chromium.chrome.browser.signin.SigninPromoUtil; import org.chromium.chrome.browser.signin.SyncPromoView; -import org.chromium.chrome.browser.sync.AndroidSyncSettings; -import org.chromium.chrome.browser.sync.AndroidSyncSettings.AndroidSyncSettingsObserver; import org.chromium.components.signin.AccountManagerFacade; import org.chromium.components.signin.AccountManagerFacadeProvider; import org.chromium.components.signin.AccountsChangeObserver; @@ -44,7 +38,7 @@ import java.lang.annotation.RetentionPolicy; * Class that manages all the logic and UI behind the signin promo header in the bookmark * content UI. The header is shown only on certain situations, (e.g., not signed in). */ -class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObserver, +class BookmarkPromoHeader implements ProfileDataCache.Observer, AccountsChangeObserver { /** * Specifies the various states in which the Bookmarks promo can be. @@ -65,7 +59,6 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs private static @Nullable @PromoState Integer sPromoStateForTests; private final Context mContext; - private final SigninManager mSignInManager; private final AccountManagerFacade mAccountManagerFacade; private final Runnable mPromoHeaderChangeAction; @@ -81,12 +74,6 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs mContext = context; mPromoHeaderChangeAction = promoHeaderChangeAction; - AndroidSyncSettings.get().registerObserver(this); - - mSignInManager = IdentityServicesProvider.get().getSigninManager( - Profile.getLastUsedRegularProfile()); - mSignInManager.addSignInStateObserver(this); - mAccountManagerFacade = AccountManagerFacadeProvider.getInstance(); mPromoState = calculatePromoState(); @@ -111,15 +98,11 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs * Clean ups the class. Must be called once done using this class. */ void destroy() { - AndroidSyncSettings.get().unregisterObserver(this); - if (mSigninPromoController != null) { mAccountManagerFacade.removeObserver(this); mProfileDataCache.removeObserver(this); mSigninPromoController.onPromoDestroyed(); } - - mSignInManager.removeSignInStateObserver(this); } /** @@ -130,18 +113,6 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs return mPromoState; } - /** - * @return Personalized signin promo header {@link ViewHolder} instance that can be used with - * {@link RecyclerView}. - */ - ViewHolder createPersonalizedSigninAndSyncPromoHolder(ViewGroup parent) { - View view = LayoutInflater.from(mContext).inflate( - R.layout.personalized_signin_promo_view_bookmarks, parent, false); - - // ViewHolder is abstract and it cannot be instantiated directly. - return new ViewHolder(view) {}; - } - /** * @return Sync promo header {@link ViewHolder} instance that can be used with * {@link RecyclerView}. @@ -153,54 +124,11 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs return new ViewHolder(view) {}; } - /** - * Configures the personalized signin promo and records promo impressions. - * @param view The view to be configured. - */ - void setupPersonalizedSigninPromo(PersonalizedSigninPromoView view) { - SigninPromoUtil.setupSigninPromoViewFromCache(mSigninPromoController, mProfileDataCache, - view, this::setPersonalizedSigninPromoDeclined); - } - - void setupPersonalizedSyncPromo(PersonalizedSigninPromoView view) { - SigninPromoUtil.setupSyncPromoViewFromCache(mSigninPromoController, mProfileDataCache, view, - this::setPersonalizedSigninPromoDeclined); - } - - /** - * Detaches the previously configured {@link PersonalizedSigninPromoView}. - */ - void detachPersonalizePromoView() { - if (mSigninPromoController != null) mSigninPromoController.detach(); - } - - /** - * Saves that the personalized signin promo was declined and updates the UI. - */ - private void setPersonalizedSigninPromoDeclined() { - SharedPreferencesManager.getInstance().writeBoolean( - ChromePreferenceKeys.SIGNIN_PROMO_PERSONALIZED_DECLINED, true); - mPromoState = calculatePromoState(); - triggerPromoUpdate(); - } - - /** - * @return Whether the user declined the personalized signin promo. - */ - @VisibleForTesting - static boolean wasPersonalizedSigninPromoDeclined() { - return SharedPreferencesManager.getInstance().readBoolean( - ChromePreferenceKeys.SIGNIN_PROMO_PERSONALIZED_DECLINED, false); - } - /** * @return Whether the personalized signin promo should be shown to user. */ private boolean shouldShowBookmarkSigninPromo() { - return mSignInManager.isSignInAllowed() - && SigninPromoController.hasNotReachedImpressionLimit( - SigninAccessPoint.BOOKMARK_MANAGER) - && !wasPersonalizedSigninPromoDeclined(); + return false; } private @PromoState int calculatePromoState() { @@ -208,53 +136,9 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs return sPromoStateForTests; } - if (!AndroidSyncSettings.get().doesMasterSyncSettingAllowChromeSync()) { - return PromoState.PROMO_NONE; - } - - if (!mSignInManager.getIdentityManager().hasPrimaryAccount()) { - if (ChromeFeatureList.isEnabled(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)) { - CoreAccountInfo primaryAccount = - mSignInManager.getIdentityManager().getPrimaryAccountInfo( - ConsentLevel.NOT_REQUIRED); - if (primaryAccount != null && !wasPersonalizedSigninPromoDeclined()) { - return PromoState.PROMO_SYNC_PERSONALIZED; - } - } - return shouldShowBookmarkSigninPromo() ? PromoState.PROMO_SIGNIN_PERSONALIZED - : PromoState.PROMO_NONE; - } - - boolean impressionLimitNotReached = - SharedPreferencesManager.getInstance().readInt( - ChromePreferenceKeys.SIGNIN_AND_SYNC_PROMO_SHOW_COUNT) - < MAX_SIGNIN_AND_SYNC_PROMO_SHOW_COUNT; - if (!AndroidSyncSettings.get().isChromeSyncEnabled() && impressionLimitNotReached) { - return PromoState.PROMO_SYNC; - } return PromoState.PROMO_NONE; } - // AndroidSyncSettingsObserver implementation. - @Override - public void androidSyncSettingsChanged() { - mPromoState = calculatePromoState(); - triggerPromoUpdate(); - } - - // SignInStateObserver implementation. - @Override - public void onSignedIn() { - mPromoState = calculatePromoState(); - triggerPromoUpdate(); - } - - @Override - public void onSignedOut() { - mPromoState = calculatePromoState(); - triggerPromoUpdate(); - } - // ProfileDataCache.Observer implementation. @Override public void onProfileDataUpdated(String accountId) { @@ -268,7 +152,6 @@ class BookmarkPromoHeader implements AndroidSyncSettingsObserver, SignInStateObs } private void triggerPromoUpdate() { - detachPersonalizePromoView(); mPromoHeaderChangeAction.run(); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoController.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoController.java --- a/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoController.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoController.java @@ -70,28 +70,7 @@ public class SigninPromoController { * @param accessPoint The access point for which the impression limit is being checked. */ public static boolean hasNotReachedImpressionLimit(@AccessPoint int accessPoint) { - SharedPreferencesManager preferencesManager = SharedPreferencesManager.getInstance(); - switch (accessPoint) { - case SigninAccessPoint.BOOKMARK_MANAGER: - return getSigninPromoImpressionsCountBookmarks() < MAX_IMPRESSIONS_BOOKMARKS; - case SigninAccessPoint.NTP_CONTENT_SUGGESTIONS: - int maxImpressions = ChromeFeatureList.getFieldTrialParamByFeatureAsInt( - ChromeFeatureList.ENHANCED_PROTECTION_PROMO_CARD, - "MaxSigninPromoImpressions", Integer.MAX_VALUE); - return SharedPreferencesManager.getInstance().readInt( - ChromePreferenceKeys.SIGNIN_PROMO_IMPRESSIONS_COUNT_NTP) - < maxImpressions; - case SigninAccessPoint.RECENT_TABS: - // There is no impression limit for Recent Tabs. - return true; - case SigninAccessPoint.SETTINGS: - return preferencesManager.readInt( - ChromePreferenceKeys.SIGNIN_PROMO_IMPRESSIONS_COUNT_SETTINGS) - < MAX_IMPRESSIONS_SETTINGS; - default: - assert false : "Unexpected value for access point: " + accessPoint; - return false; - } + return false; } /** -- 2.17.1