Create OOBE screen for Voice Interaction value proposition

chrome/browser/chromeos/login/wizard_controller.cc

Index: chrome/browser/chromeos/login/wizard_controller.cc
diff --git a/chrome/browser/chromeos/login/wizard_controller.cc b/chrome/browser/chromeos/login/wizard_controller.cc
index 0d52be1bffebc54f6a75ed4d06ca172f3fadf56c..fbb85b7bda4dfba1a1d77abf410f2467d5bbeef2 100644
--- a/chrome/browser/chromeos/login/wizard_controller.cc
+++ b/chrome/browser/chromeos/login/wizard_controller.cc
@@ -30,6 +30,7 @@
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.h"
 #include "chrome/browser/chromeos/customization/customization_document.h"
 #include "chrome/browser/chromeos/login/enrollment/auto_enrollment_check_screen.h"
 #include "chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h"
@@ -52,6 +53,7 @@
 #include "chrome/browser/chromeos/login/screens/terms_of_service_screen.h"
 #include "chrome/browser/chromeos/login/screens/update_screen.h"
 #include "chrome/browser/chromeos/login/screens/user_image_screen.h"
+#include "chrome/browser/chromeos/login/screens/voice_interaction_value_prop_screen.h"
 #include "chrome/browser/chromeos/login/screens/wrong_hwid_screen.h"
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
@@ -85,6 +87,8 @@
 #include "chromeos/settings/cros_settings_provider.h"
 #include "chromeos/settings/timezone_settings.h"
 #include "chromeos/timezone/timezone_provider.h"
+#include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_service_manager.h"
 #include "components/crash/content/app/breakpad_linux.h"
 #include "components/pairing/bluetooth_controller_pairing_controller.h"
 #include "components/pairing/bluetooth_host_pairing_controller.h"
@@ -424,6 +428,9 @@ BaseScreen* WizardController::CreateScreen(OobeScreen screen) {
   } else if (screen == OobeScreen::SCREEN_ENCRYPTION_MIGRATION) {
     return new EncryptionMigrationScreen(
         this, oobe_ui_->GetEncryptionMigrationScreenView());
+  } else if (screen == OobeScreen::SCREEN_VOICE_INTERACTION_VALUE_PROP) {
+    return new VoiceInteractionValuePropScreen(
+        this, oobe_ui_->GetVoiceInteractionValuePropScreenView());
   }
 
   return nullptr;
