commit f3a95bf41633f161184a584a4475ba2d663684c1
author Alex Vakulenko <avakulenko@chromium.org> Wed Jul 01 07:52:13 2015 -0700
committer ChromeOS Commit Bot <chromeos-commit-bot@chromium.org> Wed Jul 01 23:23:41 2015 +0000
tree 2e6ef5542e75073dab52cc1128e56c7280dcf167
parent be4254b12f90fb048c9cb2902895f0ba6f1380d7

buffet: Queue device resource updates to prevent parallel requests

As with command and device status updates, device resource updates must
be gated to prevent multiple simultaneous requests sent out the server
at the same time, or else they might overwrite each other's data and be
processed out of order.

BUG=brillo:1230
TEST=`FEATURES=test emerge-link buffet`

Change-Id: I5d74a76d513d64fd6edd4b641ab54c9edef2f536
Reviewed-on: https://chromium-review.googlesource.com/282828
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>

buffet/device_registration_info.cc

diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 2de7e52..66ee054 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -4,6 +4,7 @@
 
 #include "buffet/device_registration_info.h"
 
+#include <algorithm>
 #include <memory>
 #include <set>
 #include <utility>
@@ -794,18 +795,62 @@
 void DeviceRegistrationInfo::UpdateDeviceResource(
     const base::Closure& on_success,
     const CloudRequestErrorCallback& on_failure) {
-  VLOG(1) << "Updating GCD server with CDD...";
-  std::unique_ptr<base::DictionaryValue> device_resource =
-      BuildDeviceResource(nullptr);
-  if (!device_resource)
+  queued_resource_update_callbacks_.emplace_back(on_success, on_failure);
+  if (!in_progress_resource_update_callbacks_.empty()) {
+    VLOG(1) << "Another request is already pending.";
     return;
+  }
+
+  StartQueuedUpdateDeviceResource();
+}
+
+void DeviceRegistrationInfo::StartQueuedUpdateDeviceResource() {
+  CHECK(in_progress_resource_update_callbacks_.empty());
+  if (queued_resource_update_callbacks_.empty())
+    return;
+
+  std::swap(queued_resource_update_callbacks_,
+            in_progress_resource_update_callbacks_);
+
+  VLOG(1) << "Updating GCD server with CDD...";
+  chromeos::ErrorPtr error;
+  std::unique_ptr<base::DictionaryValue> device_resource =
+      BuildDeviceResource(&error);
+  if (!device_resource) {
+    OnUpdateDeviceResourceError(error.get());
+    return;
+  }
 
   DoCloudRequest(
       chromeos::http::request_type::kPut,
       GetDeviceURL(),
       device_resource.get(),
-      base::Bind(&IgnoreCloudResultWithCallback, on_success),
-      on_failure);
+      base::Bind(&DeviceRegistrationInfo::OnUpdateDeviceResourceSuccess,
+                 AsWeakPtr()),
+      base::Bind(&DeviceRegistrationInfo::OnUpdateDeviceResourceError,
+                 AsWeakPtr()));
+}
+
+void DeviceRegistrationInfo::OnUpdateDeviceResourceSuccess(
+    const base::DictionaryValue& reply) {
+  // Make a copy of the callback list so that if the callback triggers another
+  // call to UpdateDeviceResource(), we do not modify the list we are iterating
+  // over.
+  auto callback_list = std::move(in_progress_resource_update_callbacks_);
+  for (const auto& callback_pair : callback_list)
+    callback_pair.first.Run();
+  StartQueuedUpdateDeviceResource();
+}
+
+void DeviceRegistrationInfo::OnUpdateDeviceResourceError(
+    const chromeos::Error* error) {
+  // Make a copy of the callback list so that if the callback triggers another
+  // call to UpdateDeviceResource(), we do not modify the list we are iterating
+  // over.
+  auto callback_list = std::move(in_progress_resource_update_callbacks_);
+  for (const auto& callback_pair : callback_list)
+    callback_pair.second.Run(error);
+  StartQueuedUpdateDeviceResource();
 }
 
 namespace {

buffet/device_registration_info.h

diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index b41b55a..dcf54c0 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -241,6 +241,10 @@
 
   void UpdateDeviceResource(const base::Closure& on_success,
                             const CloudRequestErrorCallback& on_failure);
+  void StartQueuedUpdateDeviceResource();
+  // Success/failure callbacks for UpdateDeviceResource().
+  void OnUpdateDeviceResourceSuccess(const base::DictionaryValue& reply);
+  void OnUpdateDeviceResourceError(const chromeos::Error* error);
 
   void FetchCommands(
       const base::Callback<void(const base::ListValue&)>& on_success,
@@ -314,6 +318,15 @@
   // to the cloud server.
   bool device_state_update_pending_{false};
 
+  using ResourceUpdateCallbackList =
+      std::vector<std::pair<base::Closure, CloudRequestErrorCallback>>;
+  // Success/error callbacks for device resource update request currently in
+  // flight to the cloud server.
+  ResourceUpdateCallbackList in_progress_resource_update_callbacks_;
+  // Success/error callbacks for device resource update requests queued while
+  // another request is in flight to the cloud server.
+  ResourceUpdateCallbackList queued_resource_update_callbacks_;
+
   const bool notifications_enabled_;
   std::unique_ptr<NotificationChannel> primary_notification_channel_;
   std::unique_ptr<PullChannel> pull_channel_;