Remove RenameCryptohome from HomedirMethods.

Bug: 741274
Change-Id: Ib6a62720c7f3e89a7d480a63c4bc4a470d75aed9
Reviewed-on: https://chromium-review.googlesource.com/861368
Commit-Queue: Maajid <maajid@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530423}

chrome/browser/chromeos/policy/pre_signin_policy_fetcher.cc

@@ -90,7 +90,7 @@ bool PreSigninPolicyFetcher::ForceTimeoutForTesting() {
 
 void PreSigninPolicyFetcher::OnMountTemporaryUserHome(
     base::Optional<cryptohome::BaseReply> reply) {
-  if (BaseReplyToMountError(reply) != cryptohome::MOUNT_ERROR_NONE) {
+  if (MountExReplyToMountError(reply) != cryptohome::MOUNT_ERROR_NONE) {
     LOG(ERROR) << "Temporary user home mount failed.";
     NotifyCallback(PolicyFetchResult::ERROR, nullptr);
     return;

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

@@ -581,11 +581,13 @@ void EncryptionMigrationScreenHandler::StartMigration() {
 
 void EncryptionMigrationScreenHandler::OnMountExistingVault(
     base::Optional<cryptohome::BaseReply> reply) {
-  if (cryptohome::BaseReplyToMountError(reply) !=
-      cryptohome::MOUNT_ERROR_NONE) {
+  cryptohome::MountError return_code =
+      cryptohome::MountExReplyToMountError(reply);
+  if (return_code != cryptohome::MOUNT_ERROR_NONE) {
     RecordMigrationResultMountFailure(IsResumingIncompleteMigration(),
                                       IsArcKiosk());
     UpdateUIState(UIState::MIGRATION_FAILED);
+    LOG(ERROR) << "Mount existing vault failed. Error: " << return_code;
     return;
   }
 

chromeos/cryptohome/cryptohome_util.cc

@@ -17,23 +17,37 @@
 
 namespace cryptohome {
 
-MountError BaseReplyToMountError(const base::Optional<BaseReply>& reply) {
+namespace {
+
+bool IsEmpty(const base::Optional<BaseReply>& reply) {
   if (!reply.has_value()) {
-    LOGIN_LOG(ERROR) << "MountEx failed with no reply.";
-    return MOUNT_ERROR_FATAL;
+    LOGIN_LOG(ERROR) << "Cryptohome call failed with empty reply.";
+    return true;
   }
+  return false;
+}
+
+}  // namespace
+
+MountError MountExReplyToMountError(const base::Optional<BaseReply>& reply) {
+  if (IsEmpty(reply))
+    return MOUNT_ERROR_FATAL;
+
   if (!reply->HasExtension(MountReply::reply)) {
     LOGIN_LOG(ERROR) << "MountEx failed with no MountReply extension in reply.";
     return MOUNT_ERROR_FATAL;
   }
-  if (reply->has_error() && reply->error() != CRYPTOHOME_ERROR_NOT_SET) {
-    LOGIN_LOG(ERROR) << "MountEx failed (CryptohomeErrorCode): "
-                     << reply->error();
-  }
   return CryptohomeErrorToMountError(reply->error());
 }
 
-const std::string& BaseReplyToMountHash(const BaseReply& reply) {
+MountError BaseReplyToMountError(const base::Optional<BaseReply>& reply) {
+  if (IsEmpty(reply))
+    return MOUNT_ERROR_FATAL;
+
+  return CryptohomeErrorToMountError(reply->error());
+}
+
+const std::string& MountExReplyToMountHash(const BaseReply& reply) {
   return reply.GetExtension(MountReply::reply).sanitized_username();
 }
 

chromeos/cryptohome/cryptohome_util.h

@@ -21,11 +21,17 @@ namespace cryptohome {
 CHROMEOS_EXPORT MountError
 BaseReplyToMountError(const base::Optional<BaseReply>& reply);
 
+// Returns a MountError code from the MountEx |reply| returning
+// MOUNT_ERROR_NONE if the reply is well-formed and there is no error.
+CHROMEOS_EXPORT MountError
+MountExReplyToMountError(const base::Optional<BaseReply>& reply);
+
 // Extracts the mount hash from |reply|.
 // This method assumes |reply| is well-formed. To check if a reply
 // is well-formed, callers can check if BaseReplyToMountError returns
 // MOUNT_ERROR_NONE.
-CHROMEOS_EXPORT const std::string& BaseReplyToMountHash(const BaseReply& reply);
+CHROMEOS_EXPORT const std::string& MountExReplyToMountHash(
+    const BaseReply& reply);
 
 // Creates an AuthorizationRequest from the given secret and label.
 CHROMEOS_EXPORT AuthorizationRequest

chromeos/cryptohome/homedir_methods.cc

@@ -82,15 +82,6 @@ class HomedirMethodsImpl : public HomedirMethods {
                        weak_ptr_factory_.GetWeakPtr(), callback));
   }
 
-  void RenameCryptohome(const Identification& id_from,
-                        const Identification& id_to,
-                        const Callback& callback) override {
-    DBusThreadManager::Get()->GetCryptohomeClient()->RenameCryptohome(
-        id_from, id_to,
-        base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback,
-                       weak_ptr_factory_.GetWeakPtr(), callback));
-  }
-
   void GetAccountDiskUsage(
       const Identification& id,
       const GetAccountDiskUsageCallback& callback) override {

chromeos/cryptohome/homedir_methods.h

@@ -77,12 +77,6 @@ class CHROMEOS_EXPORT HomedirMethods {
                            const RemoveKeyRequest& request,
                            const Callback& callback) = 0;
 
-  // Asks cryptohomed to change cryptohome identification |id_from| to |id_to|,
-  // which results in cryptohome directory renaming.
-  virtual void RenameCryptohome(const Identification& id_from,
-                                const Identification& id_to,
-                                const Callback& callback) = 0;
-
   // Asks cryptohomed to compute the size of cryptohome for user identified by
   // |id|.
   virtual void GetAccountDiskUsage(

chromeos/cryptohome/mock_homedir_methods.h

@@ -47,10 +47,6 @@ class CHROMEOS_EXPORT MockHomedirMethods : public HomedirMethods {
                     const AuthorizationRequest& auth,
                     const UpdateKeyRequest& request,
                     const Callback& callback));
-  MOCK_METHOD3(RenameCryptohome,
-               void(const Identification& id_from,
-                    const Identification& id_to,
-                    const Callback& callback));
   MOCK_METHOD2(GetAccountDiskUsage,
                void(const Identification& id,
                     const GetAccountDiskUsageCallback& callback));

chromeos/login/auth/cryptohome_authenticator.cc

@@ -141,11 +141,13 @@ void OnMount(const base::WeakPtr<AuthAttemptState>& attempt,
   chromeos::LoginEventRecorder::Get()->AddLoginTimeMarker(
       public_mount ? "CryptohomeMountPublic-End" : "CryptohomeMount-End",
       false);
-  attempt->RecordCryptohomeStatus(BaseReplyToMountError(reply));
-  if (attempt->cryptohome_code() == cryptohome::MOUNT_ERROR_NONE)
-    attempt->RecordUsernameHash(BaseReplyToMountHash(reply.value()));
-  else
+  attempt->RecordCryptohomeStatus(MountExReplyToMountError(reply));
+  if (attempt->cryptohome_code() == cryptohome::MOUNT_ERROR_NONE) {
+    attempt->RecordUsernameHash(MountExReplyToMountHash(reply.value()));
+  } else {
+    LOGIN_LOG(ERROR) << "MountEx failed. Error: " << attempt->cryptohome_code();
     attempt->RecordUsernameHashFailed();
+  }
   resolver->Resolve();
 }
 
@@ -187,7 +189,7 @@ void DoMount(const base::WeakPtr<AuthAttemptState>& attempt,
   auth_key->set_secret(key->GetSecret());
   DBusThreadManager::Get()->GetCryptohomeClient()->MountEx(
       cryptohome::Identification(attempt->user_context.GetAccountId()), auth,
-      mount, base::Bind(&OnMount, attempt, resolver));
+      mount, base::BindOnce(&OnMount, attempt, resolver));
 }
 
 // Handle cryptohome migration status.
@@ -195,12 +197,12 @@ void OnCryptohomeRenamed(const base::WeakPtr<AuthAttemptState>& attempt,
                          scoped_refptr<CryptohomeAuthenticator> resolver,
                          bool ephemeral,
                          bool create_if_nonexistent,
-                         bool success,
-                         cryptohome::MountError return_code) {
+                         base::Optional<cryptohome::BaseReply> reply) {
+  cryptohome::MountError return_code = cryptohome::BaseReplyToMountError(reply);
   chromeos::LoginEventRecorder::Get()->AddLoginTimeMarker(
       "CryptohomeRename-End", false);
   const AccountId account_id = attempt->user_context.GetAccountId();
-  if (success) {
+  if (return_code == cryptohome::MOUNT_ERROR_NONE) {
     cryptohome::SetGaiaIdMigrationStatusDone(account_id);
     UMACryptohomeMigrationToGaiaId(CryptohomeMigrationToGaiaId::SUCCESS);
   } else {
@@ -255,11 +257,11 @@ void EnsureCryptohomeMigratedToGaiaId(
     const std::string cryptohome_id_to =
         attempt->user_context.GetAccountId().GetAccountIdKey();
 
-    cryptohome::HomedirMethods::GetInstance()->RenameCryptohome(
+    DBusThreadManager::Get()->GetCryptohomeClient()->RenameCryptohome(
         cryptohome::Identification::FromString(cryptohome_id_from),
         cryptohome::Identification::FromString(cryptohome_id_to),
-        base::Bind(&OnCryptohomeRenamed, attempt, resolver, ephemeral,
-                   create_if_nonexistent));
+        base::BindOnce(&OnCryptohomeRenamed, attempt, resolver, ephemeral,
+                       create_if_nonexistent));
     return;
   }
   if (!already_migrated && has_account_key) {
@@ -434,7 +436,7 @@ void MountPublic(const base::WeakPtr<AuthAttemptState>& attempt,
   DBusThreadManager::Get()->GetCryptohomeClient()->MountEx(
       cryptohome::Identification(attempt->user_context.GetAccountId()),
       cryptohome::AuthorizationRequest(), mount,
-      base::Bind(&OnMount, attempt, resolver));
+      base::BindOnce(&OnMount, attempt, resolver));
 }
 
 // Calls cryptohome's key migration method.

chromeos/login/auth/extended_authenticator_impl.cc

@@ -266,11 +266,12 @@ void ExtendedAuthenticatorImpl::OnMountComplete(
     const UserContext& user_context,
     const ResultCallback& success_callback,
     base::Optional<cryptohome::BaseReply> reply) {
-  cryptohome::MountError return_code = cryptohome::BaseReplyToMountError(reply);
+  cryptohome::MountError return_code =
+      cryptohome::MountExReplyToMountError(reply);
   RecordEndMarker(time_marker);
   if (return_code == cryptohome::MOUNT_ERROR_NONE) {
     const std::string& mount_hash =
-        cryptohome::BaseReplyToMountHash(reply.value());
+        cryptohome::MountExReplyToMountHash(reply.value());
     if (!success_callback.is_null())
       success_callback.Run(mount_hash);
     if (old_consumer_) {
@@ -280,6 +281,7 @@ void ExtendedAuthenticatorImpl::OnMountComplete(
     }
     return;
   }
+  LOG(ERROR) << "MountEx failed. Error: " << return_code;
   AuthState state = FAILED_MOUNT;
   if (return_code == cryptohome::MOUNT_ERROR_TPM_COMM_ERROR ||
       return_code == cryptohome::MOUNT_ERROR_TPM_DEFEND_LOCK ||