[B4P] Implement AccountStorageAuthHelper::TriggerSignIn on Lacros

`TriggerSignIn()` now goes through signin_ui_util which provides a
common path for Dice platforms and Lacros.

Bug: 1446066
Change-Id: Ic57af856901bfd0149e53836c41171e08883750c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4793758
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186518}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -656,7 +656,7 @@ void ChromePasswordManagerClient::TriggerReauthForPrimaryAccount(
 
 void ChromePasswordManagerClient::TriggerSignIn(
     signin_metrics::AccessPoint access_point) {
-#if BUILDFLAG(ENABLE_DICE_SUPPORT)
+#if BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS)
   account_storage_auth_helper_.TriggerSignIn(access_point);
 #endif
 }
@@ -1332,6 +1332,7 @@ ChromePasswordManagerClient::ChromePasswordManagerClient(
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 #if BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS)
       account_storage_auth_helper_(
+          profile_,
           IdentityManagerFactory::GetForProfile(profile_),
           &password_feature_manager_,
           base::BindRepeating(

chrome/browser/signin/signin_ui_delegate_impl_lacros.cc

@@ -46,6 +46,9 @@ GetAddAccountSourceFromAccessPoint(signin_metrics::AccessPoint access_point) {
     case signin_metrics::AccessPoint::ACCESS_POINT_MENU:
       return account_manager::AccountManagerFacade::AccountAdditionSource::
           kChromeMenuTurnOnSync;
+    case signin_metrics::AccessPoint::ACCESS_POINT_AUTOFILL_DROPDOWN:
+      return account_manager::AccountManagerFacade::AccountAdditionSource::
+          kChromeSigninPromoAddAccount;
     default:
       NOTREACHED() << "Add account is requested from an unknown access point "
                    << static_cast<int>(access_point);

chrome/browser/signin/signin_ui_util.cc

@@ -269,6 +269,28 @@ void ShowExtensionSigninPrompt(Profile* profile,
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 }
 
+void ShowSigninPromptFromPromo(Profile* profile,
+                               signin_metrics::AccessPoint access_point) {
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  NOTREACHED();
+#elif BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS)
+  CHECK_NE(signin_metrics::AccessPoint::ACCESS_POINT_UNKNOWN, access_point);
+  CHECK(!profile->IsOffTheRecord());
+
+  signin::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile);
+  if (identity_manager->HasPrimaryAccount(signin::ConsentLevel::kSignin)) {
+    DVLOG(1) << "The user is already signed in.";
+    return;
+  }
+
+  GetSigninUiDelegate()->ShowSigninUI(
+      profile, /*enable_sync=*/false, access_point,
+      signin_metrics::PromoAction::
+          PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT);
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+}
+
 void EnableSyncFromSingleAccountPromo(
     Profile* profile,
     const CoreAccountInfo& account,

chrome/browser/signin/signin_ui_util.h

@@ -52,6 +52,11 @@ void ShowExtensionSigninPrompt(Profile* profile,
                                bool enable_sync,
                                const std::string& email_hint);
 
+// This function is used to sign-in the user into Chrome without offering sync.
+// This function does nothing if the user is already signed in to Chrome.
+void ShowSigninPromptFromPromo(Profile* profile,
+                               signin_metrics::AccessPoint access_point);
+
 // This function is used to enable sync for a given account:
 // * This function does nothing if the user is already signed in to Chrome.
 // * If |account| is empty, then it presents the Chrome sign-in page.

chrome/browser/signin/signin_ui_util_unittest.cc

@@ -29,6 +29,7 @@
 #include "components/google/core/common/google_util.h"
 #include "components/signin/public/base/consent_level.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"
 #include "components/signin/public/identity_manager/accounts_mutator.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
@@ -573,6 +574,31 @@ TEST_F(SigninUiUtilTest, ShowExtensionSigninPrompt_AsLockedProfile) {
                             /*email_hint=*/std::string());
   EXPECT_EQ(0, tab_strip->count());
 }
+
+TEST_F(SigninUiUtilTest, ShowSigninPromptFromPromo) {
+  Profile* profile = browser()->profile();
+  TabStripModel* tab_strip = browser()->tab_strip_model();
+  ShowSigninPromptFromPromo(profile, access_point_);
+  EXPECT_EQ(1, tab_strip->count());
+
+  content::WebContents* tab = tab_strip->GetWebContentsAt(0);
+  ASSERT_TRUE(tab);
+  EXPECT_TRUE(
+      base::StartsWith(tab->GetVisibleURL().spec(),
+                       GaiaUrls::GetInstance()->add_account_url().spec(),
+                       base::CompareCase::INSENSITIVE_ASCII));
+}
+
+TEST_F(SigninUiUtilTest, ShowSigninPromptFromPromoWithExistingAccount) {
+  signin::MakePrimaryAccountAvailable(GetIdentityManager(), "foo@example.com",
+                                      signin::ConsentLevel::kSignin);
+
+  Profile* profile = browser()->profile();
+  TabStripModel* tab_strip = browser()->tab_strip_model();
+  EXPECT_EQ(0, tab_strip->count());
+  ShowSigninPromptFromPromo(profile, access_point_);
+  EXPECT_EQ(0, tab_strip->count());
+}
 #endif  // !BUILDFLAG(IS_CHROMEOS_LACROS)
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT) || BUILDFLAG(IS_CHROMEOS_LACROS)
 
@@ -748,6 +774,25 @@ TEST_F(MirrorSigninUiUtilTest,
   ShowExtensionSigninPrompt(profile(), /*enable_sync=*/true, kMainEmail);
 }
 
+TEST_F(MirrorSigninUiUtilTest, ShowSigninPromptFromPromo) {
+  signin_metrics::AccessPoint kAccessPoint =
+      signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN;
+  ExpectAddAccount(
+      /*enable_sync=*/false, kAccessPoint,
+      signin_metrics::PromoAction::
+          PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT);
+  ShowSigninPromptFromPromo(profile(), kAccessPoint);
+}
+
+TEST_F(MirrorSigninUiUtilTest, ShowSigninPromptFromPromoWithExistingAccount) {
+  signin::MakePrimaryAccountAvailable(
+      IdentityManagerFactory::GetForProfile(profile()), "foo@example.com",
+      signin::ConsentLevel::kSignin);
+  ShowSigninPromptFromPromo(
+      profile(),
+      signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN);
+}
+
 #endif  // BUILDFLAG(IS_CHROMEOS_LACROS)
 
 // This test does not use the SigninUiUtilTest test fixture, because it

chrome/browser/ui/passwords/account_storage_auth_helper.cc

@@ -8,7 +8,9 @@
 #include <utility>
 
 #include "base/functional/bind.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/reauth_result.h"
+#include "chrome/browser/signin/signin_ui_util.h"
 #include "components/password_manager/core/browser/password_feature_manager.h"
 #include "components/signin/public/base/consent_level.h"
 #include "components/signin/public/base/signin_buildflags.h"
@@ -21,11 +23,13 @@ using ReauthSucceeded =
 }
 
 AccountStorageAuthHelper::AccountStorageAuthHelper(
+    Profile* profile,
     signin::IdentityManager* identity_manager,
     password_manager::PasswordFeatureManager* password_feature_manager,
     base::RepeatingCallback<SigninViewController*()>
         signin_view_controller_getter)
-    : identity_manager_(identity_manager),
+    : profile_(profile),
+      identity_manager_(identity_manager),
       password_feature_manager_(password_feature_manager),
       signin_view_controller_getter_(std::move(signin_view_controller_getter)) {
   DCHECK(password_feature_manager_);
@@ -69,16 +73,10 @@ void AccountStorageAuthHelper::TriggerOptInReauth(
                      std::move(reauth_callback)));
 }
 
-// TODO(https://crbug.com/1446066): make this work on Lacros as well by using
-// utilities from chrome/browser/signin/signin_ui_util.h
-#if BUILDFLAG(ENABLE_DICE_SUPPORT)
 void AccountStorageAuthHelper::TriggerSignIn(
     signin_metrics::AccessPoint access_point) {
-  if (SigninViewController* controller = signin_view_controller_getter_.Run()) {
-    controller->ShowDiceAddAccountTab(access_point, std::string());
-  }
+  signin_ui_util::ShowSigninPromptFromPromo(profile_, access_point);
 }
-#endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
 void AccountStorageAuthHelper::OnOptInReauthCompleted(
     base::OnceCallback<void(ReauthSucceeded)> reauth_callback,

chrome/browser/ui/passwords/account_storage_auth_helper.h

@@ -26,6 +26,7 @@ enum class ReauthAccessPoint;
 }
 
 class SigninViewController;
+class Profile;
 
 // Responsible for triggering authentication flows related to the passwords
 // account storage. Used only by desktop.
@@ -37,6 +38,7 @@ class AccountStorageAuthHelper {
   // because the controller is per window, while this helper is per tab. It
   // may return null.
   AccountStorageAuthHelper(
+      Profile* profile,
       signin::IdentityManager* identity_manager,
       password_manager::PasswordFeatureManager* password_feature_manager,
       base::RepeatingCallback<SigninViewController*()>
@@ -56,11 +58,9 @@ class AccountStorageAuthHelper {
           void(password_manager::PasswordManagerClient::ReauthSucceeded)>
           reauth_callback);
 
-#if BUILDFLAG(ENABLE_DICE_SUPPORT)
-  // Redirects the user to a sign-in in a new tab. |access_point| is used for
-  // metrics recording and represents where the sign-in was triggered.
+  // Shows a sign-in prompt to the user. |access_point| is used for metrics
+  // recording and represents where the sign-in was triggered.
   void TriggerSignIn(signin_metrics::AccessPoint access_point);
-#endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
  private:
   void OnOptInReauthCompleted(
@@ -69,6 +69,8 @@ class AccountStorageAuthHelper {
           reauth_callback,
       signin::ReauthResult result);
 
+  const raw_ptr<Profile> profile_;
+
   const raw_ptr<signin::IdentityManager> identity_manager_;
 
   const raw_ptr<password_manager::PasswordFeatureManager>

chrome/browser/ui/passwords/account_storage_auth_helper_unittest.cc

@@ -11,12 +11,17 @@
 #include "base/test/bind.h"
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
+#include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
 #include "chrome/browser/signin/reauth_result.h"
+#include "chrome/browser/signin/signin_ui_delegate.h"
+#include "chrome/browser/signin/signin_ui_util.h"
 #include "chrome/browser/ui/signin/signin_view_controller.h"
+#include "chrome/test/base/testing_profile.h"
 #include "components/password_manager/core/browser/mock_password_feature_manager.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "components/signin/public/base/signin_metrics.h"
 #include "components/signin/public/identity_manager/identity_test_environment.h"
+#include "content/public/test/browser_task_environment.h"
 #include "google_apis/gaia/core_account_id.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -39,38 +44,59 @@ class MockSigninViewController : public SigninViewController {
                signin_metrics::ReauthAccessPoint,
                base::OnceCallback<void(signin::ReauthResult)>),
               (override));
+};
 
-#if BUILDFLAG(ENABLE_DICE_SUPPORT)
+class MockSigninUiDelegate : public signin_ui_util::SigninUiDelegate {
+ public:
+  MOCK_METHOD(void,
+              ShowSigninUI,
+              (Profile*,
+               bool,
+               signin_metrics::AccessPoint,
+               signin_metrics::PromoAction),
+              (override));
   MOCK_METHOD(void,
-              ShowDiceAddAccountTab,
-              (signin_metrics::AccessPoint, const std::string&),
+              ShowReauthUI,
+              (Profile*,
+               const std::string&,
+               bool,
+               signin_metrics::AccessPoint,
+               signin_metrics::PromoAction),
               (override));
-#endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 };
 
 }  // namespace
 
 class AccountStorageAuthHelperTest : public ::testing::Test {
  public:
   AccountStorageAuthHelperTest()
-      : auth_helper_(
-            identity_test_env_.identity_manager(),
+      : profile_(IdentityTestEnvironmentProfileAdaptor::
+                     CreateProfileForIdentityTestEnvironment()),
+        identity_env_adaptor_(profile_.get()),
+        auth_helper_(
+            profile_.get(),
+            identity_test_env()->identity_manager(),
             &mock_password_feature_manager_,
             base::BindLambdaForTesting([this]() -> SigninViewController* {
               return &mock_signin_view_controller_;
             })) {}
   ~AccountStorageAuthHelperTest() override = default;
 
   CoreAccountId MakeUnconsentedAccountAvailable() {
-    return identity_test_env_
-        .MakePrimaryAccountAvailable("alice@gmail.com",
-                                     signin::ConsentLevel::kSignin)
+    return identity_test_env()
+        ->MakePrimaryAccountAvailable("alice@gmail.com",
+                                      signin::ConsentLevel::kSignin)
         .account_id;
   }
 
+  signin::IdentityTestEnvironment* identity_test_env() {
+    return identity_env_adaptor_.identity_test_env();
+  }
+
  protected:
-  base::test::SingleThreadTaskEnvironment task_environment_;
-  signin::IdentityTestEnvironment identity_test_env_;
+  content::BrowserTaskEnvironment task_environment_;
+  std::unique_ptr<TestingProfile> profile_;
+  IdentityTestEnvironmentProfileAdaptor identity_env_adaptor_;
   password_manager::MockPasswordFeatureManager mock_password_feature_manager_;
   MockSigninViewController mock_signin_view_controller_;
   AccountStorageAuthHelper auth_helper_;
@@ -112,13 +138,16 @@ TEST_F(AccountStorageAuthHelperTest, ShouldNotSetOptInOnFailedReauth) {
   auth_helper_.TriggerOptInReauth(kReauthAccessPoint, base::DoNothing());
 }
 
-#if BUILDFLAG(ENABLE_DICE_SUPPORT)
-TEST_F(AccountStorageAuthHelperTest, ShouldTriggerSigninIfDiceEnabled) {
+TEST_F(AccountStorageAuthHelperTest, ShouldTriggerSignin) {
+  testing::StrictMock<MockSigninUiDelegate> mock_signin_ui_delegate;
+  base::AutoReset<signin_ui_util::SigninUiDelegate*> delegate_auto_reset =
+      signin_ui_util::SetSigninUiDelegateForTesting(&mock_signin_ui_delegate);
   const signin_metrics::AccessPoint kAccessPoint =
       signin_metrics::AccessPoint::ACCESS_POINT_AUTOFILL_DROPDOWN;
-  EXPECT_CALL(mock_signin_view_controller_,
-              ShowDiceAddAccountTab(kAccessPoint, _));
+  EXPECT_CALL(mock_signin_ui_delegate,
+              ShowSigninUI(profile_.get(), /*enable_sync=*/false, kAccessPoint,
+                           signin_metrics::PromoAction::
+                               PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT));
 
   auth_helper_.TriggerSignIn(kAccessPoint);
 }
-#endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)

components/account_manager_core/account_manager_facade.h

@@ -98,8 +98,10 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER_CORE) AccountManagerFacade {
     kChromeOSProjectorAppReauth = 17,
     // Chrome Menu -> Turn on Sync
     kChromeMenuTurnOnSync = 18,
+    // Sign-in promo with a new account.
+    kChromeSigninPromoAddAccount = 19,
 
-    kMaxValue = kChromeMenuTurnOnSync
+    kMaxValue = kChromeSigninPromoAddAccount
   };
 
   AccountManagerFacade();

components/account_manager_core/account_manager_facade_impl.cc

@@ -103,6 +103,8 @@ bool GetIsAvailableInArcBySource(
     case AccountManagerFacade::AccountAdditionSource::
         kChromeSettingsTurnOnSyncButton:
     case AccountManagerFacade::AccountAdditionSource::kChromeMenuTurnOnSync:
+    case AccountManagerFacade::AccountAdditionSource::
+        kChromeSigninPromoAddAccount:
       return false;
     // These are reauthentication cases. ARC visibility shouldn't change for
     // reauthentication.

tools/metrics/histograms/enums.xml

@@ -1299,6 +1299,7 @@ Unknown properties are collapsed to zero. -->
   <int value="17"
       label="Launched from ChromeOS Projector App for re-authentication"/>
   <int value="18" label="Turn on sync menu item in Chrome app menu"/>
+  <int value="19" label="Account addition flow triggered from a Sign-in promo"/>
 </enum>
 
 <enum name="AccountManagerAccountUpsertionResultStatus">