Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7709)

Unified Diff: chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc

Issue 2910423002: cros: Add UMA metrics in migration UI to get more information from migration failures. (Closed)
Patch Set: Add Cryptohome.MigrationUI.ShowFailureScreen. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc
index e47c7ad10f96d3a801b44b0f14e9630dd8d51b20..5209e73bf1ce60e839de1e5de3a47b680a544c0e 100644
--- a/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc
@@ -12,10 +12,13 @@
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/metrics/histogram_macros.h"
+#include "base/metrics/user_metrics.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
#include "base/sys_info.h"
#include "base/task_scheduler/post_task.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/arc/arc_migration_constants.h"
#include "chrome/browser/chromeos/login/ui/login_feedback.h"
@@ -46,17 +49,22 @@ constexpr char kCheckStoragePath[] = "/home";
constexpr char kJsApiStartMigration[] = "startMigration";
constexpr char kJsApiSkipMigration[] = "skipMigration";
constexpr char kJsApiRequestRestart[] = "requestRestart";
+constexpr char kJsApiRequestRestartOnFailure[] = "requestRestartOnFailure";
constexpr char kJsApiOpenFeedbackDialog[] = "openFeedbackDialog";
// UMA names.
constexpr char kUmaNameFirstScreen[] = "Cryptohome.MigrationUI.FirstScreen";
constexpr char kUmaNameUserChoice[] = "Cryptohome.MigrationUI.UserChoice";
+constexpr char kUmaNameMigrationResult[] =
+ "Cryptohome.MigrationUI.MigrationResult";
+constexpr char kUmaNameRemoveCryptohomeResult[] =
+ "Cryptohome.MigrationUI.RemoveCryptohomeResult";
constexpr char kUmaNameConsumedBatteryPercent[] =
"Cryptohome.MigrationUI.ConsumedBatteryPercent";
-// This enum must match the numbering for Cryptohome.MigrationUI.FirstScreen in
-// histograms.xml. Do not reorder or remove items, only add new items before
-// FIRST_SCREEN_MAX.
+// This enum must match the numbering for MigrationUIFirstScreen in
+// histograms/enums.xml. Do not reorder or remove items, only add new items
+// before FIRST_SCREEN_COUNT.
enum class FirstScreen {
FIRST_SCREEN_READY = 0,
FIRST_SCREEN_RESUME = 1,
@@ -64,15 +72,37 @@ enum class FirstScreen {
FIRST_SCREEN_COUNT
};
-// This enum must match the numbering for Cryptohome.MigrationUI.UserChoice in
-// histograms.xml. Do not reorder or remove items, only add new items before
-// FIRST_SCREEN_MAX.
+// This enum must match the numbering for MigrationUIUserChoice in
+// histograms/enums.xml. Do not reorder or remove items, only add new items
+// before USER_CHOICE_COUNT.
enum class UserChoice {
USER_CHOICE_UPDATE = 0,
USER_CHOICE_SKIP = 1,
USER_CHOICE_COUNT
};
+// This enum must match the numbering for MigrationUIMigrationResult in
+// histograms/enums.xml. Do not reorder or remove items, only add new items
+// before COUNT.
+enum class MigrationResult {
fukino 2017/05/31 12:21:28 MigrationResult and RemoveCryptohomeResult have th
+ SUCCESS_IN_NEW_MIGRATION = 0,
+ SUCCESS_IN_RESUMED_MIGRATION = 1,
+ FAILURE_IN_NEW_MIGRATION = 2,
+ FAILURE_IN_RESUMED_MIGRATION = 3,
+ COUNT
+};
+
+// This enum must match the numbering for MigrationUIRemoveCryptohomeResult in
+// histograms/enums.xml. Do not reorder or remove items, only add new items
+// before COUNT.
+enum class RemoveCryptohomeResult {
+ SUCCESS_IN_NEW_MIGRATION = 0,
+ SUCCESS_IN_RESUMED_MIGRATION = 1,
+ FAILURE_IN_NEW_MIGRATION = 2,
+ FAILURE_IN_RESUMED_MIGRATION = 3,
+ COUNT
+};
+
bool IsTestingUI() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(
chromeos::switches::kTestEncryptionMigrationUI);
@@ -89,6 +119,36 @@ void RecordUserChoice(UserChoice user_choice) {
static_cast<int>(UserChoice::USER_CHOICE_COUNT));
}
+void RecordMigrationResult(bool success, bool is_resumed_migration) {
+ MigrationResult result =
+ success ? (is_resumed_migration
+ ? MigrationResult::SUCCESS_IN_RESUMED_MIGRATION
+ : MigrationResult::SUCCESS_IN_NEW_MIGRATION)
+ : (is_resumed_migration
+ ? MigrationResult::FAILURE_IN_RESUMED_MIGRATION
+ : MigrationResult::FAILURE_IN_NEW_MIGRATION);
+ UMA_HISTOGRAM_ENUMERATION(kUmaNameMigrationResult, static_cast<int>(result),
+ static_cast<int>(MigrationResult::COUNT));
Ilya Sherman 2017/05/31 23:34:44 nit: static_casts shouldn't be needed anymore when
fukino 2017/06/01 09:28:38 Done.
+}
+
+void RecordRemoveCryptohomeResult(bool success, bool is_resumed_migration) {
+ RemoveCryptohomeResult result =
+ success ? (is_resumed_migration
+ ? RemoveCryptohomeResult::SUCCESS_IN_RESUMED_MIGRATION
+ : RemoveCryptohomeResult::SUCCESS_IN_NEW_MIGRATION)
+ : (is_resumed_migration
+ ? RemoveCryptohomeResult::FAILURE_IN_RESUMED_MIGRATION
+ : RemoveCryptohomeResult::FAILURE_IN_NEW_MIGRATION);
+ UMA_HISTOGRAM_ENUMERATION(kUmaNameRemoveCryptohomeResult,
+ static_cast<int>(result),
+ static_cast<int>(RemoveCryptohomeResult::COUNT));
+}
+
+void RecordShowFailureScreen() {
+ base::RecordAction(
+ base::UserMetricsAction("Cryptohome.MigrationUI.ShowFailureScreen"));
+}
+
} // namespace
namespace chromeos {
@@ -205,6 +265,8 @@ void EncryptionMigrationScreenHandler::RegisterMessages() {
&EncryptionMigrationScreenHandler::HandleSkipMigration);
AddCallback(kJsApiRequestRestart,
&EncryptionMigrationScreenHandler::HandleRequestRestart);
+ AddCallback(kJsApiRequestRestartOnFailure,
+ &EncryptionMigrationScreenHandler::HandleRequestRestartOnFailure);
AddCallback(kJsApiOpenFeedbackDialog,
&EncryptionMigrationScreenHandler::HandleOpenFeedbackDialog);
}
@@ -261,6 +323,12 @@ void EncryptionMigrationScreenHandler::HandleRequestRestart() {
DBusThreadManager::Get()->GetPowerManagerClient()->RequestRestart();
}
+void EncryptionMigrationScreenHandler::HandleRequestRestartOnFailure() {
+ base::RecordAction(base::UserMetricsAction(
+ "Cryptohome.MigrationUI.RestartClickedOnFailure"));
+ DBusThreadManager::Get()->GetPowerManagerClient()->RequestRestart();
+}
+
void EncryptionMigrationScreenHandler::HandleOpenFeedbackDialog() {
const std::string description = base::StringPrintf(
"Auto generated feedback for http://crbug.com/719266.\n"
@@ -398,6 +466,7 @@ void EncryptionMigrationScreenHandler::OnRemoveCryptohome(
cryptohome::MountError return_code) {
LOG_IF(ERROR, !success) << "Removing cryptohome failed. return code: "
<< return_code;
+ RecordRemoveCryptohomeResult(success, should_resume_);
UpdateUIState(UIState::MIGRATION_FAILED);
}
@@ -429,6 +498,7 @@ void EncryptionMigrationScreenHandler::OnMigrationProgress(
CallJS("setMigrationProgress", static_cast<double>(current) / total);
break;
case cryptohome::DIRCRYPTO_MIGRATION_SUCCESS:
+ RecordMigrationResult(true, should_resume_);
// If the battery level decreased during migration, record the consumed
// battery level.
if (*current_battery_percent_ < initial_battery_percent_) {
@@ -441,6 +511,15 @@ void EncryptionMigrationScreenHandler::OnMigrationProgress(
DBusThreadManager::Get()->GetPowerManagerClient()->RequestRestart();
break;
case cryptohome::DIRCRYPTO_MIGRATION_FAILED:
+ RecordMigrationResult(false, should_resume_);
+ // Record an event that the user sees the failure screen.
+ // We record it after delay to make sure that the user was actually able
+ // to see the failure screen. There is a potential race condition where
+ // the user can see only flash of failure screen on shutting down.
+ // crbug.com/727980.
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, base::BindOnce(&RecordShowFailureScreen),
+ base::TimeDelta::FromSeconds(1));
// Stop listening to the progress updates.
DBusThreadManager::Get()
->GetCryptohomeClient()

Powered by Google App Engine
This is Rietveld 408576698