initial add of SQL based storage

components/offline_pages/offline_page_metadata_store_sql.cc

+// 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.
+
+#include "components/offline_pages/offline_page_metadata_store_sql.h"
+
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/sequenced_task_runner.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/thread_task_runner_handle.h"
+#include "components/offline_pages/offline_page_item.h"
+#include "sql/connection.h"
+#include "sql/error_delegate_util.h"
+#include "sql/meta_table.h"
+#include "sql/statement.h"
+#include "sql/transaction.h"
+
+namespace offline_pages {
+
+namespace {
+
+const int kCurrentVersion = 1;
+const int kCompatibleVersion = 1;
+
+// This is a macro instead of a const so that
+// it can be used inline in other SQL statements below.
+#define OFFLINE_PAGES_TABLE_NAME "offlinepages"
+
+const char kOfflinePagesColumns[] =
+    "(offline_id INTEGER PRIMARY KEY NOT NULL,"
+    " creation_time INTEGER,"
+    " file_size INTEGER,"
+    " version INTEGER,"
+    " last_access_time INTEGER,"
+    " access_count INTEGER,"
+    " status INTEGER,"
+    " user_initiated INTEGER,"  // this is actually a boolean
+    " client_namespace VARCHAR,"
+    " client_id VARCHAR,"
+    " online_url VARCHAR,"
+    " offline_url VARCHAR,"
+    " file_path VARCHAR"
+    ")";
+
+// This is cloned from //content/browser/appcache/appcache_database.cc
+struct TableInfo {
+  const char* table_name;
+  const char* columns;
+};
+
+const TableInfo kOfflinePagesTable{OFFLINE_PAGES_TABLE_NAME,
+                                   kOfflinePagesColumns};
+
+// This enum is used to define the indices for the columns in each row
+// that hold the different pieces of offline page.
+enum : int {
+  OP_OFFLINE_ID = 0,
+  OP_CREATION_TIME,
+  OP_FILE_SIZE,
+  OP_VERSION,
+  OP_LAST_ACCESS_TIME,
+  OP_ACCESS_COUNT,
+  OP_STATUS,
+  OP_USER_INITIATED,
+  OP_CLIENT_NAMESPACE,
+  OP_CLIENT_ID,
+  OP_ONLINE_URL,
+  OP_OFFLINE_URL,
+  OP_FILE_PATH
+};
+
+bool CreateTable(sql::Connection* db, const TableInfo& table_info) {
+  std::string sql("CREATE TABLE ");
+  sql += table_info.table_name;
+  sql += table_info.columns;
+  return db->Execute(sql.c_str());
+}
+
+bool CreateSchema(sql::MetaTable* meta_table, sql::Connection* db) {
+  // 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 false;
+
+  if (!meta_table->Init(db, kCurrentVersion, kCompatibleVersion))
+    return false;
+
+  if (!CreateTable(db, kOfflinePagesTable))
+    return false;
+
+  // TODO(bburns): Add indices here.
+  return transaction.Commit();
+}
+
+bool DeleteByOfflineId(sql::Connection* db, int64_t offline_id) {
+  const char kSql[] =
+      "DELETE FROM " OFFLINE_PAGES_TABLE_NAME " WHERE offline_id=?";
+  sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
+  statement.BindInt64(0, offline_id);
+  return statement.Run();
+}
+
+// Create an offline page item from a SQL result.  Expects complete rows with
+// all columns present.
+OfflinePageItem MakeOfflinePageItem(sql::Statement* statement) {
+  int64_t id = statement->ColumnInt64(OP_OFFLINE_ID);
+  GURL url(statement->ColumnString(OP_ONLINE_URL));
+  ClientId client_id(statement->ColumnString(OP_CLIENT_NAMESPACE),
+                     statement->ColumnString(OP_CLIENT_ID));
+#if defined(OS_POSIX)
+  base::FilePath path(statement->ColumnString(OP_FILE_PATH));
+#elif defined(OS_WIN)
+  base::FilePath path(base::UTF8ToWide(statement->ColumnString(OP_FILE_PATH)));
+#endif
+  int64_t file_size = statement->ColumnInt64(OP_FILE_SIZE);
+  base::Time creation_time =
+      base::Time::FromInternalValue(statement->ColumnInt64(OP_CREATION_TIME));
+
+  OfflinePageItem item(url, id, client_id, path, file_size, creation_time);
+  item.last_access_time = base::Time::FromInternalValue(
+      statement->ColumnInt64(OP_LAST_ACCESS_TIME));
+  item.version = statement->ColumnInt(OP_VERSION);
+  item.access_count = statement->ColumnInt(OP_ACCESS_COUNT);
+  return item;
+}
+
+bool InsertOrReplace(sql::Connection* db, const OfflinePageItem& item) {
+  const char kSql[] =
+      "INSERT OR REPLACE INTO " OFFLINE_PAGES_TABLE_NAME
+      " (offline_id, online_url, client_namespace, client_id, file_path, "
+      "file_size, creation_time, last_access_time, version, access_count)"
+      " VALUES "
+      " (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
+
+  sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
+  statement.BindInt64(0, item.offline_id);
+  statement.BindString(1, item.url.spec());
+  statement.BindString(2, item.client_id.name_space);
+  statement.BindString(3, item.client_id.id);
+#if defined(OS_POSIX)
+  std::string path_string = item.file_path.value();
+#elif defined(OS_WIN)
+  std::string path_string = base::WideToUTF8(item.file_path.value());
+#endif
+  statement.BindString(4, path_string);
+  statement.BindInt64(5, item.file_size);
+  statement.BindInt64(6, item.creation_time.ToInternalValue());
+  statement.BindInt64(7, item.last_access_time.ToInternalValue());
+  statement.BindInt(8, item.version);
+  statement.BindInt(9, item.access_count);
+  return statement.Run();
+}
+
+}  // anonymous namespace
+
+OfflinePageMetadataStoreSQL::OfflinePageMetadataStoreSQL(
+    scoped_refptr<base::SequencedTaskRunner> background_task_runner,
+    const base::FilePath& path)
+    : background_task_runner_(background_task_runner),
+      db_file_path_(path.AppendASCII("OfflinePages.db")),
+      use_in_memory_(false),
+      weak_ptr_factory_(this) {}
+
+OfflinePageMetadataStoreSQL::~OfflinePageMetadataStoreSQL() {
+  if (db_.get() &&
+      !background_task_runner_->DeleteSoon(FROM_HERE, db_.release())) {
+    DLOG(WARNING) << "SQL database will not be deleted.";
+  }
+}
+
+void OfflinePageMetadataStoreSQL::LoadSync(
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    const LoadCallback& callback) {
+  bool opened = false;
+  db_.reset(new sql::Connection);
+  std::unique_ptr<sql::MetaTable> meta_table(new sql::MetaTable());
+
+  if (use_in_memory_) {
+    opened = db_->OpenInMemory();
+  } else {
+    base::File::Error err;
+    if (!base::CreateDirectoryAndGetError(db_file_path_.DirName(), &err)) {
+      LOG(ERROR) << "Failed to create offline pages db directory: "
+                 << base::File::ErrorToString(err);
+    } else {
+      opened = db_->Open(db_file_path_);
+      if (opened)
+        db_->Preload();
+    }
+  }
+  if (!opened) {
+    LOG(ERROR) << "Failed to open database";
+    NotifyLoadResult(runner, callback, STORE_INIT_FAILED,
+                     std::vector<OfflinePageItem>());
+    return;
+  }
+
+  if (!sql::MetaTable::DoesTableExist(db_.get())) {

[Scott Hess - ex-Googler, 2016/04/25 19:57:55]

The design doc suggestions that you'd use table_name_v1, table_name_v2, etc, but
OFFLINE_PAGES_TABLE_NAME has no embedded version info, and you have a meta table
here.  So which way were you using to track it?

[bburns, 2016/04/26 23:27:02]

Ah, this was copy/pasted before I really knew what I was doing.  Removed (and v1
added to the table name, since it was implicit before)

+    if (!CreateSchema(meta_table.get(), db_.get())) {
+      LOG(ERROR) << "Failed to create schema";
+      NotifyLoadResult(runner, callback, STORE_INIT_FAILED,
+                       std::vector<OfflinePageItem>());
+      return;
+    }
+  } else if (!meta_table->Init(db_.get(), kCurrentVersion,
+                               kCompatibleVersion)) {
+    LOG(ERROR) << "Failed to initialize database";
+    NotifyLoadResult(runner, callback, STORE_INIT_FAILED,
+                     std::vector<OfflinePageItem>());
+    return;
+  }
+
+  if (meta_table->GetCompatibleVersionNumber() > kCurrentVersion) {
+    LOG(WARNING) << "Offline database is too new.";
+    NotifyLoadResult(runner, callback, STORE_INIT_FAILED,
+                     std::vector<OfflinePageItem>());
+    return;
+  }
+
+  const char kSql[] = "SELECT * FROM " OFFLINE_PAGES_TABLE_NAME;
+
+  sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
+
+  std::vector<OfflinePageItem> result;
+  while (statement.Step()) {
+    result.push_back(MakeOfflinePageItem(&statement));
+  }
+
+  if (statement.Succeeded()) {
+    NotifyLoadResult(runner, callback, LOAD_SUCCEEDED, result);
+  } else {
+    NotifyLoadResult(runner, callback, STORE_LOAD_FAILED,
+                     std::vector<OfflinePageItem>());
+  }
+}
+
+void OfflinePageMetadataStoreSQL::AddOrUpdateOfflinePageSync(
+    const OfflinePageItem& offline_page,
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    const UpdateCallback& callback) {
+  DCHECK(db_.get());
+  // TODO(bburns): add UMA metrics here (and for levelDB).
+  bool ok = InsertOrReplace(db_.get(), offline_page);
+  runner->PostTask(FROM_HERE, base::Bind(callback, ok));
+}
+
+void OfflinePageMetadataStoreSQL::RemoveOfflinePagesSync(
+    const std::vector<int64_t>& offline_ids,
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    const UpdateCallback& callback) {
+  DCHECK(db_.get());
+  // TODO(bburns): add UMA metrics here (and for levelDB).
+
+  // 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_.get());
+  if (!transaction.Begin()) {
+    runner->PostTask(FROM_HERE, base::Bind(callback, false));
+    return;
+  }
+  for (auto offline_id : offline_ids) {
+    if (!DeleteByOfflineId(db_.get(), offline_id)) {
+      runner->PostTask(FROM_HERE, base::Bind(callback, false));
+      return;
+    }
+  }
+
+  bool success = transaction.Commit();
+  runner->PostTask(FROM_HERE, base::Bind(callback, success));
+}
+
+void OfflinePageMetadataStoreSQL::ResetSync(
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    const ResetCallback& callback) {
+  const char kSql[] = "DELETE * FROM " OFFLINE_PAGES_TABLE_NAME;
+  sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
+  if (!statement.Run()) {
+    runner->PostTask(FROM_HERE, base::Bind(callback, false));
+    return;
+  }
+  db_.reset();
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
+  runner->PostTask(FROM_HERE, base::Bind(callback, true));
+}
+
+void OfflinePageMetadataStoreSQL::NotifyLoadResult(
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    const LoadCallback& callback,
+    LoadStatus status,
+    const std::vector<OfflinePageItem>& result) {
+  // TODO(bburns): Switch to SQL specific UMA metrics.
+  UMA_HISTOGRAM_ENUMERATION("OfflinePages.LoadStatus", status,
+                            OfflinePageMetadataStore::LOAD_STATUS_COUNT);
+  if (status == LOAD_SUCCEEDED) {
+    UMA_HISTOGRAM_COUNTS("OfflinePages.SavedPageCount", result.size());
+  } else {
+    DVLOG(1) << "Offline pages database loading failed: " << status;
+    db_.reset();
+  }
+  runner->PostTask(FROM_HERE, base::Bind(callback, status, result));
+}

[Scott Hess - ex-Googler, 2016/04/25 19:57:55]

AFAICT, the only reason LoadSync() needs the runner and callback are to pass
them to here, and the only reason NotifyLoadResult() exists is to be called by
LoadSync().  I'm wondering if the flow of this would have been easier as
something like:

bool InitDatabase() {
  // Almost everything in LoadSync(), failure returns false;
}

void LoadSync(runner, callback) {
  bool success = InitDatabase();
  // Everything in NotifyLoadResult().
}

Or s/bool/LoadStatus/, of course.

[I see that offline_page_metadata_store_impl does have a more complicated
loading sequence which needs the continuation, but AFAICT there's no strong
reason to retain the complexity here.]

[bburns, 2016/04/26 23:27:02]

Mostly done.  I retained NotifyLoadResult since there are actually three result
codes, and so re-using the code helps clean things up some.

+
+void OfflinePageMetadataStoreSQL::Load(const LoadCallback& callback) {
+  background_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&OfflinePageMetadataStoreSQL::LoadSync,
+                            weak_ptr_factory_.GetWeakPtr(),
+                            base::ThreadTaskRunnerHandle::Get(), callback));
+}
+
+void OfflinePageMetadataStoreSQL::AddOrUpdateOfflinePage(
+    const OfflinePageItem& offline_page,
+    const UpdateCallback& callback) {
+  background_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&OfflinePageMetadataStoreSQL::AddOrUpdateOfflinePageSync,
+                 weak_ptr_factory_.GetWeakPtr(), offline_page,
+                 base::ThreadTaskRunnerHandle::Get(), callback));
+}
+
+void OfflinePageMetadataStoreSQL::RemoveOfflinePages(
+    const std::vector<int64_t>& offline_ids,
+    const UpdateCallback& callback) {
+  if (offline_ids.empty()) {
+    // 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, true));
+    return;
+  }
+
+  background_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&OfflinePageMetadataStoreSQL::RemoveOfflinePagesSync,
+                 weak_ptr_factory_.GetWeakPtr(), offline_ids,
+                 base::ThreadTaskRunnerHandle::Get(), callback));
+}
+
+void OfflinePageMetadataStoreSQL::Reset(const ResetCallback& callback) {
+  background_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&OfflinePageMetadataStoreSQL::ResetSync,
+                            weak_ptr_factory_.GetWeakPtr(),
+                            base::ThreadTaskRunnerHandle::Get(), callback));
+}
+
+void OfflinePageMetadataStoreSQL::SetInMemoryDatabaseForTesting(
+    bool in_memory) {
+  // Must be called prior to Load(...)
+  DCHECK(db_.get() == NULL);
+
+  use_in_memory_ = in_memory;
+}
+
+}  // namespace offline_pages