Shows error dialog when MGS provisioning fails in ARC++

Calls to StartArc are wrapped in a timeout check for all managed users, except Kiosk and demo sessions.
Correct initialization of the error reporting infrastructure for MGS interactive sessions.

BUG=b:156271583
TEST=Tested manually by forcing ARC to fail.

Change-Id: I2d914e73d2a08f6514eae90dce3566d5256f73e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2230497
Commit-Queue: Giovanni Pezzino <giovax@google.com>
Reviewed-by: Josh Horwich <jhorwich@chromium.org>
Reviewed-by: Long Cheng <lgcheng@google.com>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783847}

chrome/browser/chromeos/arc/session/arc_session_manager.cc

@@ -154,6 +154,32 @@ bool ShouldLaunchPlayStoreApp(Profile* profile,
   return true;
 }
 
+// Defines the conditions that require UI to present eventual error conditions
+// to the end user.
+//
+// Don't show UI for ARC Kiosk because the only one UI in kiosk mode must
+// be the kiosk app. In case of error the UI will be useless as well, because
+// in typical use case there will be no one nearby the kiosk device, who can
+// do some action to solve the problem be means of UI.
+// Same considerations apply for MGS sessions in Demo Mode.
+// All other managed sessions will be attended by a user and require an error
+// UI.
+bool ShouldUseErrorDialog() {
+  if (!g_ui_enabled)
+    return false;
+
+  if (IsArcOptInVerificationDisabled())
+    return false;
+
+  if (IsArcKioskMode())
+    return false;
+
+  if (chromeos::DemoSession::IsDeviceInDemoMode())
+    return false;
+
+  return true;
+}
+
 void ResetStabilityMetrics() {
   // TODO(shaochuan): Make this an event observable by StabilityMetricsManager
   // and eliminate this null check.
@@ -538,13 +564,7 @@ void ArcSessionManager::Initialize() {
   // ARC support Chrome app is rarely used (only opt-in and re-auth flow).
   // So, it may be better to initialize it lazily.
   // TODO(hidehiko): Revisit to think about lazy initialization.
-  //
-  // Don't show UI for ARC Kiosk because the only one UI in kiosk mode must
-  // be the kiosk app. In case of error the UI will be useless as well, because
-  // in typical use case there will be no one nearby the kiosk device, who can
-  // do some action to solve the problem be means of UI.
-  if (g_ui_enabled && !IsArcOptInVerificationDisabled() &&
-      !IsRobotOrOfflineDemoAccountMode()) {
+  if (ShouldUseErrorDialog()) {
     DCHECK(!support_host_);
     support_host_ = std::make_unique<ArcSupportHost>(profile_);
     support_host_->SetErrorDelegate(this);
@@ -1020,11 +1040,6 @@ void ArcSessionManager::OnAndroidManagementChecked(
   switch (result) {
     case policy::AndroidManagementClient::Result::UNMANAGED:
       VLOG(1) << "Starting ARC for first sign in.";
-      sign_in_start_time_ = base::TimeTicks::Now();
-      arc_sign_in_timer_.Start(
-          FROM_HERE, GetArcSignInTimeout(),
-          base::BindOnce(&ArcSessionManager::OnArcSignInTimeout,
-                         weak_ptr_factory_.GetWeakPtr()));
       StartArc();
       // Since opt-in is an explicit user (or admin) action, relax the
       // cgroups restriction now.
@@ -1091,6 +1106,8 @@ void ArcSessionManager::StartArc() {
       << state_;
   state_ = State::ACTIVE;
 
+  MaybeStartTimer();
+
   // ARC must be started only if no pending data removal request exists.
   DCHECK(!profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested));
 
@@ -1210,6 +1227,22 @@ void ArcSessionManager::MaybeReenableArc() {
   RequestEnableImpl();
 }
 
+// Starts a timer to check if provisioning takes too loong.
+// The timer will not be set if this device was previously provisioned
+// successfully.
+void ArcSessionManager::MaybeStartTimer() {
+  if (IsArcProvisioned(profile_)) {
+    return;
+  }
+
+  VLOG(1) << "Setup provisioning timer";
+  sign_in_start_time_ = base::TimeTicks::Now();
+  arc_sign_in_timer_.Start(
+      FROM_HERE, GetArcSignInTimeout(),
+      base::BindOnce(&ArcSessionManager::OnArcSignInTimeout,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
 void ArcSessionManager::OnWindowClosed() {
   CancelAuthCode();
 }

chrome/browser/chromeos/arc/session/arc_session_manager.h

@@ -374,6 +374,11 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   // is fixed.
   void MaybeReenableArc();
 
+  // Starts a timer to check if provisioning takes too long.
+  // The timer will not be set if this device was previously provisioned
+  // successfully.
+  void MaybeStartTimer();
+
   // Requests the support host (if it exists) to show the error, and notifies
   // the observers.
   void ShowArcSupportHostError(ArcSupportHost::Error error,

chrome/browser/chromeos/arc/session/arc_session_manager_unittest.cc

@@ -325,9 +325,10 @@ TEST_F(ArcSessionManagerTest, BaseWorkflow) {
   arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
   ASSERT_EQ(ArcSessionManager::State::CHECKING_ANDROID_MANAGEMENT,
             arc_session_manager()->state());
+  EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
   arc_session_manager()->StartArcForTesting();
 
-  EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
+  EXPECT_FALSE(arc_session_manager()->sign_in_start_time().is_null());
   EXPECT_FALSE(arc_session_manager()->arc_start_time().is_null());
 
   ASSERT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
@@ -509,7 +510,9 @@ TEST_F(ArcSessionManagerTest, Provisioning_Success) {
   arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
   ASSERT_EQ(ArcSessionManager::State::CHECKING_ANDROID_MANAGEMENT,
             arc_session_manager()->state());
+  EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
   arc_session_manager()->StartArcForTesting();
+  EXPECT_FALSE(arc_session_manager()->sign_in_start_time().is_null());
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
 
   // Here, provisining is not yet completed, so kArcSignedIn should be false.
@@ -521,7 +524,6 @@ TEST_F(ArcSessionManagerTest, Provisioning_Success) {
   arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
   EXPECT_TRUE(prefs->GetBoolean(prefs::kArcSignedIn));
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
-  EXPECT_TRUE(arc_session_manager()->sign_in_start_time().is_null());
   EXPECT_TRUE(arc_session_manager()->IsPlaystoreLaunchRequestedForTesting());
 }