cros: Remove user directory when encryption migration fails.

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

 // 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.
 
 #include "chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.h"
 
 #include <string>
 #include <utility>
 
 #include "ash/system/devicetype_utils.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/sys_info.h"
 #include "base/task_scheduler/post_task.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/chromeos_switches.h"
+#include "chromeos/cryptohome/async_method_caller.h"
 #include "chromeos/cryptohome/homedir_methods.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "components/login/localized_values_builder.h"
 #include "content/public/browser/browser_thread.h"
 #include "device/power_save_blocker/power_save_blocker.h"
 #include "ui/base/text/bytes_formatting.h"
 
@@ skipping 276 matching lines @@
             content::BrowserThread::FILE)));
   }
 }
 
 void EncryptionMigrationScreenHandler::StopBlockingPowerSave() {
   if (power_save_blocker_.get()) {
     power_save_blocker_.reset();
   }
 }
 
+void EncryptionMigrationScreenHandler::RemoveCryptohome() {
+  cryptohome::AsyncMethodCaller::GetInstance()->AsyncRemove(

[fukino, 2017/04/26 11:01:43]

Instead of calling AsyncRemove directly, we should be able to call
UserManager::RemoveUser() to avoid http://crbug.com/715474, but I'm concerned
about that UserManager::RemoveUser() can't remove owner profile.

Maybe we should remove cryptohome by AsyncRemove call and call
UserManager::RemoveUserList() to remove user pod?

[xiyuan, 2017/04/26 17:48:24]

Agree that we should not use RemoveUser().

We can use 
  UserManager::Get()->SaveUserOAuthStatus(
        account_id, user_manager::User::OAUTH2_TOKEN_STATUS_INVALID);

to force user to go through Gaia on the next sign-in. And Gaia flow would
restore the token status on success.

[fukino, 2017/04/27 01:38:23]

Thank you for the suggestion!
Updated the CL to invalidate token status, and confirmed that the user with
missing cryptohome can log in in the first attempt after the call.
Done.

+      cryptohome::Identification(user_context_.GetAccountId()),
+      base::Bind(&EncryptionMigrationScreenHandler::OnRemoveCryptohome,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void EncryptionMigrationScreenHandler::OnRemoveCryptohome(
+    bool success,
+    cryptohome::MountError return_code) {
+  if (!success)
+    LOG(ERROR) << "Removing cryptohome failed. return code: " << return_code;

[xiyuan, 2017/04/26 17:48:24]

nit: LOG_IF(ERROR, !success) << ....

[fukino, 2017/04/27 01:38:23]

Done.

+
+  UpdateUIState(UIState::MIGRATION_FAILED);
+}
+
 cryptohome::KeyDefinition EncryptionMigrationScreenHandler::GetAuthKey() {
   // |auth_key| is created in the same manner as CryptohomeAuthenticator.
   const Key* key = user_context_.GetKey();
   // If the |key| is a plain text password, crash rather than attempting to
   // mount the cryptohome with a plain text password.
   CHECK_NE(Key::KEY_TYPE_PASSWORD_PLAIN, key->GetKeyType());
   // Set the authentication's key label to an empty string, which is a wildcard
   // allowing any key to match. This is necessary because cryptohomes created by
   // Chrome OS M38 and older will have a legacy key with no label while those
   // created by Chrome OS M39 and newer will have a key with the label
@@ skipping 12 matching lines @@
       break;
     case cryptohome::DIRCRYPTO_MIGRATION_IN_PROGRESS:
       UpdateUIState(UIState::MIGRATING);
       CallJS("setMigrationProgress", static_cast<double>(current) / total);
       break;
     case cryptohome::DIRCRYPTO_MIGRATION_SUCCESS:
       // Restart immediately after successful migration.
       DBusThreadManager::Get()->GetPowerManagerClient()->RequestRestart();
       break;
     case cryptohome::DIRCRYPTO_MIGRATION_FAILED:
-      UpdateUIState(UIState::MIGRATION_FAILED);
       // Stop listening to the progress updates.
       DBusThreadManager::Get()
           ->GetCryptohomeClient()
           ->SetDircryptoMigrationProgressHandler(
               CryptohomeClient::DircryptoMigrationProgessHandler());
+      // Shows error screen after removing user directory is completed.
+      RemoveCryptohome();
       break;
     default:
       break;
   }
 }
 
 void EncryptionMigrationScreenHandler::OnMigrationRequested(bool success) {
-  // This function is called when MigrateToDircrypto is correctly requested.
-  // It does not mean that the migration is completed. We should know the
-  // completion by DircryptoMigrationProgressHandler. success == false means a
-  // failure in DBus communication.
-  // TODO(fukino): Handle this case. Should we retry or restart?
-  DCHECK(success);
+  if (!success) {
+    LOG(ERROR) << "Requesting MigrateToDircrypto failed. Removing cryptohome.";
+    RemoveCryptohome();

[xiyuan, 2017/04/26 17:48:24]

I think we should only RemoveCryptohome() only when migration really fails. Do
this on failed MigrateToDircrypto call feels too aggressive.

Maybe only do
  UpdateUIState(UIState::MIGRATION_FAILED);

[fukino, 2017/04/27 01:38:23]

Done.

+  }
 }
 
 }  // namespace chromeos