Clean up Smart Lock cryptohome keys logic:

chrome/browser/chromeos/login/easy_unlock/easy_unlock_key_manager.cc

 // Copyright 2014 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/chromeos/login/easy_unlock/easy_unlock_key_manager.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
@@ skipping 12 matching lines @@
 const char kKeyPsk[] = "psk";
 
 const char kKeyLabelPrefix[] = "easy-unlock-";
 
 const char kPermitPermitIdFormat[] = "permit://google.com/easyunlock/v1/%s";
 const char kPermitTypeLicence[] = "licence";
 
 }  // namespace
 
 EasyUnlockKeyManager::EasyUnlockKeyManager()
-    : operation_id_(0),
-      weak_ptr_factory_(this) {
+    : operation_in_progress_(false), weak_ptr_factory_(this) {
 }
 
 EasyUnlockKeyManager::~EasyUnlockKeyManager() {
-  STLDeleteContainerPairSecondPointers(get_keys_ops_.begin(),
-                                       get_keys_ops_.end());
 }
 
 void EasyUnlockKeyManager::RefreshKeys(const UserContext& user_context,
                                        const base::ListValue& remote_devices,
                                        const RefreshKeysCallback& callback) {
-  // Must have the secret.
-  DCHECK(!user_context.GetKey()->GetSecret().empty());
-
   EasyUnlockDeviceKeyDataList devices;
   if (!RemoteDeviceListToDeviceDataList(remote_devices, &devices))
     devices.clear();
 
-  // Only one pending request.
-  DCHECK(!HasPendingOperations());
-  create_keys_op_.reset(new EasyUnlockCreateKeysOperation(
-      user_context,
-      devices,
-      base::Bind(&EasyUnlockKeyManager::OnKeysCreated,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 devices.size(),
-                 callback)));
-  create_keys_op_->Start();
-}
-
-void EasyUnlockKeyManager::RemoveKeys(const UserContext& user_context,
-                                      size_t start_index,
-                                      const RemoveKeysCallback& callback) {
-  // Must have the secret.
-  DCHECK(!user_context.GetKey()->GetSecret().empty());
-  // Only one pending request.
-  DCHECK(!HasPendingOperations());
-
-  remove_keys_op_.reset(
-      new EasyUnlockRemoveKeysOperation(
-          user_context,
-          start_index,
-          base::Bind(&EasyUnlockKeyManager::OnKeysRemoved,
-                     weak_ptr_factory_.GetWeakPtr(),
-                     callback)));
-  remove_keys_op_->Start();
+  write_operation_queue_.push_back(
+      make_scoped_ptr(new EasyUnlockRefreshKeysOperation(
+          user_context, devices,
+          base::Bind(&EasyUnlockKeyManager::OnKeysRefreshed,
+                     weak_ptr_factory_.GetWeakPtr(), callback))));
+  RunNextOperation();

[xiyuan, 2014/12/17 00:23:14]

What happens if there is a running operation? This could cause the two
operations to run concurrently. Maybe we should check |operation_in_progress_|
either here or inside RunNextOperation().

[Tim Song, 2014/12/17 02:05:14]

Done. Sorry, I meant to check operation_in_progress_ in RunNextOperation. I had
to shuffle some code around and replace this |operation_in_progress_| variable.

 }
 
 void EasyUnlockKeyManager::GetDeviceDataList(
     const UserContext& user_context,
     const GetDeviceDataListCallback& callback) {
-  // Defer the get operation if there is pending write operations.
-  if (create_keys_op_ || remove_keys_op_) {
-    pending_ops_.push_back(base::Bind(&EasyUnlockKeyManager::GetDeviceDataList,
-                                      weak_ptr_factory_.GetWeakPtr(),
-                                      user_context,
-                                      callback));
-    return;
-  }
-
-  const int op_id = GetNextOperationId();
-  scoped_ptr<EasyUnlockGetKeysOperation> op(new EasyUnlockGetKeysOperation(
-      user_context,
-      base::Bind(&EasyUnlockKeyManager::OnKeysFetched,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 op_id,
-                 callback)));
-  op->Start();
-  get_keys_ops_[op_id] = op.release();
+  read_operation_queue_.push_back(
+      make_scoped_ptr(new EasyUnlockGetKeysOperation(
+          user_context, base::Bind(&EasyUnlockKeyManager::OnKeysFetched,
+                                   weak_ptr_factory_.GetWeakPtr(), callback))));
+  RunNextOperation();

[xiyuan, 2014/12/17 00:23:14]

same here.

[Tim Song, 2014/12/17 02:05:14]

Done.

 }
 
 // static
 void EasyUnlockKeyManager::DeviceDataToRemoteDeviceDictionary(
     const std::string& user_id,
     const EasyUnlockDeviceKeyData& data,
     base::DictionaryValue* dict) {
   dict->SetString(kKeyBluetoothAddress, data.bluetooth_address);
   dict->SetString(kKeyPsk, data.psk);
   scoped_ptr<base::DictionaryValue> permit_record(new base::DictionaryValue);
@@ skipping 61 matching lines @@
 
   data_list->swap(parsed_devices);
   return true;
 }
 
 // static
 std::string EasyUnlockKeyManager::GetKeyLabel(size_t key_index) {
   return base::StringPrintf("%s%zu", kKeyLabelPrefix, key_index);
 }
 
-bool EasyUnlockKeyManager::HasPendingOperations() const {
-  return create_keys_op_ || remove_keys_op_ || !get_keys_ops_.empty();
+void EasyUnlockKeyManager::RunNextOperation() {
+  // The front operation will be removed upon completion.
+  if (!write_operation_queue_.empty()) {
+    operation_in_progress_ = true;
+    write_operation_queue_.front()->Start();
+  } else if (!read_operation_queue_.empty()) {
+    operation_in_progress_ = true;
+    read_operation_queue_.front()->Start();
+  }
 }
 
-int EasyUnlockKeyManager::GetNextOperationId() {
-  return ++operation_id_;
-}
+void EasyUnlockKeyManager::OnKeysRefreshed(const RefreshKeysCallback& callback,
+                                           bool refresh_success) {
+  if (!callback.is_null())
+    callback.Run(refresh_success);
 
-void EasyUnlockKeyManager::RunNextPendingOp() {
-  if (pending_ops_.empty())
-    return;
-
-  pending_ops_.front().Run();
-  pending_ops_.pop_front();
-}
-
-void EasyUnlockKeyManager::OnKeysCreated(
-    size_t remove_start_index,
-    const RefreshKeysCallback& callback,
-    bool create_success) {
-  scoped_ptr<EasyUnlockCreateKeysOperation> op = create_keys_op_.Pass();
-  if (!callback.is_null())
-    callback.Run(create_success);
-
-  // Remove extra existing keys.
-  RemoveKeys(op->user_context(), remove_start_index, RemoveKeysCallback());
-}
-
-void EasyUnlockKeyManager::OnKeysRemoved(const RemoveKeysCallback& callback,
-                                         bool remove_success) {
-  scoped_ptr<EasyUnlockRemoveKeysOperation> op = remove_keys_op_.Pass();
-  if (!callback.is_null())
-    callback.Run(remove_success);
-
-  if (!HasPendingOperations())
-    RunNextPendingOp();
+  operation_in_progress_ = false;
+  DCHECK(!write_operation_queue_.empty());
+  if (!write_operation_queue_.empty())
+    write_operation_queue_.pop_front();
+  RunNextOperation();
 }
 
 void EasyUnlockKeyManager::OnKeysFetched(
-    int op_id,
     const GetDeviceDataListCallback& callback,
     bool fetch_success,
     const EasyUnlockDeviceKeyDataList& fetched_data) {
-  std::map<int, EasyUnlockGetKeysOperation*>::iterator it =
-      get_keys_ops_.find(op_id);
-  scoped_ptr<EasyUnlockGetKeysOperation> op;
-  if (it != get_keys_ops_.end()) {
-    op.reset(it->second);
-    get_keys_ops_.erase(it);
-  } else {
-    NOTREACHED();
-  }
-
   if (!callback.is_null())
     callback.Run(fetch_success, fetched_data);
 
-  if (!HasPendingOperations())
-    RunNextPendingOp();
+  operation_in_progress_ = false;
+  DCHECK(!read_operation_queue_.empty());
+  if (!read_operation_queue_.empty())
+    read_operation_queue_.pop_front();
+  RunNextOperation();
 }
 
 }  // namespace chromeos