Move assistant optin into oobe flow

Bug: b/112564431, b/78190629
Test: Manual Test
Change-Id: Ie046c8ab05918d4aabaa931a59a4fcdf0270e4f4
Reviewed-on: https://chromium-review.googlesource.com/1176377
Commit-Queue: Yue Li <updowndota@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584833}

chrome/browser/chromeos/first_run/first_run.cc

@@ -21,11 +21,9 @@
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/extensions/app_launch_params.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
-#include "chrome/browser/ui/webui/chromeos/assistant_optin/assistant_optin_ui.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
-#include "chromeos/assistant/buildflags.h"
 #include "chromeos/chromeos_switches.h"
 #include "components/arc/arc_prefs.h"
 #include "components/arc/arc_service_manager.h"
@@ -135,15 +133,6 @@ class DialogLauncher : public content::NotificationObserver {
       }
     }
 
-#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
-    // Launch Assistant OOBE flow if Assistant is enabled.
-    if (account_supported && chromeos::switches::IsAssistantEnabled()) {
-      chromeos::AssistantOptInDialog::Show();
-      delete this;
-      return;
-    }
-#endif
-
     // If voice interaction value prop needs to be shown, the tutorial will be
     // shown after the voice interaction OOBE flow.
     if (account_supported && arc::IsArcPlayStoreEnabledForProfile(profile_) &&

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -97,6 +97,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/assistant/buildflags.h"
 #include "chromeos/cert_loader.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
@@ -147,6 +148,10 @@
 #include "components/rlz/rlz_tracker.h"
 #endif
 
+#if BUILDFLAG(ENABLE_CROS_ASSISTANT)
+#include "chrome/browser/ui/ash/assistant/assistant_client.h"
+#endif
+
 namespace chromeos {
 
 namespace {
@@ -1536,6 +1541,13 @@ bool UserSessionManager::InitializeUserSession(Profile* profile) {
   ProfileHelper::Get()->ProfileStartup(profile);
 
   if (start_session_type_ == PRIMARY_USER_SESSION) {
+#if BUILDFLAG(ENABLE_CROS_ASSISTANT)
+    // Initialize Assistant early to be used in post login Oobe steps.
+    if (chromeos::switches::IsAssistantEnabled()) {
+      AssistantClient::Get()->MaybeInit(
+          content::BrowserContext::GetConnectorFor(profile));
+    }
+#endif
     UserFlow* user_flow = ChromeUserManager::Get()->GetCurrentUserFlow();
     WizardController* oobe_controller = WizardController::default_controller();
     base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();

chrome/browser/chromeos/login/wizard_controller.cc

@@ -81,12 +81,14 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/tablet_mode_client.h"
+#include "chrome/browser/ui/webui/chromeos/assistant_optin/assistant_optin_ui.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
 #include "chrome/browser/ui/webui/help/help_utils_chromeos.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/assistant/buildflags.h"
 #include "chromeos/audio/cras_audio_handler.h"
 #include "chromeos/chromeos_constants.h"
 #include "chromeos/chromeos_switches.h"
@@ -663,7 +665,7 @@ void WizardController::ShowArcTermsOfServiceScreen() {
           arc::prefs::kArcTermsShownInOobe, true);
     }
   } else {
-    ShowUserImageScreen();
+    ShowAssistantOptIn();
   }
 }
 
@@ -781,6 +783,25 @@ void WizardController::ShowDiscoverScreen() {
   SetCurrentScreen(GetScreen(OobeScreen::SCREEN_DISCOVER));
 }
 
+void WizardController::ShowAssistantOptIn() {
+#if BUILDFLAG(ENABLE_CROS_ASSISTANT)
+  if (chromeos::switches::IsAssistantEnabled()) {
+    DCHECK(!chromeos::AssistantOptInDialog::IsActive());
+    // TODO(updowndota) Refactor Assistant opt-in code to better fit into the
+    // oobe flow logic.
+    chromeos::AssistantOptInDialog::Show(
+        base::BindOnce(&WizardController::OnAssistantOptInCompleted,
+                       weak_factory_.GetWeakPtr()));
+  }
+#else
+  ShowUserImageScreen();
+#endif
+}
+
+void WizardController::OnAssistantOptInCompleted(bool accepted) {
+  ShowUserImageScreen();
+}
+
 void WizardController::SkipToLoginForTesting(
     const LoginScreenContext& context) {
   VLOG(1) << "SkipToLoginForTesting.";
@@ -1033,9 +1054,10 @@ void WizardController::OnArcTermsOfServiceSkipped() {
     OnOobeFlowFinished();
     return;
   }
+
   // If the user finished with the PlayStore Terms of Service, advance to the
-  // user image screen.
-  ShowUserImageScreen();
+  // assistant opt-in flow.
+  ShowAssistantOptIn();
 }
 
 void WizardController::OnArcTermsOfServiceAccepted() {
@@ -1051,11 +1073,11 @@ void WizardController::OnArcTermsOfServiceAccepted() {
 
   // If the feature flag for recommend app screen is on, show it after the user
   // finished with the PlayStore Terms of Service. Otherwise, advance to the
-  // user image screen.
+  // assistant opt-in flow.
   if (ShouldShowRecommendAppsScreen()) {
     ShowRecommendAppsScreen();
   } else {
-    ShowUserImageScreen();
+    ShowAssistantOptIn();
   }
 }
 
@@ -1066,7 +1088,7 @@ void WizardController::OnArcTermsOfServiceBack() {
 }
 
 void WizardController::OnRecommendAppsSkipped() {
-  ShowUserImageScreen();
+  ShowAssistantOptIn();
 }
 
 void WizardController::OnRecommendAppsSelected() {
@@ -1074,7 +1096,7 @@ void WizardController::OnRecommendAppsSelected() {
 }
 
 void WizardController::OnAppDownloadingFinished() {
-  ShowUserImageScreen();
+  ShowAssistantOptIn();
 }
 
 void WizardController::OnVoiceInteractionValuePropSkipped() {

chrome/browser/chromeos/login/wizard_controller.h

@@ -192,6 +192,10 @@ class WizardController : public BaseScreenDelegate,
   void ShowUpdateRequiredScreen();
   void ShowDiscoverScreen();
 
+  // Shows assistant opt-in flow.
+  void ShowAssistantOptIn();
+  void OnAssistantOptInCompleted(bool accepted);
+
   // Shows images login screen.
   void ShowLoginScreen(const LoginScreenContext& context);
 

chrome/browser/ui/webui/chromeos/assistant_optin/assistant_optin_ui.cc

@@ -6,10 +6,13 @@
 
 #include <memory>
 
+#include "ash/public/cpp/shell_window_ids.h"
 #include "base/bind.h"
 #include "base/macros.h"
+#include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/webui/chromeos/assistant_optin/get_more_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/assistant_optin/ready_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/assistant_optin/third_party_screen_handler.h"
@@ -19,6 +22,7 @@
 #include "chrome/grit/browser_resources.h"
 #include "components/arc/arc_prefs.h"
 #include "components/prefs/pref_service.h"
+#include "components/session_manager/core/session_manager.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
 
@@ -108,7 +112,13 @@ void AssistantOptInDialog::Show(
     ash::mojom::AssistantSetup::StartAssistantOptInFlowCallback callback) {
   DCHECK(!is_active);
   AssistantOptInDialog* dialog = new AssistantOptInDialog(std::move(callback));
-  dialog->ShowSystemDialog(true);
+
+  int container_id = session_manager::SessionManager::Get()->session_state() ==
+                             session_manager::SessionState::ACTIVE
+                         ? ash::kShellWindowId_AlwaysOnTopContainer
+                         : ash::kShellWindowId_LockSystemModalContainer;
+  chrome::ShowWebDialogInContainer(
+      container_id, ProfileManager::GetActiveUserProfile(), dialog, true);
 }
 
 // static
@@ -120,7 +130,9 @@ AssistantOptInDialog::AssistantOptInDialog(
     ash::mojom::AssistantSetup::StartAssistantOptInFlowCallback callback)
     : SystemWebDialogDelegate(GURL(chrome::kChromeUIAssistantOptInURL),
                               base::string16()),
-      callback_(std::move(callback)) {
+      callback_(std::move(callback)),
+      modal_type_(StartupUtils::IsOobeCompleted() ? ui::MODAL_TYPE_SYSTEM
+                                                  : ui::MODAL_TYPE_WINDOW) {
   DCHECK(!is_active);
   is_active = true;
 }
@@ -129,6 +141,10 @@ AssistantOptInDialog::~AssistantOptInDialog() {
   is_active = false;
 }
 
+ui::ModalType AssistantOptInDialog::GetDialogModalType() const {
+  return modal_type_;
+}
+
 void AssistantOptInDialog::GetDialogSize(gfx::Size* size) const {
   size->SetSize(kAssistantOptInDialogWidth, kAssistantOptInDialogHeight);
 }

chrome/browser/ui/webui/chromeos/assistant_optin/assistant_optin_ui.h

@@ -56,6 +56,7 @@ class AssistantOptInDialog : public SystemWebDialogDelegate {
   ~AssistantOptInDialog() override;
 
   // ui::WebDialogDelegate
+  ui::ModalType GetDialogModalType() const override;
   void GetDialogSize(gfx::Size* size) const override;
   std::string GetDialogArgs() const override;
   bool ShouldShowDialogTitle() const override;
@@ -65,6 +66,9 @@ class AssistantOptInDialog : public SystemWebDialogDelegate {
   // Callback to run if the flow is completed.
   ash::mojom::AssistantSetup::StartAssistantOptInFlowCallback callback_;
 
+  // Type of modality applied to the dialog window.
+  ui::ModalType modal_type_;
+
   DISALLOW_COPY_AND_ASSIGN(AssistantOptInDialog);
 };