@@ -545,27 +552,7 @@ void WizardController::ShowTermsOfServiceScreen() {
 }
 
 void WizardController::ShowArcTermsOfServiceScreen() {
-  bool show_arc_terms = false;
-  const Profile* profile = ProfileManager::GetActiveUserProfile();
-
-  const base::CommandLine* command_line =
-      base::CommandLine::ForCurrentProcess();
-  if (!command_line->HasSwitch(chromeos::switches::kEnableArcOOBEOptIn)) {
-    VLOG(1) << "Skip ARC Terms of Service screen because ARC OOBE OptIn is "
-            << "disabled.";
-  } else if (!user_manager::UserManager::Get()->IsUserLoggedIn()) {
-    VLOG(1) << "Skip ARC Terms of Service screen because user is not "
-            << "logged in.";
-  } else if (!arc::IsArcAllowedForProfile(profile)) {
-    VLOG(1) << "Skip ARC Terms of Service screen because ARC is not allowed.";
-  } else if (profile->GetPrefs()->IsManagedPreference(prefs::kArcEnabled) &&
-             !profile->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
-    VLOG(1) << "Skip ARC Terms of Service screen because ARC is disabled.";
-  } else {
-    show_arc_terms = true;
-  }
-
-  if (show_arc_terms) {
+  if (ShouldShowArcTerms()) {
     VLOG(1) << "Showing ARC Terms of Service screen.";
     UpdateStatusAreaVisibilityForScreen(
         OobeScreen::SCREEN_ARC_TERMS_OF_SERVICE);
@@ -640,6 +627,18 @@ void WizardController::ShowEncryptionMigrationScreen() {
   SetCurrentScreen(GetScreen(OobeScreen::SCREEN_ENCRYPTION_MIGRATION));
 }
 
+void WizardController::ShowVoiceInteractionValuePropScreen() {
+  if (ShouldShowVoiceInteractionValueProp()) {
+    VLOG(1) << "Showing voice interaction value prop screen.";
+    UpdateStatusAreaVisibilityForScreen(
+        OobeScreen::SCREEN_VOICE_INTERACTION_VALUE_PROP);
+    SetCurrentScreen(
+        GetScreen(OobeScreen::SCREEN_VOICE_INTERACTION_VALUE_PROP));
+  } else {
+    OnOobeFlowFinished();
+  }
+}
+
 void WizardController::SkipToLoginForTesting(
     const LoginScreenContext& context) {
   VLOG(1) << "SkipToLoginForTesting.";
@@ -763,24 +762,7 @@ void WizardController::OnUserImageSelected() {
       return;
     }
   }
-  if (!time_oobe_started_.is_null()) {
-    base::TimeDelta delta = base::Time::Now() - time_oobe_started_;
-    UMA_HISTOGRAM_CUSTOM_TIMES(
-        "OOBE.BootToSignInCompleted",
-        delta,
-        base::TimeDelta::FromMilliseconds(10),
-        base::TimeDelta::FromMinutes(30),
-        100);
-    time_oobe_started_ = base::Time();
-  }
-
-  // Launch browser and delete login host controller.
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&UserSessionManager::DoBrowserLaunch,
-                 base::Unretained(UserSessionManager::GetInstance()),
-                 ProfileManager::GetActiveUserProfile(), host_));
-  host_ = nullptr;
+  ShowVoiceInteractionValuePropScreen();
 }
 
 void WizardController::OnUserImageSkipped() {
@@ -851,6 +833,20 @@ void WizardController::OnArcTermsOfServiceFinished() {
   ShowUserImageScreen();
 }
 
+void WizardController::OnVoiceInteractionValuePropSkipped() {
+  OnOobeFlowFinished();
+}
+
+void WizardController::OnVoiceInteractionValuePropAccepted() {
+  // Start voice interaction setup wizard in container
+  arc::ArcVoiceInteractionFrameworkService* service =
+      arc::ArcServiceManager::Get()
+          ->GetService<arc::ArcVoiceInteractionFrameworkService>();
+  if (service)
+    service->StartVoiceInteractionSetupWizard();
+  OnOobeFlowFinished();

[xc, 2017/05/31 18:37:52]

I am not sure we want to call this here.  It would start the browser and at the
same time with starting voice interaction suw. 

+}
+
 void WizardController::OnControllerPairingFinished() {
   ShowAutoEnrollmentCheckScreen();
 }
@@ -865,6 +861,24 @@ void WizardController::OnAutoEnrollmentCheckCompleted() {
           weak_factory_.GetWeakPtr()));
 }
 
