Commit a19aa56d authored by Jérôme Lebel's avatar Jérôme Lebel Committed by Commit Bot

[iOS][Signin] Moving all the sign-in promo view preferences to SigninPromoViewMediator

The SigninPromoViewMediator should be in charge of registering and using:
  - prefs::kIosBookmarkSigninPromoDisplayedCount
  - prefs::kIosBookmarkPromoAlreadySeen

Also, once the sign-in promo view has been seen too much by the user, it should not set
prefs::kIosBookmarkPromoAlreadySeen to true. kIosBookmarkPromoAlreadySeen should be used only for
the close button. Some views could have a max number of views, without a close button.

This work is to prepare the clean up for the sign-in promo view in the settings
(crrev.com/c/774519).

Bug: 772050
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id7f7cfc1146afbec552aaa159b628caac691d105
Reviewed-on: https://chromium-review.googlesource.com/776780Reviewed-by: default avatarEric Noyau <noyau@chromium.org>
Reviewed-by: default avatarLouis Romero <lpromero@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517845}
parent 82119cae
......@@ -69,6 +69,7 @@ source_set("browser_prefs") {
"//ios/chrome/browser/net",
"//ios/chrome/browser/physical_web",
"//ios/chrome/browser/signin",
"//ios/chrome/browser/ui/authentication",
"//ios/chrome/browser/ui/bookmarks",
"//ios/chrome/browser/voice:prefs",
"//ios/public/provider/chrome/browser",
......
......@@ -38,7 +38,6 @@
#include "components/update_client/update_client.h"
#include "components/variations/service/variations_service.h"
#include "components/web_resource/web_resource_pref_names.h"
#include "ios/chrome/browser/bookmarks/bookmark_new_generation_features.h"
#include "ios/chrome/browser/browser_state/browser_state_info_cache.h"
#include "ios/chrome/browser/desktop_promotion/desktop_promotion_sync_service.h"
#include "ios/chrome/browser/first_run/first_run.h"
......@@ -49,10 +48,8 @@
#include "ios/chrome/browser/physical_web/physical_web_prefs_registration.h"
#include "ios/chrome/browser/pref_names.h"
#include "ios/chrome/browser/signin/signin_manager_factory.h"
#import "ios/chrome/browser/ui/bookmarks/bookmark_collection_view.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h"
#import "ios/chrome/browser/ui/bookmarks/bookmark_mediator.h"
#import "ios/chrome/browser/ui/bookmarks/bookmark_promo_controller.h"
#import "ios/chrome/browser/ui/bookmarks/bookmark_table_view.h"
#include "ios/chrome/browser/voice/voice_search_prefs_registration.h"
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#include "ui/base/l10n/l10n_util.h"
......@@ -122,13 +119,8 @@ void RegisterBrowserStatePrefs(user_prefs::PrefRegistrySyncable* registry) {
DesktopPromotionSyncService::RegisterDesktopPromotionUserPrefs(registry);
RegisterVoiceSearchBrowserStatePrefs(registry);
if (base::FeatureList::IsEnabled(kBookmarkNewGeneration)) {
[BookmarkTableView registerBrowserStatePrefs:registry];
} else {
[BookmarkCollectionView registerBrowserStatePrefs:registry];
}
[BookmarkMediator registerBrowserStatePrefs:registry];
[BookmarkPromoController registerBrowserStatePrefs:registry];
[SigninPromoViewMediator registerBrowserStatePrefs:registry];
[HandoffManager registerBrowserStatePrefs:registry];
registry->RegisterIntegerPref(prefs::kIosSettingsSigninPromoDisplayedCount,
0);
......
......@@ -33,6 +33,10 @@ enum class SigninPromoViewState {
};
} // namespace ios
namespace user_prefs {
class PrefRegistrySyncable;
} // namespace user_prefs
// Class that monitors the available identities and creates
// SigninPromoViewConfigurator. This class makes the link between the model and
// the view. The consumer will receive notification if default identity is
......@@ -49,6 +53,16 @@ enum class SigninPromoViewState {
// Sign-in promo view state.
@property(nonatomic) ios::SigninPromoViewState signinPromoViewState;
// Registers the feature preferences.
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry;
// Tests if the sign-in promo view should be displayed according to the number
// of times it has been displayed and if the user closed the sign-in promo view.
+ (BOOL)shouldDisplaySigninPromoViewWithAccessPoint:
(signin_metrics::AccessPoint)accessPoint
browserState:(ios::ChromeBrowserState*)
browserState;
// See -[SigninPromoViewMediator initWithBrowserState:].
- (instancetype)init NS_UNAVAILABLE;
......
......@@ -237,6 +237,33 @@ const char* AlreadySeenSigninViewPreferenceKey(
@synthesize signinPromoViewState = _signinPromoViewState;
@synthesize presenter = _presenter;
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry {
registry->RegisterBooleanPref(prefs::kIosBookmarkPromoAlreadySeen, false);
registry->RegisterIntegerPref(prefs::kIosBookmarkSigninPromoDisplayedCount,
0);
}
+ (BOOL)shouldDisplaySigninPromoViewWithAccessPoint:
(signin_metrics::AccessPoint)accessPoint
browserState:(ios::ChromeBrowserState*)
browserState {
PrefService* prefs = browserState->GetPrefs();
const char* displayedCountPreferenceKey =
DisplayedCountPreferenceKey(accessPoint);
if (displayedCountPreferenceKey &&
prefs->GetInteger(displayedCountPreferenceKey) >=
kAutomaticSigninPromoViewDismissCount) {
return NO;
}
const char* alreadySeenSigninViewPreferenceKey =
AlreadySeenSigninViewPreferenceKey(accessPoint);
if (alreadySeenSigninViewPreferenceKey &&
prefs->GetBoolean(alreadySeenSigninViewPreferenceKey)) {
return NO;
}
return YES;
}
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
accessPoint:(signin_metrics::AccessPoint)accessPoint
presenter:(id<SigninPresenter>)presenter {
......@@ -341,12 +368,6 @@ const char* AlreadySeenSigninViewPreferenceKey(
int displayedCount = prefs->GetInteger(displayedCountPreferenceKey);
++displayedCount;
prefs->SetInteger(displayedCountPreferenceKey, displayedCount);
const char* alreadySeenSigninViewPreferenceKey =
AlreadySeenSigninViewPreferenceKey(_accessPoint);
if (displayedCount >= kAutomaticSigninPromoViewDismissCount &&
alreadySeenSigninViewPreferenceKey) {
prefs->SetBoolean(alreadySeenSigninViewPreferenceKey, true);
}
}
- (void)signinPromoViewHidden {
......@@ -357,11 +378,15 @@ const char* AlreadySeenSigninViewPreferenceKey(
- (void)signinPromoViewClosed {
DCHECK(_isSigninPromoViewVisible && ![self isInvalidOrClosed]);
_signinPromoViewState = ios::SigninPromoViewState::Closed;
const char* alreadySeenSigninViewPreferenceKey =
AlreadySeenSigninViewPreferenceKey(_accessPoint);
DCHECK(alreadySeenSigninViewPreferenceKey);
PrefService* prefs = _browserState->GetPrefs();
prefs->SetBoolean(alreadySeenSigninViewPreferenceKey, true);
const char* displayedCountPreferenceKey =
DisplayedCountPreferenceKey(_accessPoint);
if (!displayedCountPreferenceKey)
return;
PrefService* prefs = _browserState->GetPrefs();
int displayedCount = prefs->GetInteger(displayedCountPreferenceKey);
RecordImpressionsTilXButtonHistogramForAccessPoint(_accessPoint,
displayedCount);
......
......@@ -25,9 +25,6 @@ class GURL;
namespace bookmarks {
class BookmarkNode;
} // namespace bookmarks
namespace user_prefs {
class PrefRegistrySyncable;
} // namespace user_prefs
@protocol BookmarkCollectionViewDelegate<NSObject>
......@@ -94,9 +91,6 @@ class PrefRegistrySyncable;
@interface BookmarkCollectionView
: UIView<BookmarkHomePrimaryView, BookmarkModelBridgeObserver>
// Registers the feature preferences.
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry;
// Designated initializer.
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
frame:(CGRect)frame
......
......@@ -21,12 +21,10 @@
#include "components/favicon/core/large_icon_service.h"
#include "components/favicon_base/fallback_icon_style.h"
#include "components/favicon_base/favicon_types.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "ios/chrome/browser/bookmarks/bookmark_model_factory.h"
#include "ios/chrome/browser/bookmarks/bookmarks_utils.h"
#include "ios/chrome/browser/experimental_flags.h"
#include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h"
#include "ios/chrome/browser/pref_names.h"
#import "ios/chrome/browser/sync/synced_sessions_bridge.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view_configurator.h"
......@@ -154,11 +152,6 @@ CGFloat minFaviconSizePt = 16;
@synthesize shadow = _shadow;
@synthesize presenter = _presenter;
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry {
registry->RegisterIntegerPref(prefs::kIosBookmarkSigninPromoDisplayedCount,
0);
}
#pragma mark - Initialization
- (void)setupViews {
......
......@@ -13,10 +13,6 @@ namespace ios {
class ChromeBrowserState;
} // namespace ios
namespace user_prefs {
class PrefRegistrySyncable;
} // namespace user_prefs
@protocol BookmarkPromoControllerDelegate
// Controls the state of the promo.
......@@ -34,9 +30,6 @@ class PrefRegistrySyncable;
// call the promoStateChanged: selector on the delegate.
@property(nonatomic, assign) BOOL promoState;
// Registers the feature preferences.
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry;
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
delegate:
(id<BookmarkPromoControllerDelegate>)delegate
......
......@@ -13,8 +13,8 @@
#include "components/signin/core/browser/signin_manager.h"
#include "google_apis/gaia/google_service_auth_error.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/pref_names.h"
#include "ios/chrome/browser/signin/signin_manager_factory.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h"
#import "ios/chrome/browser/ui/commands/show_signin_command.h"
#import "ios/chrome/browser/ui/signin_interaction/public/signin_presenter.h"
#import "ios/chrome/browser/ui/ui_util.h"
......@@ -99,10 +99,6 @@ class SignInObserver : public SigninManagerBase::Observer {
@synthesize promoState = _promoState;
@synthesize presenter = _presenter;
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry {
registry->RegisterBooleanPref(prefs::kIosBookmarkPromoAlreadySeen, false);
}
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
delegate:
(id<BookmarkPromoControllerDelegate>)delegate
......@@ -155,8 +151,6 @@ class SignInObserver : public SigninManagerBase::Observer {
UMA_HISTOGRAM_ENUMERATION(kBookmarksPromoActionsHistogram,
BOOKMARKS_PROMO_ACTION_DISMISSED,
BOOKMARKS_PROMO_ACTION_COUNT);
PrefService* prefs = _browserState->GetPrefs();
prefs->SetBoolean(prefs::kIosBookmarkPromoAlreadySeen, true);
self.promoState = NO;
}
......@@ -173,8 +167,10 @@ class SignInObserver : public SigninManagerBase::Observer {
return;
DCHECK(_browserState);
PrefService* prefs = _browserState->GetPrefs();
if (!prefs->GetBoolean(prefs::kIosBookmarkPromoAlreadySeen)) {
if ([SigninPromoViewMediator
shouldDisplaySigninPromoViewWithAccessPoint:
signin_metrics::AccessPoint::ACCESS_POINT_BOOKMARK_MANAGER
browserState:_browserState]) {
SigninManager* signinManager =
ios::SigninManagerFactory::GetForBrowserState(_browserState);
self.promoState = !signinManager->IsAuthenticated();
......
......@@ -19,10 +19,6 @@ namespace ios {
class ChromeBrowserState;
}
namespace user_prefs {
class PrefRegistrySyncable;
} // namespace user_prefs
@class BookmarkTableView;
@class MDCFlexibleHeaderView;
......@@ -93,9 +89,6 @@ class PrefRegistrySyncable;
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype) new NS_UNAVAILABLE;
// Registers the feature preferences.
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry;
// Called when something outside the view causes the promo state to change.
- (void)promoStateChangedAnimated:(BOOL)animated;
......
......@@ -11,12 +11,10 @@
#include "components/favicon/core/large_icon_service.h"
#include "components/favicon_base/fallback_icon_style.h"
#include "components/favicon_base/favicon_types.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "ios/chrome/browser/bookmarks/bookmark_model_factory.h"
#include "ios/chrome/browser/bookmarks/bookmarks_utils.h"
#include "ios/chrome/browser/experimental_flags.h"
#include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h"
#include "ios/chrome/browser/pref_names.h"
#import "ios/chrome/browser/sync/synced_sessions_bridge.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view_configurator.h"
......@@ -136,11 +134,6 @@ using IntegerPair = std::pair<NSInteger, NSInteger>;
@synthesize addingNewFolder = _addingNewFolder;
@synthesize editingFolderCell = _editingFolderCell;
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry {
registry->RegisterIntegerPref(prefs::kIosBookmarkSigninPromoDisplayedCount,
0);
}
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
delegate:(id<BookmarkTableViewDelegate>)delegate
rootNode:(const BookmarkNode*)rootNode
......
......@@ -1248,8 +1248,8 @@ id<GREYMatcher> ActionSheet(Action action) {
// Check the sign-in promo view is visible.
[SigninEarlGreyUtils
checkSigninPromoVisibleWithMode:SigninPromoViewModeColdState];
// Check the sign-in promo will not be shown anymore.
[BookmarksTestCase verifyPromoAlreadySeen:YES];
// Check the sign-in promo already-seen state didn't change.
[BookmarksTestCase verifyPromoAlreadySeen:NO];
GREYAssertEqual(
20, prefs->GetInteger(prefs::kIosBookmarkSigninPromoDisplayedCount),
@"Should have incremented the display count");
......
......@@ -1998,8 +1998,8 @@ id<GREYMatcher> TappableBookmarkNodeWithLabel(NSString* label) {
// Check the sign-in promo view is visible.
[SigninEarlGreyUtils
checkSigninPromoVisibleWithMode:SigninPromoViewModeColdState];
// Check the sign-in promo will not be shown anymore.
[BookmarksNewGenTestCase verifyPromoAlreadySeen:YES];
// Check the sign-in promo already-seen state didn't change.
[BookmarksNewGenTestCase verifyPromoAlreadySeen:NO];
GREYAssertEqual(
20, prefs->GetInteger(prefs::kIosBookmarkSigninPromoDisplayedCount),
@"Should have incremented the display count");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment