[NTP Snippets] Persist snippets in a LevelDB instead of prefs

components/ntp_snippets/ntp_snippets_database.cc

Index: components/ntp_snippets/ntp_snippets_database.cc
diff --git a/components/ntp_snippets/ntp_snippets_database.cc b/components/ntp_snippets/ntp_snippets_database.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3608f27ae0621c7cd7dcf118f43ef5594d188804
--- /dev/null
+++ b/components/ntp_snippets/ntp_snippets_database.cc
@@ -0,0 +1,157 @@
+// 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/ntp_snippets/ntp_snippets_database.h"
+
+#include <utility>
+
+#include "base/files/file_path.h"
+#include "components/leveldb_proto/proto_database_impl.h"
+#include "components/ntp_snippets/proto/ntp_snippets.pb.h"
+
+using leveldb_proto::ProtoDatabaseImpl;
+
+namespace {
+// Statistics are logged to UMA with this string as part of histogram name. They
+// can all be found under LevelDB.*.NTPSnippets. Changing this needs to
+// synchronize with histograms.xml, AND will also become incompatible with older
+// browsers still reporting the previous values.
+const char kDatabaseUMAClientName[] = "NTPSnippets";
+}
+
+namespace ntp_snippets {
+
+NTPSnippetsDatabase::NTPSnippetsDatabase(
+    const base::FilePath& database_dir,
+    scoped_refptr<base::SequencedTaskRunner> file_task_runner)
+    : database_(new ProtoDatabaseImpl<SnippetProto>(file_task_runner)),

[Bernhard Bauer, 2016/05/25 15:01:15]

std::move the |file_task_runner|?

[Marc Treib, 2016/05/27 14:03:12]

Done.

+      database_inited_(false),
+      weak_ptr_factory_(this) {
+  database_->Init(kDatabaseUMAClientName, database_dir,
+                  base::Bind(&NTPSnippetsDatabase::OnDatabaseInited,
+                             weak_ptr_factory_.GetWeakPtr()));
+}
+
+NTPSnippetsDatabase::~NTPSnippetsDatabase() {}
+
+void NTPSnippetsDatabase::Load(const SnippetsLoadedCallback& callback) {
+  DCHECK(!callback.is_null());
+  bool callback_was_null = callback_.is_null();

[Bernhard Bauer, 2016/05/25 15:01:15]

Couldn't we DCHECK instead that we only do one callback?

[Marc Treib, 2016/05/27 14:03:12]

Well, it changes the interface - the caller will have to make sure not to call
this again before a previous call finished. But since at least for now, it's
only ever called once, I guess that's fine, so done.

+  callback_ = callback;
+  if (callback_was_null && database_ && database_inited_)
+    LoadImpl();
+}
+
+void NTPSnippetsDatabase::Save(const NTPSnippet& snippet) {
+  std::unique_ptr<KeyEntryVector> entries_to_save(new KeyEntryVector());
+  entries_to_save->emplace_back(snippet.id(), snippet.ToProto());
+  SaveImpl(std::move(entries_to_save));
+}
+
+void NTPSnippetsDatabase::Save(const NTPSnippet::PtrVector& snippets) {
+  std::unique_ptr<KeyEntryVector> entries_to_save(new KeyEntryVector());
+  for (const std::unique_ptr<NTPSnippet>& snippet : snippets)
+    entries_to_save->emplace_back(snippet->id(), snippet->ToProto());
+  SaveImpl(std::move(entries_to_save));
+}
+
+void NTPSnippetsDatabase::Delete(const std::string& snippet_id) {
+  DeleteImpl(base::WrapUnique(new std::vector<std::string>(1, snippet_id)));
+}
+
+void NTPSnippetsDatabase::Delete(const NTPSnippet::PtrVector& snippets) {
+  std::unique_ptr<std::vector<std::string>> keys_to_remove(
+      new std::vector<std::string>());
+  for (const std::unique_ptr<NTPSnippet>& snippet : snippets)
+    keys_to_remove->emplace_back(snippet->id());
+  DeleteImpl(std::move(keys_to_remove));
+}
+
+void NTPSnippetsDatabase::OnDatabaseInited(bool success) {
+  DCHECK(!database_inited_);
+  if (!success) {
+    DVLOG(1) << "NTPSnippetsDatabase init failed.";
+    database_.reset();
+    return;
+  }
+  database_inited_ = true;
+  if (!callback_.is_null())
+    LoadImpl();
+}
+
+void NTPSnippetsDatabase::OnDatabaseLoaded(
+    bool success,
+    std::unique_ptr<std::vector<SnippetProto>> entries) {
+  if (!success) {
+    DVLOG(1) << "NTPSnippetsDatabase load failed.";
+    database_.reset();
+    return;
+  }
+
+  std::unique_ptr<std::vector<std::string>> keys_to_remove(
+      new std::vector<std::string>());
+
+  NTPSnippet::PtrVector snippets;
+  for (const SnippetProto& proto : *entries) {
+    std::unique_ptr<NTPSnippet> snippet = NTPSnippet::CreateFromProto(proto);
+    if (snippet) {
+      snippets.emplace_back(std::move(snippet));
+    } else {
+      LOG(WARNING) << "Invalid proto from DB " << proto.id();
+      keys_to_remove->emplace_back(proto.id());
+    }
+  }
+
+  // We only start loading if we have a non-null callback, but it's possible
+  // that it's been cleared in the meantime.
+  DCHECK(!callback_.is_null());
+  callback_.Run(std::move(snippets));
+  callback_.Reset();
+
+  // If any of the snippet protos couldn't be converted to actual snippets,
+  // clean them up now.
+  if (!keys_to_remove->empty())
+    DeleteImpl(std::move(keys_to_remove));
+}
+
+void NTPSnippetsDatabase::OnDatabaseSaved(bool success) {
+  if (!success) {
+    DVLOG(1) << "NTPSnippetsDatabase save failed.";
+    database_.reset();
+  }
+}
+
+void NTPSnippetsDatabase::LoadImpl() {
+  DCHECK(database_);
+  DCHECK(database_inited_);
+  database_->LoadEntries(base::Bind(&NTPSnippetsDatabase::OnDatabaseLoaded,
+                                    weak_ptr_factory_.GetWeakPtr()));
+}
+
+void NTPSnippetsDatabase::SaveImpl(
+    std::unique_ptr<KeyEntryVector> entries_to_save) {
+  if (!database_ || !database_inited_)
+    return;
+
+  std::unique_ptr<std::vector<std::string>> keys_to_remove(
+      new std::vector<std::string>());
+  database_->UpdateEntries(std::move(entries_to_save),

[Bernhard Bauer, 2016/05/25 15:01:15]

It sucks a bit that ProtoDatabase takes unique_ptr's everywhere (I assume it
predates std::move use in Chrome), but that doesn't necessarily mean we have to
do that as well. Could we take the KeyEntryVector directly and move into a
unique_ptr here?

[Marc Treib, 2016/05/27 14:03:12]

Hm, I guess. But then it'd be easy to accidentally copy the vector. Also moving
into the unique_ptr is kinda ugly... I prefer it like this. So, do you insist?
;)

+                           std::move(keys_to_remove),
+                           base::Bind(&NTPSnippetsDatabase::OnDatabaseSaved,
+                                      weak_ptr_factory_.GetWeakPtr()));
+}
+
+void NTPSnippetsDatabase::DeleteImpl(
+    std::unique_ptr<std::vector<std::string>> keys_to_remove) {
+  if (!database_ || !database_inited_)
+    return;
+
+  std::unique_ptr<KeyEntryVector> entries_to_save(new KeyEntryVector());
+  database_->UpdateEntries(std::move(entries_to_save),
+                           std::move(keys_to_remove),
+                           base::Bind(&NTPSnippetsDatabase::OnDatabaseSaved,
+                                      weak_ptr_factory_.GetWeakPtr()));
+}
+
+}  // namespace ntp_snippets