[Avatar button] Fine tune the trigger points for showing user email

This CL introduces two kill-switches that guards showing user email on
startup and on sign-in in the avatar button. This way, we simplify the
launch by decoupling the visual changes in the button (the master
feature) from the individual trigger points.

This CL also tweaks the startup trigger by only showing the pill for
multi-profile or multi-account users.

Bug: 1009441
Change-Id: I4f097b385a8469fc59d94494f5528fbefeda8b38
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831771
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703634}

chrome/browser/signin/signin_ui_util.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ -14,11 +15,13 @@
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/base/signin_pref_names.h"
@@ -260,4 +263,30 @@ std::string GetAllowedDomain(std::string signin_pattern) {
   return domain;
 }
 
+bool ShouldShowIdentityOnOpeningProfile(
+    const ProfileAttributesStorage& profile_attributes_storage,
+    Profile* profile) {
+  DCHECK(profile);
+  signin::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile);
+  DCHECK(identity_manager->AreRefreshTokensLoaded());
+  // Wait with the potential positive response until refresh tokens are loaded
+  // so that we never show it twice on startup.
+  // TODO(crbug.com/1009441): Make it only appear once per profile
+  // instantiation (currently it appears for every new window which have their
+  // own instance of AvatarToolbarButton).
+  if (!base::FeatureList::IsEnabled(
+          features::kAnimatedAvatarButtonOnOpeningProfile)) {
+    return false;
+  }
+
+  // Show the user identity for users with multiple profiles.
+  if (profile_attributes_storage.GetNumberOfProfiles() > 1) {
+    return true;
+  }
+
+  // Show the user identity for users with multiple signed-in accounts.
+  return identity_manager->GetAccountsWithRefreshTokens().size() > 1;
+}
+
 }  // namespace signin_ui_util

chrome/browser/signin/signin_ui_util.h

@@ -18,6 +18,7 @@
 struct AccountInfo;
 class Browser;
 class Profile;
+class ProfileAttributesStorage;
 
 // Utility functions to gather status information from the various signed in
 // services and construct messages suitable for showing in UI.
@@ -90,6 +91,13 @@ void EnableSyncFromPromo(
 }  // namespace internal
 #endif
 
+// Returns whether Chrome should show the identity of the user (using a brief
+// animation) on opening a profile. IdentityManager's refresh tokens must be
+// loaded when this function gets called.
+bool ShouldShowIdentityOnOpeningProfile(
+    const ProfileAttributesStorage& profile_attributes_storage,
+    Profile* profile);
+
 }  // namespace signin_ui_util
 
 #endif  // CHROME_BROWSER_SIGNIN_SIGNIN_UI_UTIL_H_

chrome/browser/signin/signin_ui_util_unittest.cc

@@ -9,11 +9,15 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/metrics/user_action_tester.h"
 #include "build/buildflag.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
 #include "chrome/browser/signin/scoped_account_consistency.h"
 #include "chrome/browser/signin/signin_promo.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "components/account_id/account_id.h"
 #include "components/google/core/common/google_util.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "components/signin/public/identity_manager/account_info.h"
