Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(429)

Unified Diff: components/offline_pages/background/request_queue_store_sql.cc

Issue 2221323003: Add an API to Pause and Resume background offlining requests. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: CR feedback per Dimich Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/offline_pages/background/request_queue_store_sql.cc
diff --git a/components/offline_pages/background/request_queue_store_sql.cc b/components/offline_pages/background/request_queue_store_sql.cc
index 264b2acb42734ee9db93c9ceefd8bde8a97f26b6..f45139fb5487b295d1ad1e36746d9ce5c91acec0 100644
--- a/components/offline_pages/background/request_queue_store_sql.cc
+++ b/components/offline_pages/background/request_queue_store_sql.cc
@@ -77,6 +77,18 @@ bool DeleteRequestByClientId(sql::Connection* db, const ClientId& client_id) {
return statement.Run();
}
+bool ChangeRequestState(sql::Connection* db,
+ const int64_t request_id,
+ const SavePageRequest::RequestState new_state) {
+ const char kSql[] = "UPDATE " REQUEST_QUEUE_TABLE_NAME
+ " SET state=?"
+ " WHERE request_id=?";
+ sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
+ statement.BindInt64(0, static_cast<long>(new_state));
fgorski 2016/08/10 16:00:09 please update the cast to type to int64_t. This wi
Pete Williamson 2016/08/10 21:38:01 Done.
+ statement.BindInt64(1, request_id);
+ return statement.Run();
+}
+
bool DeleteRequestsByIds(sql::Connection* db,
const std::vector<int64_t>& request_ids,
int* count) {
@@ -123,6 +135,26 @@ bool DeleteRequestsByClientIds(sql::Connection* db,
return true;
}
+RequestQueueStore::UpdateStatus ChangeRequestsState(
+ sql::Connection* db,
+ const std::vector<int64_t>& request_ids,
+ SavePageRequest::RequestState new_state) {
+ // If you create a transaction but don't Commit() it is automatically
+ // rolled back by its destructor when it falls out of scope.
+ sql::Transaction transaction(db);
+ if (!transaction.Begin())
+ return RequestQueueStore::UpdateStatus::FAILED;
+ for (const auto& request_id : request_ids) {
+ if (!ChangeRequestState(db, request_id, new_state))
+ return RequestQueueStore::UpdateStatus::FAILED;
+ }
+
+ if (!transaction.Commit())
+ return RequestQueueStore::UpdateStatus::FAILED;
+
+ return RequestQueueStore::UpdateStatus::UPDATED;
+}
+
// Create a save page request from a SQL result. Expects complete rows with
// all columns present. Columns are in order they are defined in select query
// in |RequestQueueStore::RequestSync| method.
@@ -290,6 +322,19 @@ void RequestQueueStoreSQL::RemoveRequestsByClientIdSync(
}
// static
+void RequestQueueStoreSQL::ChangeRequestsStateSync(
+ sql::Connection* db,
+ scoped_refptr<base::SingleThreadTaskRunner> runner,
+ const std::vector<int64_t>& request_ids,
+ const SavePageRequest::RequestState new_state,
+ const UpdateCallback& callback) {
+ // TODO(fgorski): add UMA metrics here.
+ RequestQueueStore::UpdateStatus status =
+ offline_pages::ChangeRequestsState(db, request_ids, new_state);
+ runner->PostTask(FROM_HERE, base::Bind(callback, status));
+}
+
+// static
void RequestQueueStoreSQL::ResetSync(
sql::Connection* db,
const base::FilePath& db_file_path,
@@ -303,15 +348,22 @@ void RequestQueueStoreSQL::ResetSync(
runner->PostTask(FROM_HERE, base::Bind(callback, success));
}
-void RequestQueueStoreSQL::GetRequests(const GetRequestsCallback& callback) {
+bool RequestQueueStoreSQL::CheckDb(const base::Closure& callback) {
DCHECK(db_.get());
if (!db_.get()) {
// Nothing to do, but post a callback instead of calling directly
// to preserve the async style behavior to prevent bugs.
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(callback, false, std::vector<SavePageRequest>()));
- return;
+ base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+ base::Bind(callback));
+ return false;
}
+ return true;
+}
+
+void RequestQueueStoreSQL::GetRequests(const GetRequestsCallback& callback) {
+ DCHECK(db_.get());
+ if (!CheckDb(base::Bind(callback, false, std::vector<SavePageRequest>())))
+ return;
background_task_runner_->PostTask(
FROM_HERE, base::Bind(&RequestQueueStoreSQL::GetRequestsSync, db_.get(),
@@ -321,13 +373,8 @@ void RequestQueueStoreSQL::GetRequests(const GetRequestsCallback& callback) {
void RequestQueueStoreSQL::AddOrUpdateRequest(const SavePageRequest& request,
const UpdateCallback& callback) {
DCHECK(db_.get());
- if (!db_.get()) {
- // Nothing to do, but post a callback instead of calling directly
- // to preserve the async style behavior to prevent bugs.
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(callback, UpdateStatus::FAILED));
+ if (!CheckDb(base::Bind(callback, UpdateStatus::FAILED)))
return;
- }
background_task_runner_->PostTask(
FROM_HERE,
@@ -335,21 +382,11 @@ void RequestQueueStoreSQL::AddOrUpdateRequest(const SavePageRequest& request,
base::ThreadTaskRunnerHandle::Get(), request, callback));
}
-// TODO(petewil): This is unused, since request_coordinator doesn't keep
-// request_ids, and neither do clients. Plan is to remove this API in a future
-// changelist. If we do discover a need to keep it, name it
-// RemoveRequestsByRequestId to be more parallell with RemoveRequestsByClientId.
void RequestQueueStoreSQL::RemoveRequests(
const std::vector<int64_t>& request_ids,
const RemoveCallback& callback) {
- DCHECK(db_.get());
- if (!db_.get()) {
- // Nothing to do, but post a callback instead of calling directly
- // to preserve the async style behavior to prevent bugs.
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(callback, false, 0));
+ if (!CheckDb(base::Bind(callback, false, 0)))
return;
- }
background_task_runner_->PostTask(
FROM_HERE,
@@ -375,15 +412,23 @@ void RequestQueueStoreSQL::RemoveRequestsByClientId(
base::ThreadTaskRunnerHandle::Get(), client_ids, callback));
}
+void RequestQueueStoreSQL::ChangeRequestsState(
+ const std::vector<int64_t>& request_ids,
+ const SavePageRequest::RequestState new_state,
+ const UpdateCallback& callback) {
+ if (!CheckDb(base::Bind(callback, UpdateStatus::FAILED)))
+ return;
+
+ background_task_runner_->PostTask(
+ FROM_HERE, base::Bind(&RequestQueueStoreSQL::ChangeRequestsStateSync,
+ db_.get(), base::ThreadTaskRunnerHandle::Get(),
+ request_ids, new_state, callback));
+}
+
void RequestQueueStoreSQL::Reset(const ResetCallback& callback) {
DCHECK(db_.get());
- if (!db_.get()) {
- // Nothing to do, but post a callback instead of calling directly
- // to preserve the async style behavior to prevent bugs.
- base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
- base::Bind(callback, false));
+ if (!CheckDb(base::Bind(callback, false)))
return;
- }
background_task_runner_->PostTask(
FROM_HERE,

Powered by Google App Engine
This is Rietveld 408576698