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

components/payments/content/payment_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/content/payment_manifest_section_table.h"
+
+#include "base/logging.h"
+#include "sql/statement.h"
+
+namespace payments {
+namespace {

[please use gerrit instead, 2017/04/05 20:33:30]

Newline after "namespace {".

[gogerald1, 2017/04/05 23:38:48]

Done.

+// Note that the finger print is calculated with SHA-256.
+const uint32_t kFingerPrintLength = 32;

[please use gerrit instead, 2017/04/05 20:33:30]

size_t

[gogerald1, 2017/04/05 23:38:48]

Done.

+
+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);

[please use gerrit instead, 2017/04/05 20:33:31]

Shouldn't you cast to WebDatabaseTable::TypeKey?

[gogerald1, 2017/04/05 23:38:48]

WebDatabaseTable::TypeKey is a void pointer in essential, might be easier to
understand why we could do that to use avoid here.

+}
+}

[please use gerrit instead, 2017/04/05 20:33:31]

Newline before "}" and add a comment "  // namespace".

[gogerald1, 2017/04/05 23:38:48]

Done.

+
+PaymentManifestSectionTable::PaymentManifestSectionTable() {}
+
+PaymentManifestSectionTable::~PaymentManifestSectionTable() {}
+
+PaymentManifestSectionTable* PaymentManifestSectionTable::FromWebDatabase(
+    WebDatabase* db) {
+  return static_cast<PaymentManifestSectionTable*>(db->GetTable(GetKey()));
+}
+
+WebDatabaseTable::TypeKey PaymentManifestSectionTable::GetTypeKey() const {
+  return GetKey();
+}
+
+bool PaymentManifestSectionTable::CreateTablesIfNecessary() {
+  if (!db_->DoesTableExist("payment_manifest_section")) {
+    if (!db_->Execute("CREATE TABLE payment_manifest_section ( "
+                      "method_name VARCHAR, "
+                      "package_name VARCHAR, "
+                      "version INTEGER NOT NULL DEFAULT 0, "
+                      "finger_prints BLOB) ")) {
+      NOTREACHED();
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool PaymentManifestSectionTable::IsSyncable() {
+  return false;
+}
+
+bool PaymentManifestSectionTable::MigrateToVersion(
+    int version,
+    bool* update_compatible_version) {
+  return true;
+}
+
+bool PaymentManifestSectionTable::AddPaymentManifestSections(
+    std::string& method_name,
+    std::vector<mojom::PaymentManifestSectionPtr>& manifest) {
+  sql::Statement s1(db_->GetUniqueStatement(

[please use gerrit instead, 2017/04/05 20:33:30]

Use a transaction to group these statements together. Otherwise a failure in the
middle of the function will leave the table in an inconsistent state.

[gogerald1, 2017/04/05 23:38:48]

Done.

+      "DELETE FROM payment_manifest_section WHERE method_name=?"));
+  s1.BindString(0, method_name);
+
+  if (!s1.Run()) {

[please use gerrit instead, 2017/04/05 20:33:31]

No need for {} on a single-line body of an if-statement.

[gogerald1, 2017/04/05 23:38:48]

It's this true? long time ago (> 1 year) when I am doing C++ in Chrome :-),
bracket is always required.

+    return false;
+  }
+
+  sql::Statement s2(db_->GetUniqueStatement(
+      "INSERT INTO payment_manifest_section "
+      "(method_name, package_name, version, finger_prints) "
+      "VALUES (?, ?, ?, ?)"));
+  for (size_t i = 0; i < manifest.size(); i++) {
+    int index = 0;
+    s2.BindString(index++, method_name);
+    s2.BindString(index++, manifest[i]->package_name);
+    s2.BindInt64(index++, manifest[i]->version);
+
+    std::unique_ptr<std::vector<uint8_t>> serialized_finger_prints =
+        SerializeFingerPrints(manifest[i]->sha256_cert_fingerprints);
+    s2.BindBlob(index, serialized_finger_prints->data(),
+                serialized_finger_prints->size());
+
+    s2.Run();

[please use gerrit instead, 2017/04/05 20:33:30]

Check for error.

[gogerald1, 2017/04/05 23:38:48]

Done.

+    s2.Reset(true);
+  }
+
+  return true;
+}
+
+std::vector<mojom::PaymentManifestSectionPtr>
+PaymentManifestSectionTable::GetPaymentManifestSections(
+    std::string& method_name) {
+  std::vector<mojom::PaymentManifestSectionPtr> manifest;
+  sql::Statement s(
+      db_->GetUniqueStatement("SELECT package_name, version, finger_prints "
+                              "FROM payment_manifest_section "
+                              "WHERE method_name=?"));
+  s.BindString(0, method_name);
+
+  if (!s.is_valid()) {

[please use gerrit instead, 2017/04/05 20:33:30]

No {}.

[gogerald1, 2017/04/05 23:38:48]

Done.

[please use gerrit instead, 2017/04/06 14:35:28]

Not yet.

[gogerald1, 2017/04/06 16:42:09]

Done.

+    return manifest;
+  }
+
+  while (s.Step()) {
+    mojom::PaymentManifestSectionPtr section =
+        mojom::PaymentManifestSection::New();
+
+    int index = 0;
+    section->package_name = s.ColumnString(index++);
+    section->version = s.ColumnInt64(index++);
+
+    std::vector<uint8_t> finger_prints;
+    if (!s.ColumnBlobAsVector(index, &finger_prints)) {
+      NOTREACHED();
+      continue;
+    }
+    DeserializeFingerPrints(finger_prints, section->sha256_cert_fingerprints);
+
+    manifest.push_back(std::move(section));
+  }
+
+  return manifest;
+}
+
+std::unique_ptr<std::vector<uint8_t>>
+PaymentManifestSectionTable::SerializeFingerPrints(
+    const std::vector<std::vector<uint8_t>>& finger_prints) {
+  std::unique_ptr<std::vector<uint8_t>> serialized_finger_prints(
+      new std::vector<uint8_t>());

[please use gerrit instead, 2017/04/05 20:33:30]

base::MakeUnique.

[gogerald1, 2017/04/05 23:38:49]

Done.

+
+  if (finger_prints.empty()) {
+    return serialized_finger_prints;
+  }

[please use gerrit instead, 2017/04/05 20:33:30]

This if statement is not useful, because the for loop below does not do anything
when |finger_prints| is empty.

[gogerald1, 2017/04/05 23:38:48]

Done.

+
+  for (auto finger_print : finger_prints) {

[please use gerrit instead, 2017/04/05 20:33:30]

const auto&

[gogerald1, 2017/04/05 23:38:49]

Done.

+    DCHECK(finger_print.size() == kFingerPrintLength);
+    for (auto value : finger_print) {
+      serialized_finger_prints->push_back(value);
+    }

[please use gerrit instead, 2017/04/05 20:33:30]

Replace this for loop with:

serialized_finger_prints->insert(serialized_finger_prints->end(),
finger_print.begin(), finger_print.end());

[gogerald1, 2017/04/05 23:38:48]

Done.

+  }
+
+  return serialized_finger_prints;
+}
+
+void PaymentManifestSectionTable::DeserializeFingerPrints(
+    const std::vector<uint8_t>& finger_prints,
+    std::vector<std::vector<uint8_t>>& deserialized_finger_prints) {
+  DCHECK(finger_prints.size() % kFingerPrintLength == 0);

[please use gerrit instead, 2017/04/05 20:33:30]

Don't trust anything that comes from disk. If this condition is not met, don't
go into the loop below. You may want to have a boolean return value to indicate
to the caller whether deserialization succeeded.

[gogerald1, 2017/04/05 23:38:48]

Done.

+  for (size_t i = 0; i < finger_prints.size();) {
+    std::vector<uint8_t> finger_print;
+    for (uint32_t j = 0; j < kFingerPrintLength; j++) {
+      finger_print.push_back(finger_prints[i + j]);
+    }
+    deserialized_finger_prints.push_back(finger_print);
+    i += kFingerPrintLength;
+  }

[please use gerrit instead, 2017/04/05 20:33:31]

You can do this loop easier:

for (size_t i = 0; i < finger_prints.size(); i += kFingerPrintLength) {
  deserialized_finger_prints.emplace_back(finger_prints.begin() + i,
finger_prints.begin() + i + kFingerPrintLength);
}

[gogerald1, 2017/04/05 23:38:48]

Done.

+}

[please use gerrit instead, 2017/04/05 20:33:30]

Newline below.

[gogerald1, 2017/04/05 23:38:49]

Done.

+}  // payments