@@ -52,8 +56,12 @@ TEST_F(GetAllowedDomainTest, WithValidPattern) {
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
 
 namespace {
+
 const char kMainEmail[] = "main_email@example.com";
 const char kMainGaiaID[] = "main_gaia_id";
+const char kSecondaryEmail[] = "secondary_email@example.com";
+const char kSecondaryGaiaID[] = "secondary_gaia_id";
+
 class SigninUiUtilTestBrowserWindow : public TestBrowserWindow {
  public:
   SigninUiUtilTestBrowserWindow() = default;
@@ -76,6 +84,7 @@ class SigninUiUtilTestBrowserWindow : public TestBrowserWindow {
 
   DISALLOW_COPY_AND_ASSIGN(SigninUiUtilTestBrowserWindow);
 };
+
 }  // namespace
 
 class DiceSigninUiUtilTest : public BrowserWithTestWindowTest {
@@ -498,4 +507,59 @@ TEST_F(DiceSigninUiUtilTest, MergeDiceSigninTab) {
 }
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
+TEST_F(DiceSigninUiUtilTest,
+       ShouldShowIdentityOnOpeningProfile_ReturnsTrueForMultiProfiles) {
+  const char kSecondProfile[] = "SecondProfile";
+  const base::FilePath profile_path =
+      profile_manager()->profiles_dir().AppendASCII(kSecondProfile);
+  profile_manager()->profile_attributes_storage()->AddProfile(
+      profile_path, base::ASCIIToUTF16(kSecondProfile), std::string(),
+      base::string16(), false, 0, std::string(), EmptyAccountId());
+
+  EXPECT_TRUE(ShouldShowIdentityOnOpeningProfile(
+      *profile_manager()->profile_attributes_storage(), profile()));
+}
+
+TEST_F(DiceSigninUiUtilTest,
+       ShouldShowIdentityOnOpeningProfile_ReturnsTrueForMultiSignin) {
+  GetIdentityManager()->GetAccountsMutator()->AddOrUpdateAccount(
+      kMainGaiaID, kMainEmail, "refresh_token", false,
+      signin_metrics::SourceForRefreshTokenOperation::kUnknown);
+  GetIdentityManager()->GetAccountsMutator()->AddOrUpdateAccount(
+      kSecondaryEmail, kSecondaryGaiaID, "refresh_token", false,
+      signin_metrics::SourceForRefreshTokenOperation::kUnknown);
+
+  EXPECT_TRUE(ShouldShowIdentityOnOpeningProfile(
+      *profile_manager()->profile_attributes_storage(), profile()));
+}
+
+TEST_F(
+    DiceSigninUiUtilTest,
+    ShouldShowIdentityOnOpeningProfile_ReturnsFalseForSingleProfileSingleSignin) {
+  GetIdentityManager()->GetAccountsMutator()->AddOrUpdateAccount(
+      kMainGaiaID, kMainEmail, "refresh_token", false,
+      signin_metrics::SourceForRefreshTokenOperation::kUnknown);
+
+  EXPECT_FALSE(ShouldShowIdentityOnOpeningProfile(
+      *profile_manager()->profile_attributes_storage(), profile()));
+}
+
+TEST_F(DiceSigninUiUtilTest,
+       ShouldShowIdentityOnOpeningProfile_ReturnsFalseForFeatureDisabled) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(
+      features::kAnimatedAvatarButtonOnOpeningProfile);
+
+  // Do the same setup that makes the previous test return true.
+  GetIdentityManager()->GetAccountsMutator()->AddOrUpdateAccount(
+      kMainGaiaID, kMainEmail, "refresh_token", false,
+      signin_metrics::SourceForRefreshTokenOperation::kUnknown);
+  GetIdentityManager()->GetAccountsMutator()->AddOrUpdateAccount(
+      kSecondaryEmail, kSecondaryGaiaID, "refresh_token", false,
+      signin_metrics::SourceForRefreshTokenOperation::kUnknown);
+
+  EXPECT_FALSE(ShouldShowIdentityOnOpeningProfile(
+      *profile_manager()->profile_attributes_storage(), profile()));
+}
+
 }  // namespace signin_ui_util

chrome/browser/ui/ui_features.cc

@@ -10,6 +10,16 @@ namespace features {
 // https://crbug.com/967317
 const base::Feature kAnimatedAvatarButton{"AnimatedAvatarButton",
                                           base::FEATURE_DISABLED_BY_DEFAULT};
+// Enables an animated avatar button on the sign-in trigger. This feature is
+// guarded by kAnimatedAvatarButton and serves as a kill-switch. See
+// https://crbug.com/967317
+const base::Feature kAnimatedAvatarButtonOnSignIn{
+    "AnimatedAvatarButtonOnSignIn", base::FEATURE_ENABLED_BY_DEFAULT};
+// Enables an animated avatar button on the open-profile/startup trigger. This
+// feature is guarded by kAnimatedAvatarButton and serves as a kill-switch. See
+// https://crbug.com/967317
+const base::Feature kAnimatedAvatarButtonOnOpeningProfile{
+    "AnimatedAvatarButtonOnOpeningProfile", base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Enables showing the EV certificate details in the Page Info bubble.
 const base::Feature kEvDetailsInPageInfo{"EvDetailsInPageInfo",

chrome/browser/ui/ui_features.h

@@ -18,6 +18,8 @@ namespace features {
 // alongside the definition of their values in the .cc file.
 
 extern const base::Feature kAnimatedAvatarButton;
+extern const base::Feature kAnimatedAvatarButtonOnSignIn;
+extern const base::Feature kAnimatedAvatarButtonOnOpeningProfile;
 
 extern const base::Feature kEvDetailsInPageInfo;
 

chrome/browser/ui/views/profiles/avatar_toolbar_button.cc

@@ -48,11 +48,14 @@ constexpr base::TimeDelta kEmailExpansionDuration =
 constexpr base::TimeDelta kAvatarHighlightAnimationDuration =
     base::TimeDelta::FromSeconds(2);
 
+ProfileAttributesStorage& GetProfileAttributesStorage() {
+  return g_browser_process->profile_manager()->GetProfileAttributesStorage();
+}
+
 ProfileAttributesEntry* GetProfileAttributesEntry(Profile* profile) {
   ProfileAttributesEntry* entry;
-  if (!g_browser_process->profile_manager()
-           ->GetProfileAttributesStorage()
-           .GetProfileAttributesWithPath(profile->GetPath(), &entry)) {
+  if (!GetProfileAttributesStorage().GetProfileAttributesWithPath(
+          profile->GetPath(), &entry)) {
     return nullptr;
   }
   return entry;
@@ -65,9 +68,7 @@ bool IsGenericProfile(const ProfileAttributesEntry& entry) {
     return true;
 
   return entry.GetAvatarIconIndex() == 0 &&
-         g_browser_process->profile_manager()
-                 ->GetProfileAttributesStorage()
-                 .GetNumberOfProfiles() == 1;
+         GetProfileAttributesStorage().GetNumberOfProfiles() == 1;
 }
 
 int GetIconSizeForNonTouchUi() {
@@ -88,8 +89,7 @@ AvatarToolbarButton::AvatarToolbarButton(Browser* browser)
 #endif  // !defined(OS_CHROMEOS)
       browser_(browser),
       profile_(browser_->profile()) {
-  profile_observer_.Add(
-      &g_browser_process->profile_manager()->GetProfileAttributesStorage());
+  profile_observer_.Add(&GetProfileAttributesStorage());
 
   State state = GetState();
   if (state == State::kIncognitoProfile) {
@@ -98,7 +98,9 @@ AvatarToolbarButton::AvatarToolbarButton(Browser* browser)
     signin::IdentityManager* identity_manager =
         IdentityManagerFactory::GetForProfile(profile_);
     identity_manager_observer_.Add(identity_manager);
-    SetUserEmail(identity_manager->GetUnconsentedPrimaryAccountInfo().email);
+
+    if (identity_manager->AreRefreshTokensLoaded())
+      OnRefreshTokensLoaded();
   }
 
   // Activate on press for left-mouse-button only to mimic other MenuButtons
@@ -328,9 +330,21 @@ void AvatarToolbarButton::OnProfileNameChanged(
 
 void AvatarToolbarButton::OnUnconsentedPrimaryAccountChanged(
     const CoreAccountInfo& unconsented_primary_account_info) {
+  if (!base::FeatureList::IsEnabled(features::kAnimatedAvatarButtonOnSignIn))
+    return;
   SetUserEmail(unconsented_primary_account_info.email);
 }
 
+void AvatarToolbarButton::OnRefreshTokensLoaded() {
+  if (!signin_ui_util::ShouldShowIdentityOnOpeningProfile(
+          GetProfileAttributesStorage(), profile_)) {
+    return;
+  }
+  SetUserEmail(IdentityManagerFactory::GetForProfile(profile_)
+                   ->GetUnconsentedPrimaryAccountInfo()
+                   .email);
+}
+
 void AvatarToolbarButton::OnAccountsInCookieUpdated(
     const signin::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
     const GoogleServiceAuthError& error) {

chrome/browser/ui/views/profiles/avatar_toolbar_button.h

@@ -83,6 +83,7 @@ class AvatarToolbarButton : public ToolbarButton,
   // Needed if the first sync promo account should be displayed.
   void OnUnconsentedPrimaryAccountChanged(
       const CoreAccountInfo& unconsented_primary_account_info) override;
+  void OnRefreshTokensLoaded() override;
   void OnAccountsInCookieUpdated(
       const signin::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
       const GoogleServiceAuthError& error) override;