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

[iOS] Fixing notification from SigninPromoViewMediator to its consumer

SigninPromoViewMediator should not send update notification to its consumer
when the sign-in is in progress.
This is to avoid updating the sign-in promo from cold state to warm state
while the sign-in is not over.

Bug: 986746
Change-Id: I4f4386034549394eb556e6e48254d97dc4590656
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1718370
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691117}
parent 10087e04
...@@ -16,7 +16,8 @@ ...@@ -16,7 +16,8 @@
// Handles identity update notifications. // Handles identity update notifications.
@protocol SigninPromoViewConsumer <NSObject> @protocol SigninPromoViewConsumer <NSObject>
// Called when the default identity is changed or updated. // Called when the default identity is changed or updated. This method is not
// called when the sign-in is in progress.
// |configurator|, new instance set each time, to configure a SigninPromoView. // |configurator|, new instance set each time, to configure a SigninPromoView.
// |identityChanged| is set to YES when the default identity is changed. // |identityChanged| is set to YES when the default identity is changed.
- (void)configureSigninPromoWithConfigurator: - (void)configureSigninPromoWithConfigurator:
......
...@@ -269,7 +269,12 @@ const char* AlreadySeenSigninViewPreferenceKey( ...@@ -269,7 +269,12 @@ const char* AlreadySeenSigninViewPreferenceKey(
[self sendConsumerNotificationWithIdentityChanged:NO]; [self sendConsumerNotificationWithIdentityChanged:NO];
} }
// Sends the update notification to the consummer if the signin-in is not in
// progress. This is to avoid to update the sign-in promo view in the
// background.
- (void)sendConsumerNotificationWithIdentityChanged:(BOOL)identityChanged { - (void)sendConsumerNotificationWithIdentityChanged:(BOOL)identityChanged {
if (self.signinInProgress)
return;
SigninPromoViewConfigurator* configurator = [self createConfigurator]; SigninPromoViewConfigurator* configurator = [self createConfigurator];
[_consumer configureSigninPromoWithConfigurator:configurator [_consumer configureSigninPromoWithConfigurator:configurator
identityChanged:identityChanged]; identityChanged:identityChanged];
...@@ -378,10 +383,6 @@ const char* AlreadySeenSigninViewPreferenceKey( ...@@ -378,10 +383,6 @@ const char* AlreadySeenSigninViewPreferenceKey(
#pragma mark - ChromeIdentityServiceObserver #pragma mark - ChromeIdentityServiceObserver
- (void)identityListChanged { - (void)identityListChanged {
// Don't update the sign-in promo view while doing sign-in, to avoid UI
// glitches.
if (self.signinInProgress)
return;
ChromeIdentity* newIdentity = nil; ChromeIdentity* newIdentity = nil;
NSArray* identities = ios::GetChromeBrowserProvider() NSArray* identities = ios::GetChromeBrowserProvider()
->GetChromeIdentityService() ->GetChromeIdentityService()
...@@ -396,13 +397,7 @@ const char* AlreadySeenSigninViewPreferenceKey( ...@@ -396,13 +397,7 @@ const char* AlreadySeenSigninViewPreferenceKey(
} }
- (void)profileUpdate:(ChromeIdentity*)identity { - (void)profileUpdate:(ChromeIdentity*)identity {
// Don't update the sign-in promo view while doing sign-in, to avoid UI [self sendConsumerNotificationWithIdentityChanged:NO];
// glitches.
if (!self.signinInProgress)
return;
if (identity == _defaultIdentity) {
[self sendConsumerNotificationWithIdentityChanged:NO];
}
} }
- (void)chromeIdentityServiceWillBeDestroyed { - (void)chromeIdentityServiceWillBeDestroyed {
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#include "components/signin/public/base/signin_metrics.h" #include "components/signin/public/base/signin_metrics.h"
#include "components/unified_consent/feature.h" #include "components/unified_consent/feature.h"
#include "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h"
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_consumer.h" #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_consumer.h"
...@@ -95,11 +96,7 @@ class SigninPromoViewMediatorTest : public PlatformTest { ...@@ -95,11 +96,7 @@ class SigninPromoViewMediatorTest : public PlatformTest {
// Check a new created configurator. // Check a new created configurator.
CheckWarmStateConfigurator([mediator_ createConfigurator]); CheckWarmStateConfigurator([mediator_ createConfigurator]);
// The consumer should receive a notification related to the image. // The consumer should receive a notification related to the image.
configurator_ = nil; CheckForImageNotification();
ExpectConfiguratorNotification(NO /* identity changed */);
WaitUntilFakeChromeIdentityServiceCallbackCompleted();
// Check the configurator received by the consumer.
CheckWarmStateConfigurator(configurator_);
} }
// Expects a notification on the consumer for an identity update, and stores // Expects a notification on the consumer for an identity update, and stores
...@@ -183,6 +180,16 @@ class SigninPromoViewMediatorTest : public PlatformTest { ...@@ -183,6 +180,16 @@ class SigninPromoViewMediatorTest : public PlatformTest {
} }
} }
// Checks to receive a notification for the image upate of the current
// identity.
void CheckForImageNotification() {
configurator_ = nil;
ExpectConfiguratorNotification(NO /* identity changed */);
WaitUntilFakeChromeIdentityServiceCallbackCompleted();
// Check the configurator received by the consumer.
CheckWarmStateConfigurator(configurator_);
}
// Runs the runloop until all callback from FakeChromeIdentityService are // Runs the runloop until all callback from FakeChromeIdentityService are
// called. // called.
void WaitUntilFakeChromeIdentityServiceCallbackCompleted() { void WaitUntilFakeChromeIdentityServiceCallbackCompleted() {
...@@ -317,4 +324,67 @@ TEST_F(SigninPromoViewMediatorTest, SigninPromoViewStateSignedin) { ...@@ -317,4 +324,67 @@ TEST_F(SigninPromoViewMediatorTest, SigninPromoViewStateSignedin) {
mediator_.signinPromoViewState); mediator_.signinPromoViewState);
} }
// Tests that no update notification is sent by the mediator to its consumer,
// while the sign-in is in progress, when an identity is added.
TEST_F(SigninPromoViewMediatorTest,
SigninPromoViewNoUpdateNotificationWhileSignin) {
CreateMediator(signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS);
[mediator_ signinPromoViewVisible];
__block ShowSigninCommandCompletionCallback completion;
ShowSigninCommandCompletionCallback completion_arg =
[OCMArg checkWithBlock:^BOOL(ShowSigninCommandCompletionCallback value) {
completion = value;
return YES;
}];
OCMExpect([consumer_
signinPromoViewMediator:mediator_
shouldOpenSigninWithIdentity:nil
promoAction:
signin_metrics::PromoAction::
PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT
completion:completion_arg]);
// Starts sign-in without identity.
[mediator_ signinPromoViewDidTapSigninWithNewAccount:signin_promo_view_];
// Adds an identity while doing sign-in.
AddDefaultIdentity();
// No consumer notification should be expected.
WaitUntilFakeChromeIdentityServiceCallbackCompleted();
// Finishs the sign-in.
OCMExpect([consumer_ signinDidFinish]);
completion(YES);
}
// Tests that no update notification is sent by the mediator to its consumer,
// while the sign-in is in progress.
TEST_F(SigninPromoViewMediatorTest,
SigninPromoViewNoUpdateNotificationWhileSignin2) {
AddDefaultIdentity();
CreateMediator(signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS);
[mediator_ signinPromoViewVisible];
__block ShowSigninCommandCompletionCallback completion;
ShowSigninCommandCompletionCallback completion_arg =
[OCMArg checkWithBlock:^BOOL(ShowSigninCommandCompletionCallback value) {
completion = value;
return YES;
}];
OCMExpect([consumer_ signinPromoViewMediator:mediator_
shouldOpenSigninWithIdentity:expected_default_identity_
promoAction:signin_metrics::PromoAction::
PROMO_ACTION_WITH_DEFAULT
completion:completion_arg]);
// Starts sign-in with an identity.
[mediator_ signinPromoViewDidTapSigninWithDefaultAccount:signin_promo_view_];
EXPECT_TRUE(
[mediator_ conformsToProtocol:@protocol(ChromeIdentityServiceObserver)]);
id<ChromeIdentityServiceObserver> chromeIdentityServiceObserver =
(id<ChromeIdentityServiceObserver>)mediator_;
// Simulates an identity update.
[chromeIdentityServiceObserver profileUpdate:expected_default_identity_];
// Spins the run loop to wait for the profile image update.
WaitUntilFakeChromeIdentityServiceCallbackCompleted();
// Finishs the sign-in.
OCMExpect([consumer_ signinDidFinish]);
completion(YES);
}
} // namespace } // namespace
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