[Payments] Add web app manifest section table in SQLite web database

components/payments/android/web_app_manifest_section_table.cc

+// Copyright 2017 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/payments/android/web_app_manifest_section_table.h"
+
+#include <stdint.h>
+
+#include <memory>
+
+#include "base/logging.h"
+#include "sql/statement.h"
+
+namespace payments {
+namespace {
+
+// Note that the fingerprint is calculated with SHA-256.
+const size_t kFingerPrintLength = 32;
+
+WebDatabaseTable::TypeKey GetKey() {
+  // We just need a unique constant. Use the address of a static that
+  // COMDAT folding won't touch in an optimizing linker.
+  static int table_key = 0;
+  return reinterpret_cast<void*>(&table_key);
+}
+
+// Converts 2-dimensional vector |fingerprints| to 1-dimesional vector.
+std::unique_ptr<std::vector<uint8_t>> SerializeFingerPrints(
+    const std::vector<std::vector<uint8_t>>& fingerprints) {
+  auto serialized_fingerprints = base::MakeUnique<std::vector<uint8_t>>();
+
+  for (const auto& fingerprint : fingerprints) {
+    DCHECK_EQ(fingerprint.size(), kFingerPrintLength);
+    serialized_fingerprints->insert(serialized_fingerprints->end(),
+                                    fingerprint.begin(), fingerprint.end());
+  }
+
+  return serialized_fingerprints;
+}
+
+// Converts 1-dimensional vector created by SerializeFingerPrints back to
+// 2-dimensional vector. Each vector of the second dimensional vector has exact
+// kFingerPrintLength number of elements.
+bool DeserializeFingerPrints(
+    const std::vector<uint8_t>& fingerprints,
+    std::vector<std::vector<uint8_t>>& deserialized_fingerprints) {
+  if (fingerprints.size() % kFingerPrintLength != 0)
+    return false;
+
+  for (size_t i = 0; i < fingerprints.size(); i += kFingerPrintLength) {
+    deserialized_fingerprints.emplace_back(
+        fingerprints.begin() + i,
+        fingerprints.begin() + i + kFingerPrintLength);
+  }
+  return true;
+}
+
+}  // namespace
+
+WebAppManifestSectionTable::WebAppManifestSectionTable() {}
+
+WebAppManifestSectionTable::~WebAppManifestSectionTable() {}
+
+WebAppManifestSectionTable* WebAppManifestSectionTable::FromWebDatabase(
+    WebDatabase* db) {
+  return static_cast<WebAppManifestSectionTable*>(db->GetTable(GetKey()));
+}
+
+WebDatabaseTable::TypeKey WebAppManifestSectionTable::GetTypeKey() const {
+  return GetKey();
+}
+
+bool WebAppManifestSectionTable::CreateTablesIfNecessary() {
+  if (!db_->DoesTableExist("web_app_manifest_section")) {
+    if (!db_->Execute("CREATE TABLE web_app_manifest_section ( "

[Scott Hess - ex-Googler, 2017/04/11 17:24:06]

How about "CREATE TABLE IF NOT EXISTS web_app_manifest_section..."?

[gogerald1, 2017/04/11 19:01:04]

Done.

+                      "id VARCHAR, "
+                      "min_version INTEGER NOT NULL DEFAULT 0, "
+                      "fingerprints BLOB) ")) {
+      NOTREACHED();
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool WebAppManifestSectionTable::IsSyncable() {
+  return false;
+}
+
+bool WebAppManifestSectionTable::MigrateToVersion(
+    int version,
+    bool* update_compatible_version) {
+  return true;
+}
+
+bool WebAppManifestSectionTable::AddWebAppManifest(
+    mojom::WebAppManifestSection* manifest) {
+  if (manifest == nullptr || manifest->id.empty())

[Scott Hess - ex-Googler, 2017/04/11 17:24:06]

Is this something which should never happen because it's not appropriate use of
the API?  In that case a DCHECK seems better.

[gogerald1, 2017/04/11 19:01:04]

Done. manifest should never be nullptr, DCHECKed it.

[Scott Hess - ex-Googler, 2017/04/11 19:31:45]

I'm wondering about the empty() check, now.  All of the later false returns are
from cases where something went wrong with the database, but this one is where
the caller called with in incorrect value.  AddWebAppManifest() sounds like
something you shouldn't even call if you don't have an id, and the caller maybe
made a mistake.

It is good to keep the invalid id from getting to the database, though, because
it will be persisted even if the caller makes a mistake.  So IMHO NOTREACHED()
is reasonable.

[gogerald1, 2017/04/11 21:53:30]

Done.

+    return false;
+
+  if (!db_->BeginTransaction())

[Scott Hess - ex-Googler, 2017/04/11 17:24:06]

Manual transaction handling is brittle, use sql::Transaction unless you have a
solid reason to do otherwise.

[gogerald1, 2017/04/11 19:01:04]

Done.

+    return false;
+
+  sql::Statement s1(db_->GetUniqueStatement(
+      "DELETE FROM web_app_manifest_section WHERE id=?"));
+  s1.BindString(0, manifest->id);
+  if (!s1.Run()) {
+    db_->RollbackTransaction();
+    return false;
+  }
+
+  sql::Statement s2(
+      db_->GetUniqueStatement("INSERT INTO web_app_manifest_section "
+                              "(id, min_version, fingerprints) "
+                              "VALUES (?, ?, ?)"));
+  int index = 0;
+  s2.BindString(index++, manifest->id);
+  s2.BindInt64(index++, manifest->min_version);
+  std::unique_ptr<std::vector<uint8_t>> serialized_fingerprints =
+      SerializeFingerPrints(manifest->fingerprints);
+  s2.BindBlob(index, serialized_fingerprints->data(),
+              serialized_fingerprints->size());
+  if (!s2.Run()) {
+    db_->RollbackTransaction();
+    return false;
+  }
+
+  if (!db_->CommitTransaction()) {
+    db_->RollbackTransaction();
+    return false;
+  }
+
+  return true;
+}
+
+mojom::WebAppManifestSectionPtr WebAppManifestSectionTable::GetWebAppManifest(
+    std::string web_app) {
+  mojom::WebAppManifestSectionPtr manifest;
+  sql::Statement s(
+      db_->GetUniqueStatement("SELECT id, min_version, fingerprints "
+                              "FROM web_app_manifest_section "
+                              "WHERE id=?"
+                              "LIMIT 1"));
+  s.BindString(0, web_app);
+
+  if (!s.is_valid())
+    return manifest;

[Scott Hess - ex-Googler, 2017/04/11 17:24:06]

|manifest| is not apparently initialized at this point.  If you mean nullptr,
use nullptr.

[gogerald1, 2017/04/11 19:01:04]

Done.

+
+  if (s.Step()) {
+    manifest = mojom::WebAppManifestSection::New();
+
+    int index = 0;
+    manifest->id = s.ColumnString(index++);
+    manifest->min_version = s.ColumnInt64(index++);
+
+    std::vector<uint8_t> fingerprints;
+    if (!s.ColumnBlobAsVector(index, &fingerprints)) {
+      manifest = nullptr;
+      return manifest;

[Scott Hess - ex-Googler, 2017/04/11 17:24:06]

Just return nullptr.  I assume |manifest| is scoped to delete itself.

[gogerald1, 2017/04/11 19:01:04]

Done.

+    }
+
+    if (!DeserializeFingerPrints(fingerprints, manifest->fingerprints)) {
+      manifest = nullptr;
+      return manifest;

[Scott Hess - ex-Googler, 2017/04/11 17:24:06]

Also here.

[gogerald1, 2017/04/11 19:01:04]

Done.

+    }
+  }
+
+  return manifest;
+}
+
+}  // payments