+void WizardController::OnOobeFlowFinished() {
+  if (!time_oobe_started_.is_null()) {
+    base::TimeDelta delta = base::Time::Now() - time_oobe_started_;
+    UMA_HISTOGRAM_CUSTOM_TIMES("OOBE.BootToSignInCompleted", delta,

[xc, 2017/05/31 18:37:52]

Not sure what the original definition of "OOBE.BootToSignInCompleted" histogram.
 Can you double check the comments in histogram xml and see if we have changed
the meaning?

[yueli, 2017/06/02 02:03:46]

Seems it log the time from boot to sign-in completed. And the only use case is
here

+                               base::TimeDelta::FromMilliseconds(10),
+                               base::TimeDelta::FromMinutes(30), 100);
+    time_oobe_started_ = base::Time();
+  }
+
+  // Launch browser and delete login host controller.
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&UserSessionManager::DoBrowserLaunch,
+                 base::Unretained(UserSessionManager::GetInstance()),
+                 ProfileManager::GetActiveUserProfile(), host_));
+  host_ = nullptr;
+}
+
 void WizardController::OnDeviceDisabledChecked(bool device_disabled) {
   prescribed_enrollment_config_ = g_browser_process->platform_part()
                                       ->browser_policy_connector_chromeos()
@@ -1089,6 +1103,8 @@ void WizardController::AdvanceToScreen(OobeScreen screen) {
     ShowDeviceDisabledScreen();
   } else if (screen == OobeScreen::SCREEN_ENCRYPTION_MIGRATION) {
     ShowEncryptionMigrationScreen();
+  } else if (screen == OobeScreen::SCREEN_VOICE_INTERACTION_VALUE_PROP) {
+    ShowVoiceInteractionValuePropScreen();
   } else if (screen != OobeScreen::SCREEN_TEST_NO_WINDOW) {
     if (is_out_of_box_) {
       time_oobe_started_ = base::Time::Now();
@@ -1197,6 +1213,12 @@ void WizardController::OnExit(BaseScreen& /* screen */,
     case ScreenExitCode::CONTROLLER_PAIRING_FINISHED:
       OnControllerPairingFinished();
       break;
+    case ScreenExitCode::VOICE_INTERACTION_VALUE_PROP_SKIPPED:
+      OnVoiceInteractionValuePropSkipped();
+      break;
+    case ScreenExitCode::VOICE_INTERACTION_VALUE_PROP_ACCEPTED:
+      OnVoiceInteractionValuePropAccepted();
+      break;
     default:
       NOTREACHED();
   }
@@ -1490,6 +1512,47 @@ bool WizardController::IsRemoraPairingOobe() const {
       switches::kHostPairingOobe);
 }
 
+bool WizardController::ShouldShowArcTerms() const {
+  const Profile* profile = ProfileManager::GetActiveUserProfile();

[xiyuan, 2017/05/30 19:26:21]

nit: Move this after IsUserLoggedIn() test.

[yueli, 2017/06/02 02:03:46]

Done.

+
+  const base::CommandLine* command_line =
+      base::CommandLine::ForCurrentProcess();
+  if (!command_line->HasSwitch(chromeos::switches::kEnableArcOOBEOptIn)) {
+    VLOG(1) << "Skip ARC Terms of Service screen because ARC OOBE OptIn is "
+            << "disabled.";
+    return false;
+  }
+  if (!user_manager::UserManager::Get()->IsUserLoggedIn()) {
+    VLOG(1) << "Skip ARC Terms of Service screen because user is not "
+            << "logged in.";
+    return false;
+  }
+  if (!arc::IsArcAllowedForProfile(profile)) {
+    VLOG(1) << "Skip ARC Terms of Service screen because ARC is not allowed.";
+    return false;
+  }
+  if (profile->GetPrefs()->IsManagedPreference(prefs::kArcEnabled) &&
+      !profile->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
+    VLOG(1) << "Skip ARC Terms of Service screen because ARC is disabled.";
+    return false;
+  }
+  return true;
+}
+
+bool WizardController::ShouldShowVoiceInteractionValueProp() const {
+  if (!ShouldShowArcTerms()) {
+    VLOG(1) << "Skip Voice Interaction Value Prop screen because Arc Terms is "
+            << "skipped.";
+    return false;
+  }

[xiyuan, 2017/05/30 19:26:21]

What if we show ARC terms but user declined it ? Think we should call
IsArcPlayStoreEnabledForProfile() to skip if user declined ARC terms.

[xc, 2017/05/31 18:37:52]

+1 IsArcPlayStoreEnabledForProfile() check should replace ShouldShowArcTerms()
check.

[yueli, 2017/06/02 02:03:46]

Done.

+  if (!chromeos::switches::IsVoiceInteractionEnabled()) {
+    VLOG(1) << "Skip Voice Interaction Value Prop screen because voice "
+            << "interaction service is disabled.";
+    return false;
+  }
+  return true;
+}
+
 void WizardController::MaybeStartListeningForSharkConnection() {
   // We shouldn't be here if we are running pairing OOBE already.
   if (IsControllerDetected())