Fix ext4 migration histogram value on policy-forced migration

Make policy-forced migration to ext4 report the new
FIRST_SCREEN_START_AUTOMATICALLY UMA stat value instead of
FIRST_SCREEN_RESUME.

The introduction of EncryptionMigrationMode was required to distinguish
between these cases.

BUG=722371

Change-Id: I112406048e82c31762123a32775be445e6e731ab
Reviewed-on: https://chromium-review.googlesource.com/579382Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488590}

chrome/browser/chromeos/login/existing_user_controller.cc

@@ -725,7 +725,7 @@ void ExistingUserController::ShowKioskAutolaunchScreen() {
 
 void ExistingUserController::ShowEncryptionMigrationScreen(
     const UserContext& user_context,
-    bool has_incomplete_migration) {
+    EncryptionMigrationMode migration_mode) {
   host_->StartWizard(OobeScreen::SCREEN_ENCRYPTION_MIGRATION);
 
   EncryptionMigrationScreen* migration_screen =
@@ -733,7 +733,7 @@ void ExistingUserController::ShowEncryptionMigrationScreen(
           host_->GetWizardController()->current_screen());
   DCHECK(migration_screen);
   migration_screen->SetUserContext(user_context);
-  migration_screen->SetShouldResume(has_incomplete_migration);
+  migration_screen->SetMode(migration_mode);
   migration_screen->SetContinueLoginCallback(base::BindOnce(
       &ExistingUserController::ContinuePerformLogin, weak_factory_.GetWeakPtr(),
       login_performer_->auth_mode()));
@@ -985,16 +985,15 @@ void ExistingUserController::OnOldEncryptionDetected(
   if (has_incomplete_migration) {
     // If migration was incomplete, continue migration without checking user
     // policy.
-    ShowEncryptionMigrationScreen(user_context, has_incomplete_migration);
+    ShowEncryptionMigrationScreen(user_context,
+                                  EncryptionMigrationMode::RESUME_MIGRATION);
     return;
   }
 
   if (user_context.GetUserType() == user_manager::USER_TYPE_ARC_KIOSK_APP) {
     // For ARC kiosk, don't check user policy.
-    // Note that migration will start immediately without asking the user - this
-    // is currently checked in EncryptionMigrationScreenHandler.
     ShowEncryptionMigrationScreen(user_context,
-                                  false /* has_incomplete_migration */);
+                                  EncryptionMigrationMode::START_MIGRATION);
     return;
   }
 
@@ -1052,17 +1051,13 @@ void ExistingUserController::OnPolicyFetchResult(
 
   switch (action) {
     case EcryptfsMigrationAction::MIGRATE:
-      // TODO(pmarko): Reusing resume may be wrong from UI perspective, in case
-      // we show a UI mentioning "resume"/"interrupted". If that's the case,
-      // introduce an enum instead of the bool parameter to choose between
-      // ask_user/continue_interrupted_migration/start_migration.
-      // Otherwise, at least rename the bool parameter to indicate that this may
-      // not only mean resuming.
-      ShowEncryptionMigrationScreen(user_context, true);
+      ShowEncryptionMigrationScreen(user_context,
+                                    EncryptionMigrationMode::START_MIGRATION);
       break;
 
     case EcryptfsMigrationAction::ASK_USER:
-      ShowEncryptionMigrationScreen(user_context, false);
+      ShowEncryptionMigrationScreen(user_context,
+                                    EncryptionMigrationMode::ASK_USER);
       break;
 
     case EcryptfsMigrationAction::WIPE:

chrome/browser/chromeos/login/existing_user_controller.h

@@ -20,6 +20,7 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/app_mode/arc/arc_kiosk_app_manager.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
+#include "chrome/browser/chromeos/login/screens/encryption_migration_mode.h"
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/login/signin/token_handle_util.h"
 #include "chrome/browser/chromeos/login/ui/login_display.h"
@@ -216,7 +217,7 @@ class ExistingUserController
 
   // Shows "filesystem encryption migration" screen.
   void ShowEncryptionMigrationScreen(const UserContext& user_context,
-                                     bool has_incomplete_migration);
+                                     EncryptionMigrationMode migration_mode);
 
   // Shows "critical TPM error" screen.
   void ShowTPMError();

chrome/browser/chromeos/login/screens/encryption_migration_mode.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_ENCRYPTION_MIGRATION_MODE_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_ENCRYPTION_MIGRATION_MODE_H_
+
+namespace chromeos {
+
+// Defines the mode of the encryption migration screen.
+enum class EncryptionMigrationMode {
+  // Ask the user if migration should be started.
+  ASK_USER,
+  // Start migration immediately.
+  START_MIGRATION,
+  // Resume incomplete migration.
+  RESUME_MIGRATION
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_ENCRYPTION_MIGRATION_MODE_H_

chrome/browser/chromeos/login/screens/encryption_migration_screen.cc

@@ -53,9 +53,9 @@ void EncryptionMigrationScreen::SetUserContext(
   view_->SetUserContext(user_context);
 }
 
-void EncryptionMigrationScreen::SetShouldResume(bool should_resume) {
+void EncryptionMigrationScreen::SetMode(EncryptionMigrationMode mode) {
   DCHECK(view_);
-  view_->SetShouldResume(should_resume);
+  view_->SetMode(mode);
 }
 
 void EncryptionMigrationScreen::SetContinueLoginCallback(

chrome/browser/chromeos/login/screens/encryption_migration_screen.h

@@ -7,6 +7,7 @@
 
 #include "base/callback_forward.h"
 #include "chrome/browser/chromeos/login/screens/base_screen.h"
+#include "chrome/browser/chromeos/login/screens/encryption_migration_mode.h"
 #include "chrome/browser/chromeos/login/screens/encryption_migration_screen_view.h"
 
 namespace chromeos {
@@ -34,8 +35,8 @@ class EncryptionMigrationScreen
   // Sets the UserContext for a user whose cryptohome should be migrated.
   void SetUserContext(const UserContext& user_context);
 
-  // Sets whether the migration process should resume the previous one or not.
-  void SetShouldResume(bool should_resume);
+  // Sets the migration mode.
+  void SetMode(EncryptionMigrationMode mode);
 
   // Sets a callback, which should be called when the user want to log in to the
   // session from the migration UI.

chrome/browser/chromeos/login/screens/encryption_migration_screen_view.h

@@ -7,6 +7,7 @@
 
 #include "base/callback_forward.h"
 #include "chrome/browser/chromeos/login/oobe_screen.h"
+#include "chrome/browser/chromeos/login/screens/encryption_migration_mode.h"
 
 namespace chromeos {
 
@@ -37,7 +38,7 @@ class EncryptionMigrationScreenView {
   virtual void Hide() = 0;
   virtual void SetDelegate(Delegate* delegate) = 0;
   virtual void SetUserContext(const UserContext& user_context) = 0;
-  virtual void SetShouldResume(bool should_resume) = 0;
+  virtual void SetMode(EncryptionMigrationMode mode) = 0;
   virtual void SetContinueLoginCallback(ContinueLoginCallback callback) = 0;
   virtual void SetupInitialView() = 0;
 };

chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc

@@ -76,6 +76,7 @@ enum class FirstScreen {
   FIRST_SCREEN_RESUME = 1,
   FIRST_SCREEN_LOW_STORAGE = 2,
   FIRST_SCREEN_ARC_KIOSK = 3,
+  FIRST_SCREEN_START_AUTOMATICALLY = 4,
   FIRST_SCREEN_COUNT
 };
 
@@ -219,6 +220,21 @@ void RecordRemoveCryptohomeResultFailure(bool resume, bool arc_kiosk) {
   }
 }
 
+// Chooses the value for the MigrationUIFirstScreen UMA stat. Not used for ARC
+// kiosk.
+FirstScreen GetFirstScreenForMode(chromeos::EncryptionMigrationMode mode) {
+  switch (mode) {
+    case chromeos::EncryptionMigrationMode::ASK_USER:
+      return FirstScreen::FIRST_SCREEN_READY;
+    case chromeos::EncryptionMigrationMode::START_MIGRATION:
+      return FirstScreen::FIRST_SCREEN_START_AUTOMATICALLY;
+    case chromeos::EncryptionMigrationMode::RESUME_MIGRATION:
+      return FirstScreen::FIRST_SCREEN_RESUME;
+    default:
+      NOTREACHED();
+  }
+}
+
 }  // namespace
 
 namespace chromeos {
@@ -257,9 +273,9 @@ void EncryptionMigrationScreenHandler::SetUserContext(
   user_context_ = user_context;
 }
 
-void EncryptionMigrationScreenHandler::SetShouldResume(bool should_resume) {
-  should_resume_ = should_resume;
-  CallJS("setIsResuming", should_resume_);
+void EncryptionMigrationScreenHandler::SetMode(EncryptionMigrationMode mode) {
+  mode_ = mode;
+  CallJS("setIsResuming", IsStartImmediately());
 }
 
 void EncryptionMigrationScreenHandler::SetContinueLoginCallback(
@@ -462,11 +478,10 @@ void EncryptionMigrationScreenHandler::CheckAvailableStorage() {
 
 void EncryptionMigrationScreenHandler::OnGetAvailableStorage(int64_t size) {
   if (size >= arc::kMigrationMinimumAvailableStorage || IsTestingUI()) {
-    if (should_resume_) {
-      RecordFirstScreen(FirstScreen::FIRST_SCREEN_RESUME);
+    RecordFirstScreen(GetFirstScreenForMode(mode_));
+    if (IsStartImmediately()) {
       WaitBatteryAndMigrate();
     } else {
-      RecordFirstScreen(FirstScreen::FIRST_SCREEN_READY);
       UpdateUIState(UIState::READY);
     }
   } else {
@@ -519,7 +534,8 @@ void EncryptionMigrationScreenHandler::OnMountExistingVault(
     cryptohome::MountError return_code,
     const std::string& mount_hash) {
   if (!success || return_code != cryptohome::MOUNT_ERROR_NONE) {
-    RecordMigrationResultMountFailure(should_resume_, IsArcKiosk());
+    RecordMigrationResultMountFailure(IsResumingIncompleteMigration(),
+                                      IsArcKiosk());
     UpdateUIState(UIState::MIGRATION_FAILED);
     return;
   }
@@ -580,9 +596,11 @@ void EncryptionMigrationScreenHandler::OnRemoveCryptohome(
   LOG_IF(ERROR, !success) << "Removing cryptohome failed. return code: "
                           << return_code;
   if (success)
-    RecordRemoveCryptohomeResultSuccess(should_resume_, IsArcKiosk());
+    RecordRemoveCryptohomeResultSuccess(IsResumingIncompleteMigration(),
+                                        IsArcKiosk());
   else
-    RecordRemoveCryptohomeResultFailure(should_resume_, IsArcKiosk());
+    RecordRemoveCryptohomeResultFailure(IsResumingIncompleteMigration(),
+                                        IsArcKiosk());
 
   UpdateUIState(UIState::MIGRATION_FAILED);
 }
@@ -619,7 +637,8 @@ void EncryptionMigrationScreenHandler::OnMigrationProgress(
       CallJS("setMigrationProgress", static_cast<double>(current) / total);
       break;
     case cryptohome::DIRCRYPTO_MIGRATION_SUCCESS:
-      RecordMigrationResultSuccess(should_resume_, IsArcKiosk());
+      RecordMigrationResultSuccess(IsResumingIncompleteMigration(),
+                                   IsArcKiosk());
       // If the battery level decreased during migration, record the consumed
       // battery level.
       if (*current_battery_percent_ < initial_battery_percent_) {
@@ -632,7 +651,8 @@ void EncryptionMigrationScreenHandler::OnMigrationProgress(
       DBusThreadManager::Get()->GetPowerManagerClient()->RequestRestart();
       break;
     case cryptohome::DIRCRYPTO_MIGRATION_FAILED:
-      RecordMigrationResultGeneralFailure(should_resume_, IsArcKiosk());
+      RecordMigrationResultGeneralFailure(IsResumingIncompleteMigration(),
+                                          IsArcKiosk());
       // Stop listening to the progress updates.
       DBusThreadManager::Get()
           ->GetCryptohomeClient()
@@ -649,7 +669,8 @@ void EncryptionMigrationScreenHandler::OnMigrationProgress(
 void EncryptionMigrationScreenHandler::OnMigrationRequested(bool success) {
   if (!success) {
     LOG(ERROR) << "Requesting MigrateToDircrypto failed.";
-    RecordMigrationResultRequestFailure(should_resume_, IsArcKiosk());
+    RecordMigrationResultRequestFailure(IsResumingIncompleteMigration(),
+                                        IsArcKiosk());
     UpdateUIState(UIState::MIGRATION_FAILED);
   }
 }
@@ -664,4 +685,13 @@ void EncryptionMigrationScreenHandler::OnDelayedRecordVisibleScreen(
   UMA_HISTOGRAM_ENUMERATION(kUmaNameVisibleScreen, ui_state, UIState::COUNT);
 }
 
+bool EncryptionMigrationScreenHandler::IsResumingIncompleteMigration() {
+  return mode_ == EncryptionMigrationMode::RESUME_MIGRATION;
+}
+
+bool EncryptionMigrationScreenHandler::IsStartImmediately() {
+  return mode_ == EncryptionMigrationMode::START_MIGRATION ||
+         mode_ == EncryptionMigrationMode::RESUME_MIGRATION;
+}
+
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "chrome/browser/chromeos/login/screens/encryption_migration_mode.h"
 #include "chrome/browser/chromeos/login/screens/encryption_migration_screen_view.h"
 #include "chrome/browser/ui/webui/chromeos/login/base_screen_handler.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
@@ -34,7 +35,7 @@ class EncryptionMigrationScreenHandler : public EncryptionMigrationScreenView,
   void Hide() override;
   void SetDelegate(Delegate* delegate) override;
   void SetUserContext(const UserContext& user_context) override;
-  void SetShouldResume(bool should_resume) override;
+  void SetMode(EncryptionMigrationMode mode) override;
   void SetContinueLoginCallback(ContinueLoginCallback callback) override;
   void SetupInitialView() override;
 
@@ -99,6 +100,12 @@ class EncryptionMigrationScreenHandler : public EncryptionMigrationScreenView,
   // Records UMA about visible screen after delay.
   void OnDelayedRecordVisibleScreen(UIState state);
 
+  // True if |mode_| suggests that we are resuming an incomplete migration.
+  bool IsResumingIncompleteMigration();
+
+  // True if |mode_| suggests that migration should start immediately.
+  bool IsStartImmediately();
+
   device::mojom::WakeLock* GetWakeLock();
 
   Delegate* delegate_ = nullptr;
@@ -114,8 +121,9 @@ class EncryptionMigrationScreenHandler : public EncryptionMigrationScreenView,
   // The callback which is used to log in to the session from the migration UI.
   ContinueLoginCallback continue_login_callback_;
 
-  // True if the system should resume the previous incomplete migration.
-  bool should_resume_ = false;
+  // The migration mode (ask user / start migration automatically / resume
+  // incomplete migratoin).
+  EncryptionMigrationMode mode_ = EncryptionMigrationMode::ASK_USER;
 
   // The current battery level.
   base::Optional<double> current_battery_percent_;

tools/metrics/histograms/enums.xml

@@ -25475,6 +25475,7 @@ from previous Chrome versions.
   <int value="1" label="Resume"/>
   <int value="2" label="LowStorage"/>
   <int value="3" label="ArcKiosk"/>
+  <int value="4" label="StartAutomatically"/>
 </enum>
 
 <enum name="MigrationUIMigrationResult">