Decouple lock screen online reauth UI from password sync

This cl extracts management of LockScreenStartReauthDialog from
InSessionPasswordSyncManager. Previously lock screen online reauth flow
always went through InSessionPasswordSyncManager::CreateAndShowDialog()
even though it could be caused by timeout policy unrelated to password
sync. Also InSessionPasswordSyncManager stored a unique pointer to
LockScreenStartReauthDialog creating an illusion that it owns it, even
though in fact the dialog triggers self-destruction from
LockScreenStartReauthDialog::OnDialogClosed method which is typical for
a SystemWebDialogDelegate. This cl makes it clear that
LockScreenStartReauthDialog is a singleton independent of
InSessionPasswordSyncManager.

This is a refactoring, not a behavioral change.

Bug: 1377014
Change-Id: I0c584d89ae776660b44721ba335e2fac98595af0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4051112
Reviewed-by: Roman Sorokin <rsorokin@google.com>
Reviewed-by: Tomas Gunnarsson <tommi@chromium.org>
Reviewed-by: Mohammed Abdon <mohammedabdon@chromium.org>
Commit-Queue: Andrey Davydov <andreydav@google.com>
Cr-Commit-Position: refs/heads/main@{#1075600}

chrome/browser/ash/login/saml/in_session_password_sync_manager.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/ash/profiles/profile_helper.h"
 #include "chrome/browser/ash/settings/cros_settings.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_reauth_dialogs.h"
 #include "chromeos/ash/components/login/auth/auth_session_authenticator.h"
 #include "chromeos/ash/components/login/auth/extended_authenticator.h"
 #include "chromeos/ash/components/login/auth/password_update_flow.h"
@@ -287,81 +288,7 @@ void InSessionPasswordSyncManager::OnAuthSuccess(
   if (screenlock_bridge_->IsLocked()) {
     screenlock_bridge_->lock_handler()->Unlock(user_context.GetAccountId());
   }
-  DismissDialog();
-}
-
-void InSessionPasswordSyncManager::CreateAndShowDialog() {
-  DCHECK(!lock_screen_start_reauth_dialog_);
-  lock_screen_start_reauth_dialog_ =
-      std::make_unique<LockScreenStartReauthDialog>();
-  lock_screen_start_reauth_dialog_->Show();
-}
-
-void InSessionPasswordSyncManager::DismissDialog() {
-  if (lock_screen_start_reauth_dialog_) {
-    lock_screen_start_reauth_dialog_->Dismiss();
-  }
-}
-
-void InSessionPasswordSyncManager::ResetDialog() {
-  DCHECK(lock_screen_start_reauth_dialog_);
-  lock_screen_start_reauth_dialog_.reset();
-  OnReauthDialogClosedForTesting();  // IN-TEST
-}
-
-int InSessionPasswordSyncManager::GetDialogWidth() {
-  if (!lock_screen_start_reauth_dialog_)
-    return 0;
-  return lock_screen_start_reauth_dialog_->GetDialogWidth();
-}
-
-content::WebContents* InSessionPasswordSyncManager::GetDialogWebContents() {
-  if (!lock_screen_start_reauth_dialog_)
-    return nullptr;
-  return lock_screen_start_reauth_dialog_->GetWebContents();
-}
-
-bool InSessionPasswordSyncManager::IsReauthDialogLoadedForTesting(
-    base::OnceClosure callback) {
-  if (is_dialog_loaded_for_testing_)
-    return true;
-  DCHECK(!on_dialog_loaded_callback_for_testing_);
-  on_dialog_loaded_callback_for_testing_ = std::move(callback);
-  return false;
-}
-
-bool InSessionPasswordSyncManager::IsReauthDialogClosedForTesting(
-    base::OnceClosure callback) {
-  if (!is_dialog_loaded_for_testing_)
-    return true;
-  DCHECK(!on_dialog_closed_callback_for_testing_);
-  on_dialog_closed_callback_for_testing_ = std::move(callback);
-  return false;
-}
-
-void InSessionPasswordSyncManager::OnReauthDialogReadyForTesting() {
-  if (is_dialog_loaded_for_testing_)
-    return;
-  is_dialog_loaded_for_testing_ = true;
-  if (on_dialog_loaded_callback_for_testing_) {
-    std::move(on_dialog_loaded_callback_for_testing_).Run();
-  }
-}
-
-void InSessionPasswordSyncManager::OnReauthDialogClosedForTesting() {
-  if (!is_dialog_loaded_for_testing_)
-    return;
-  is_dialog_loaded_for_testing_ = false;
-  if (on_dialog_closed_callback_for_testing_) {
-    std::move(on_dialog_closed_callback_for_testing_).Run();
-  }
-}
-
-void InSessionPasswordSyncManager::OnWebviewLoadAborted() {
-  if (lock_screen_start_reauth_dialog_) {
-    lock_screen_start_reauth_dialog_->UpdateState(
-        NetworkError::ERROR_REASON_FRAME_ERROR);
-  }
+  LockScreenStartReauthDialog::Dismiss();
 }
 
 void InSessionPasswordSyncManager::OnPasswordUpdateSuccess(

chrome/browser/ash/login/saml/in_session_password_sync_manager.h

@@ -12,7 +12,6 @@
 #include "base/time/clock.h"
 #include "chrome/browser/ash/login/saml/password_sync_token_fetcher.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_reauth_dialogs.h"
 #include "chromeos/ash/components/login/auth/auth_status_consumer.h"
 #include "chromeos/ash/components/login/auth/public/authentication_error.h"
 #include "chromeos/ash/components/login/auth/public/user_context.h"
@@ -109,48 +108,13 @@ class InSessionPasswordSyncManager
   // with an IdP.
   void OnAuthSuccess(const UserContext& user_context) override;
 
-  // Create and show lockscreen re-authentication dialog.
-  void CreateAndShowDialog();
-
-  // Dismiss lockscreen re-authentication dialog.
-  void DismissDialog();
-
-  // Reset lockscreen re-authentication dialog.
-  void ResetDialog();
-
-  // Get lockscreen reauth dialog width.
-  int GetDialogWidth();
-
-  // Get web contents of lockscreen reauth dialog.
-  content::WebContents* GetDialogWebContents();
-
-  // Check if reauth dialog is loaded and ready for testing.
-  bool IsReauthDialogLoadedForTesting(base::OnceClosure callback);
-
-  // Check if reauth dialog is closed.
-  // |callback| is used to notify test that the reauth dialog is closed.
-  bool IsReauthDialogClosedForTesting(base::OnceClosure callback);
-
-  // Notify test that the reauth dialog is ready for testing.
-  void OnReauthDialogReadyForTesting();
-
-  // Forces network state update because webview reported frame loading error.
-  void OnWebviewLoadAborted();
-
-  LockScreenStartReauthDialog* get_reauth_dialog_for_testing() {
-    return lock_screen_start_reauth_dialog_.get();
-  }
-
  private:
   void UpdateOnlineAuth();
   void OnCookiesTransfered();
   // Password sync token API calls.
   void CreateTokenAsync();
   void FetchTokenAsync();
 
-  // Notify test that the reauth dialog is closed.
-  void OnReauthDialogClosedForTesting();
-
   void OnPasswordUpdateSuccess(std::unique_ptr<UserContext> user_context);
   void OnPasswordUpdateFailure(std::unique_ptr<UserContext> user_context,
                                AuthenticationError error);
@@ -169,19 +133,8 @@ class InSessionPasswordSyncManager
   scoped_refptr<AuthSessionAuthenticator> auth_session_authenticator_;
   std::unique_ptr<PasswordUpdateFlow> password_update_flow_;
 
-  // Used to create dialog to authenticate the user on lockscreen.
-  std::unique_ptr<LockScreenStartReauthDialog> lock_screen_start_reauth_dialog_;
-
   PasswordChangedCallback password_changed_callback_;
 
-  // A callback that is used to notify test that the reauth dialog is loaded.
-  base::OnceClosure on_dialog_loaded_callback_for_testing_;
-
-  // A callback that is used to notify test that the reauth dialog is closed.
-  base::OnceClosure on_dialog_closed_callback_for_testing_;
-
-  bool is_dialog_loaded_for_testing_ = false;
-
   friend class InSessionPasswordSyncManagerTest;
   friend class InSessionPasswordSyncManagerFactory;
 

chrome/browser/ash/login/saml/lockscreen_reauth_dialog_test_helper.cc

@@ -6,8 +6,6 @@
 
 #include "base/run_loop.h"
 #include "chrome/browser/ash/login/login_pref_names.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager_factory.h"
 #include "chrome/browser/ash/login/test/js_checker.h"
 #include "chrome/browser/ash/login/test/test_condition_waiter.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
@@ -103,24 +101,14 @@ bool LockScreenReauthDialogTestHelper::ShowDialogAndWaitImpl() {
     return false;
   }
 
-  // The screen can only be locked if there is an active user session, so
-  // ProfileManager::GetActiveUserProfile() must return a non-null Profile.
-  Profile* profile = ProfileManager::GetActiveUserProfile();
-  CHECK(profile);
-  password_sync_manager_ =
-      InSessionPasswordSyncManagerFactory::GetForProfile(profile);
-  if (!password_sync_manager_) {
-    ADD_FAILURE() << "Could not retrieve InSessionPasswordSyncManager";
-    return false;
-  }
   ProfileManager::GetActiveUserProfile()->GetPrefs()->SetBoolean(
       prefs::kLockScreenReauthenticationEnabled, true);
-  password_sync_manager_->CreateAndShowDialog();
 
-  WaitForReauthDialogToLoad();
+  LockScreenStartReauthDialog::Show();
 
   // Fetch the dialog, WebUi controller and main message handler.
-  reauth_dialog_ = password_sync_manager_->get_reauth_dialog_for_testing();
+  reauth_dialog_ = LockScreenStartReauthDialog::GetInstance();
+  WaitForReauthDialogToLoad();
   if (!reauth_dialog_ || !reauth_dialog_->GetWebUIForTest()) {
     ADD_FAILURE() << "Could not retrieve LockScreenStartReauthDialog";
     return false;
@@ -283,16 +271,14 @@ void LockScreenReauthDialogTestHelper::WaitForAuthenticatorToLoad() {
 
 void LockScreenReauthDialogTestHelper::WaitForReauthDialogToClose() {
   base::RunLoop run_loop;
-  if (!password_sync_manager_->IsReauthDialogClosedForTesting(
-          run_loop.QuitClosure())) {
+  if (!reauth_dialog_->IsClosedForTesting(run_loop.QuitClosure())) {
     run_loop.Run();
   }
 }
 
 void LockScreenReauthDialogTestHelper::WaitForReauthDialogToLoad() {
   base::RunLoop run_loop;
-  if (!password_sync_manager_->IsReauthDialogLoadedForTesting(
-          run_loop.QuitClosure())) {
+  if (!reauth_dialog_->IsLoadedForTesting(run_loop.QuitClosure())) {
     run_loop.Run();
   }
 }

chrome/browser/ash/login/saml/lockscreen_reauth_dialog_test_helper.h

@@ -15,7 +15,6 @@ class WebContents;
 
 namespace ash {
 
-class InSessionPasswordSyncManager;
 class LockScreenStartReauthDialog;
 class LockScreenStartReauthUI;
 class LockScreenReauthHandler;
@@ -150,7 +149,6 @@ class LockScreenReauthDialogTestHelper {
   void WaitForNetworkDialogToLoad();
 
   // Main Dialog
-  base::raw_ptr<InSessionPasswordSyncManager> password_sync_manager_ = nullptr;
   base::raw_ptr<LockScreenStartReauthDialog> reauth_dialog_ = nullptr;
   base::raw_ptr<LockScreenStartReauthUI> reauth_webui_controller_ = nullptr;
   base::raw_ptr<LockScreenReauthHandler> main_handler_ = nullptr;

chrome/browser/media/chromeos_login_and_lock_media_access_handler.cc

@@ -8,11 +8,10 @@
 
 #include "base/logging.h"
 #include "base/values.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager_factory.h"
 #include "chrome/browser/ash/login/ui/login_display_host.h"
 #include "chrome/browser/ash/settings/cros_settings.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_reauth_dialogs.h"
 #include "chromeos/ash/components/settings/cros_settings_names.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "content/public/browser/render_frame_host.h"
@@ -36,16 +35,11 @@ bool ChromeOSLoginAndLockMediaAccessHandler::SupportsStreamType(
     return true;
   }
   // Check if the `web_contents` corresponds to the reauthentication dialog that
-  // is shown on the lock screen. This is the case when there is an active user
-  // profile and InSessionPasswordSyncManager for this profile is showing reauth
-  // dialog with the same `web_contents`.
-  Profile* profile = ProfileManager::GetActiveUserProfile();
-  if (!profile)
-    return false;
-  auto* password_sync_manager =
-      ash::InSessionPasswordSyncManagerFactory::GetForProfile(profile);
-  return !!password_sync_manager &&
-         web_contents == password_sync_manager->GetDialogWebContents();
+  // is shown on the lock screen.
+  ash::LockScreenStartReauthDialog* lock_screen_online_reauth_dialog =
+      ash::LockScreenStartReauthDialog::GetInstance();
+  return !!lock_screen_online_reauth_dialog &&
+         web_contents == lock_screen_online_reauth_dialog->GetWebContents();
 }
 
 bool ChromeOSLoginAndLockMediaAccessHandler::CheckMediaAccessPermission(

chrome/browser/ui/ash/login_screen_client_impl.cc

@@ -20,8 +20,6 @@
 #include "chrome/browser/ash/login/login_auth_recorder.h"
 #include "chrome/browser/ash/login/login_pref_names.h"
 #include "chrome/browser/ash/login/reauth_stats.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager_factory.h"
 #include "chrome/browser/ash/login/startup_utils.h"
 #include "chrome/browser/ash/login/ui/login_display_host.h"
 #include "chrome/browser/ash/login/ui/login_display_host_webui.h"
@@ -407,16 +405,10 @@ void LoginScreenClientImpl::OnParentAccessValidation(
 void LoginScreenClientImpl::ShowGaiaSigninInternal(
     const AccountId& prefilled_account) {
   if (ash::LoginDisplayHost::default_host()) {
+    // Login screen case.
     ash::LoginDisplayHost::default_host()->ShowGaiaDialog(prefilled_account);
   } else {
-    const user_manager::User* user =
-        user_manager::UserManager::Get()->FindUser(prefilled_account);
-    Profile* profile = ash::ProfileHelper::Get()->GetProfileByUser(user);
-    DCHECK(session_manager::SessionManager::Get()->IsScreenLocked());
-    auto* password_sync_manager =
-        ash::InSessionPasswordSyncManagerFactory::GetForProfile(profile);
-    if (password_sync_manager) {
-      password_sync_manager->CreateAndShowDialog();
-    }
+    // Lock screen case.
+    ash::LockScreenStartReauthDialog::Show();
   }
 }

chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_network_dialog.cc

@@ -9,8 +9,6 @@
 #include "ash/constants/ash_features.h"
 #include "base/bind.h"
 #include "base/task/task_traits.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager_factory.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_reauth_dialogs.h"

chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_network_handler.cc

@@ -12,10 +12,9 @@
 #include "base/bind.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager_factory.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
 #include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_reauth_dialogs.h"
 #include "chrome/browser/ui/webui/ash/internet_config_dialog.h"
 #include "chrome/browser/ui/webui/ash/internet_detail_dialog.h"
 #include "chrome/common/url_constants.h"
@@ -43,14 +42,6 @@ constexpr char kShowNetworkDetails[] = "showNetworkDetails";
 constexpr char kShowNetworkConfig[] = "showNetworkConfig";
 constexpr char kGetHostname[] = "getHostname";
 
-InSessionPasswordSyncManager* GetInSessionPasswordSyncManager() {
-  const user_manager::User* user =
-      user_manager::UserManager::Get()->GetActiveUser();
-  Profile* profile = ProfileHelper::Get()->GetProfileByUser(user);
-
-  return InSessionPasswordSyncManagerFactory::GetForProfile(profile);
-}
-
 }  // namespace
 
 NetworkConfigMessageHandler::NetworkConfigMessageHandler() {}
@@ -83,9 +74,8 @@ void NetworkConfigMessageHandler::Initialize(const base::Value::List& args) {
 
   // Check if the main dialog exists and notify that the network dialog has
   // been loaded.
-  auto* password_sync_manager = GetInSessionPasswordSyncManager();
   LockScreenStartReauthDialog* start_reauth_dialog =
-      password_sync_manager->get_reauth_dialog_for_testing();
+      LockScreenStartReauthDialog::GetInstance();
   if (!start_reauth_dialog)
     return;
   start_reauth_dialog->OnNetworkDialogReadyForTesting();

chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_network_ui.cc

@@ -12,10 +12,9 @@
 #include "base/bind.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager.h"
-#include "chrome/browser/ash/login/saml/in_session_password_sync_manager_factory.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
 #include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_network_handler.h"
 #include "chrome/browser/ui/webui/ash/internet_config_dialog.h"
 #include "chrome/browser/ui/webui/ash/internet_detail_dialog.h"