[Offline pages] Applying TaskQueue to RequestQueue::ChangeRequestsState

components/offline_pages/background/change_requests_state_task.h

Index: components/offline_pages/background/change_requests_state_task.h
diff --git a/components/offline_pages/background/change_requests_state_task.h b/components/offline_pages/background/change_requests_state_task.h
new file mode 100644
index 0000000000000000000000000000000000000000..18d8c0c18122ac72ee5b37fd6358cd300e6e8a6d
--- /dev/null
+++ b/components/offline_pages/background/change_requests_state_task.h
@@ -0,0 +1,62 @@
+// Copyright 2016 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.
+
+#ifndef COMPONENTS_OFFLINE_PAGES_BACKGROUND_CHANGE_REQUESTS_STATE_TASK_H_
+#define COMPONENTS_OFFLINE_PAGES_BACKGROUND_CHANGE_REQUESTS_STATE_TASK_H_
+
+#include <stdint.h>
+
+#include <memory>
+#include <unordered_set>
+#include <vector>
+
+#include "base/memory/weak_ptr.h"
+#include "components/offline_pages/background/request_queue_store.h"
+#include "components/offline_pages/background/save_page_request.h"
+#include "components/offline_pages/core/task.h"
+
+namespace offline_pages {
+
+class ChangeRequestsStateTask : public Task {
+ public:
+  ChangeRequestsStateTask(RequestQueueStore* store,
+                          const std::vector<int64_t>& request_ids,
+                          const SavePageRequest::RequestState new_state,
+                          const RequestQueueStore::UpdateCallback& callback);
+  ~ChangeRequestsStateTask() override;
+
+  // TaskQueue::Task implementation.
+  // Step 1. Reading the requests.

[dougarnett, 2016/09/29 21:02:49]

I wonder about a pattern of separate Step 1 method ReadRequests() and then Run()
just calls Step 1 method. Just a thought.

[fgorski, 2016/09/30 23:14:35]

Done. Actually a very good idea, I also noticed that the individual steps can be
private. Moved all three below private:

+  void Run() override;
+
+  // Step 2. Selecting requests to be updated. Calls update.
+  void SelectItemsToUpdate(
+      bool success,
+      std::vector<std::unique_ptr<SavePageRequest>> requests);
+
+  // Step 3. Processes update result, calls callback.
+  void UpdateCompleted(std::unique_ptr<UpdateRequestsResult> update_result);
+
+ private:
+  void CompleteEarly(ItemActionStatus status);
+  void CompleteWithStatus(std::unique_ptr<UpdateRequestsResult> result,
+                          ItemActionStatus status);
+
+  // Store that this task updates.
+  RequestQueueStore* store_;

[dougarnett, 2016/09/29 21:02:49]

this is kind of interesting - not having the TaskQueue bound to a store. I
hadn't pictured that way initially but seems fine.

[fgorski, 2016/09/30 23:14:35]

Acknowledged. That is actually intended. TaskQueue does not care what it guards,
it just does it.

+  // Request IDs to be updated.
+  std::unordered_set<int64_t> request_ids_;
+  // New state to be set on all entries.
+  SavePageRequest::RequestState new_state_;
+  // Callback to complete the task.
+  RequestQueueStore::UpdateCallback callback_;
+
+  base::WeakPtrFactory<ChangeRequestsStateTask> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChangeRequestsStateTask);
+};
+
+}  // namespace offline_pages
+
+#endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_CHANGE_REQUESTS_STATE_TASK_H_