Fix DCHECK when opening Chrome OS settings for user with "." in email

Settings shows GAIA's notion of the user's email address. For example,
if the user signed up for a Google account "Foo.Bar@gmail.com", they
can login to their device with "foobar", but settings shows "Foo.Bar".

Under the hood, the "canonical" email is "foobar", and this is used to
create AccountId objects. signin_ui_util::GetAuthenticatedUsername()
was trying to create an AccountId object using "Foo.Bar" as input,
which caused the DCHECK. However, the function wants that AccountId
to look up the original email with periods in it.

Use ProfileHelper::Get()->GetUserByProfile(profile)->GetDisplayEmail()
instead. This avoids AccountId construction entirely. See bug for
details.

Also set a "display email" for public accounts to fix tests (and
possibly production).

Bug: 994798
Test: settings opens correctly for "james.cook@gmail.com"
Change-Id: Ia01f1d902d3439afa0f53c5535fe6097299ab916
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1757509Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690106}

chrome/browser/chromeos/arc/auth/arc_auth_service.cc

@@ -598,8 +598,8 @@ void ArcAuthService::OnPrimaryAccountAuthCodeFetched(
   DeletePendingTokenRequest(fetcher);
 
   if (success) {
-    const std::string& full_account_id = base::UTF16ToUTF8(
-        signin_ui_util::GetAuthenticatedUsername(identity_manager_));
+    const std::string& full_account_id =
+        base::UTF16ToUTF8(signin_ui_util::GetAuthenticatedUsername(profile_));
     std::move(callback).Run(
         mojom::ArcSignInStatus::SUCCESS,
         CreateAccountInfo(!IsArcOptInVerificationDisabled(), auth_code,

chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc

@@ -696,8 +696,7 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
   run_loop.Run();
 
   ASSERT_TRUE(auth_instance().account_info());
-  EXPECT_EQ(kFakeUserName,
-            auth_instance().account_info()->account_name.value());
+  EXPECT_TRUE(auth_instance().account_info()->account_name.value().empty());
   EXPECT_EQ(kFakeAuthCode, auth_instance().account_info()->auth_code.value());
   EXPECT_EQ(mojom::ChromeAccountType::ROBOT_ACCOUNT,
             auth_instance().account_info()->account_type);
@@ -724,8 +723,7 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest, GetDemoAccount) {
   run_loop.Run();
 
   ASSERT_TRUE(auth_instance().account_info());
-  EXPECT_EQ(kFakeUserName,
-            auth_instance().account_info()->account_name.value());
+  EXPECT_TRUE(auth_instance().account_info()->account_name.value().empty());
   EXPECT_EQ(kFakeAuthCode, auth_instance().account_info()->auth_code.value());
   EXPECT_EQ(mojom::ChromeAccountType::ROBOT_ACCOUNT,
             auth_instance().account_info()->account_type);
@@ -843,8 +841,7 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
   run_loop.Run();
 
   ASSERT_TRUE(auth_instance().account_info());
-  EXPECT_EQ(kFakeUserName,
-            auth_instance().account_info()->account_name.value());
+  EXPECT_TRUE(auth_instance().account_info()->account_name.value().empty());
   EXPECT_EQ(kFakeAuthCode, auth_instance().account_info()->auth_code.value());
   EXPECT_EQ(mojom::ChromeAccountType::ROBOT_ACCOUNT,
             auth_instance().account_info()->account_type);

chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc

@@ -486,11 +486,6 @@ void FakeChromeUserManager::SaveUserDisplayEmail(
   NOTREACHED();
 }
 
-std::string FakeChromeUserManager::GetUserDisplayEmail(
-    const AccountId& account_id) const {
-  return account_id.GetUserEmail();
-}
-
 void FakeChromeUserManager::SaveUserType(const user_manager::User* user) {
   NOTREACHED();
 }

chrome/browser/chromeos/login/users/fake_chrome_user_manager.h

@@ -93,7 +93,6 @@ class FakeChromeUserManager : public ChromeUserManager {
   base::string16 GetUserDisplayName(const AccountId& account_id) const override;
   void SaveUserDisplayEmail(const AccountId& account_id,
                             const std::string& display_email) override;
-  std::string GetUserDisplayEmail(const AccountId& account_id) const override;
   void SaveUserType(const user_manager::User* user) override;
   void UpdateUserAccountData(const AccountId& account_id,
                              const UserAccountData& account_data) override;

chrome/browser/chromeos/login/users/mock_user_manager.h

@@ -51,7 +51,6 @@ class MockUserManager : public ChromeUserManager {
   MOCK_CONST_METHOD1(GetUserDisplayName, base::string16(const AccountId&));
   MOCK_METHOD2(SaveUserDisplayEmail,
                void(const AccountId&, const std::string&));
-  MOCK_CONST_METHOD1(GetUserDisplayEmail, std::string(const AccountId&));
   MOCK_CONST_METHOD0(IsCurrentUserOwner, bool(void));
   MOCK_CONST_METHOD0(IsCurrentUserNew, bool(void));
   MOCK_CONST_METHOD0(IsCurrentUserNonCryptohomeDataEphemeral, bool(void));

chrome/browser/signin/signin_ui_util.cc

@@ -22,14 +22,18 @@
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/base/signin_pref_names.h"
+#include "components/signin/public/identity_manager/account_info.h"
 #include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/identity_utils.h"
-#include "components/user_manager/user_manager.h"
 #include "third_party/re2/src/re2/re2.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/text_elider.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#endif
+
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
 #include "chrome/browser/signin/account_consistency_mode_manager.h"
 #include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h"
@@ -56,20 +60,19 @@ void CreateDiceTurnSyncOnHelper(
 
 namespace signin_ui_util {
 
-base::string16 GetAuthenticatedUsername(
-    const signin::IdentityManager* identity_manager) {
+base::string16 GetAuthenticatedUsername(Profile* profile) {
+  DCHECK(profile);
   std::string user_display_name;
-
+  auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
   if (identity_manager->HasPrimaryAccount()) {
     user_display_name = identity_manager->GetPrimaryAccountInfo().email;
-
 #if defined(OS_CHROMEOS)
-    if (user_manager::UserManager::IsInitialized()) {
-      // On CrOS user email is sanitized and then passed to the identity
-      // manager. Original email (containing dots) is stored as "display email".
-      user_display_name = user_manager::UserManager::Get()->GetUserDisplayEmail(
-          AccountId::FromUserEmail(user_display_name));
-    }
+    // See https://crbug.com/994798 for details.
+    user_manager::User* user =
+        chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
+    // |user| may be null in tests.
+    if (user)
+      user_display_name = user->GetDisplayEmail();
 #endif  // defined(OS_CHROMEOS)
   }
 

chrome/browser/signin/signin_ui_util.h

@@ -14,14 +14,10 @@
 #include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "components/signin/public/base/signin_metrics.h"
-#include "components/signin/public/identity_manager/account_info.h"
 
-class Profile;
+struct AccountInfo;
 class Browser;
-
-namespace signin {
-class IdentityManager;
-}
+class Profile;
 
 // Utility functions to gather status information from the various signed in
 // services and construct messages suitable for showing in UI.
@@ -32,8 +28,7 @@ const int kUpgradeWelcomeTutorialShowMax = 1;
 
 // Returns the username of the authenticated user or an empty string if there is
 // no authenticated user.
-base::string16 GetAuthenticatedUsername(
-    const signin::IdentityManager* identity_manager);
+base::string16 GetAuthenticatedUsername(Profile* profile);
 
 // Initializes signin-related preferences.
 void InitializePrefsForProfile(Profile* profile);

chrome/browser/ui/webui/settings/people_handler.cc

@@ -1043,9 +1043,8 @@ std::unique_ptr<base::DictionaryValue> PeopleHandler::GetSyncStatusDictionary()
       "disabled", !service || disallowed_by_policy ||
                       !service->GetUserSettings()->IsSyncAllowedByPlatform());
   sync_status->SetBoolean("signedIn", identity_manager->HasPrimaryAccount());
-  sync_status->SetString(
-      "signedInUsername",
-      signin_ui_util::GetAuthenticatedUsername(identity_manager));
+  sync_status->SetString("signedInUsername",
+                         signin_ui_util::GetAuthenticatedUsername(profile_));
   sync_status->SetBoolean("hasUnrecoverableError",
                           service && service->HasUnrecoverableError());
   return sync_status;

components/user_manager/fake_user_manager.cc

@@ -213,11 +213,6 @@ base::string16 FakeUserManager::GetUserDisplayName(
   return base::string16();
 }
 
-std::string FakeUserManager::GetUserDisplayEmail(
-    const AccountId& account_id) const {
-  return std::string();
-}
-
 bool FakeUserManager::IsCurrentUserOwner() const {
   return false;
 }

components/user_manager/fake_user_manager.h

@@ -85,7 +85,6 @@ class USER_MANAGER_EXPORT FakeUserManager : public UserManagerBase {
   base::string16 GetUserDisplayName(const AccountId& account_id) const override;
   void SaveUserDisplayEmail(const AccountId& account_id,
                             const std::string& display_email) override {}
-  std::string GetUserDisplayEmail(const AccountId& account_id) const override;
   bool IsCurrentUserOwner() const override;
   bool IsCurrentUserNew() const override;
   bool IsCurrentUserNonCryptohomeDataEphemeral() const override;

components/user_manager/user.cc

@@ -479,7 +479,10 @@ std::string SupervisedUser::display_email() const {
 }
 
 PublicAccountUser::PublicAccountUser(const AccountId& account_id)
-    : DeviceLocalAccountUserBase(account_id) {}
+    : DeviceLocalAccountUserBase(account_id) {
+  // Public accounts do not have a real email address, so they do not set
+  // |display_email_|.
+}
 
 PublicAccountUser::~PublicAccountUser() {
 }

components/user_manager/user.h

@@ -278,7 +278,10 @@ class USER_MANAGER_EXPORT User : public UserInfo {
   AccountId account_id_;
   base::string16 display_name_;
   base::string16 given_name_;
-  // The displayed user email, defaults to |email_|.
+  // User email for display, which may include capitals and non-significant
+  // periods. For example, "John.Steinbeck@gmail.com" is a display email, but
+  // "johnsteinbeck@gmail.com" is the canonical form. Defaults to
+  // account_id_.GetUserEmail().
   std::string display_email_;
   bool using_saml_ = false;
   std::unique_ptr<UserImage> user_image_;

components/user_manager/user_manager.h

@@ -241,12 +241,6 @@ class USER_MANAGER_EXPORT UserManager {
   virtual void SaveUserDisplayEmail(const AccountId& account_id,
                                     const std::string& display_email) = 0;
 
-  // Returns the display email for user |account_id| if it is known (was
-  // previously set by a |SaveUserDisplayEmail| call).
-  // Otherwise, returns |account_id| itself.
-  virtual std::string GetUserDisplayEmail(
-      const AccountId& account_id) const = 0;
-
   // Saves user's type for |user| into local state preferences.
   virtual void SaveUserType(const User* user) = 0;
 

components/user_manager/user_manager_base.cc

@@ -485,12 +485,6 @@ void UserManagerBase::SaveUserDisplayEmail(const AccountId& account_id,
                                base::Value(display_email));
 }
 
-std::string UserManagerBase::GetUserDisplayEmail(
-    const AccountId& account_id) const {
-  const User* user = FindUser(account_id);
-  return user ? user->display_email() : account_id.GetUserEmail();
-}
-
 void UserManagerBase::SaveUserType(const User* user) {
   DCHECK(!task_runner_ || task_runner_->RunsTasksInCurrentSequence());
 

components/user_manager/user_manager_base.h

@@ -79,7 +79,6 @@ class USER_MANAGER_EXPORT UserManagerBase : public UserManager {
   base::string16 GetUserDisplayName(const AccountId& account_id) const override;
   void SaveUserDisplayEmail(const AccountId& account_id,
                             const std::string& display_email) override;
-  std::string GetUserDisplayEmail(const AccountId& account_id) const override;
   void SaveUserType(const User* user) override;
   void UpdateUserAccountData(const AccountId& account_id,
                              const UserAccountData& account_data) override;