[NTP Snippets] Cache images in a LevelDB

components/ntp_snippets/ntp_snippets_database.h

Index: components/ntp_snippets/ntp_snippets_database.h
diff --git a/components/ntp_snippets/ntp_snippets_database.h b/components/ntp_snippets/ntp_snippets_database.h
index f421b209cacc6da62cce0d088b62561a72a309fe..63c87cb1f4b3b4353c2e0f9279256b64c7eabebd 100644
--- a/components/ntp_snippets/ntp_snippets_database.h
+++ b/components/ntp_snippets/ntp_snippets_database.h
@@ -23,29 +23,47 @@ class FilePath;
 
 namespace ntp_snippets {
 
+class SnippetImageProto;
 class SnippetProto;
 
 class NTPSnippetsDatabase {
  public:
-  using SnippetsLoadedCallback = base::Callback<void(NTPSnippet::PtrVector)>;
+  using SnippetsCallback = base::Callback<void(NTPSnippet::PtrVector)>;
+  using SnippetImageCallback = base::Callback<void(std::string)>;
 
   NTPSnippetsDatabase(
       const base::FilePath& database_dir,
       scoped_refptr<base::SequencedTaskRunner> file_task_runner);
   ~NTPSnippetsDatabase();
 
+  // Returns whether the database has finished initialization. While this is
+  // false, loads may already be started (they'll be serviced after
+  // initialization finishes), but any updates are ignored.
+  bool IsInitialized() const;
+
   // Loads all snippets from storage and passes them to |callback|.
-  void Load(const SnippetsLoadedCallback& callback);
+  void LoadSnippets(const SnippetsCallback& callback);
 
   // Adds or updates the given snippet.
-  void Save(const NTPSnippet& snippet);
+  void SaveSnippet(const NTPSnippet& snippet);
   // Adds or updates all the given snippets.
-  void Save(const NTPSnippet::PtrVector& snippets);
+  void SaveSnippets(const NTPSnippet::PtrVector& snippets);
+
+  // Deletes the snippet with the given ID, and its image.
+  void DeleteSnippet(const std::string& snippet_id);
+  // Deletes all the given snippets (identified by their IDs) and their images.
+  void DeleteSnippets(const NTPSnippet::PtrVector& snippets);
+
+  // Loads the image data for the snippet with the given ID and passes it to
+  // |callback|. Passes an empty string if not found.
+  void LoadImage(const std::string& snippet_id,
+                 const SnippetImageCallback& callback);
 
-  // Deletes the snippet with the given ID.
-  void Delete(const std::string& snippet_id);
-  // Deletes all the given snippets (identified by their IDs).
-  void Delete(const NTPSnippet::PtrVector& snippets);
+  // Adds or updates the image data for the given snippet ID.
+  void SaveImage(const std::string& snippet_id, const std::string& image_data);
+
+  // Deletes the image data for the given snippet ID.
+  void DeleteImage(const std::string& snippet_id);
 
  private:
   friend class NTPSnippetsDatabaseTest;
@@ -53,23 +71,44 @@ class NTPSnippetsDatabase {
   using KeyEntryVector =
       leveldb_proto::ProtoDatabase<SnippetProto>::KeyEntryVector;
 
-  // Callbacks for ProtoDatabase operations.
+  using ImageKeyEntryVector =
+      leveldb_proto::ProtoDatabase<SnippetImageProto>::KeyEntryVector;
+
+  // Callbacks for ProtoDatabase<SnippetProto> operations.
   void OnDatabaseInited(bool success);
-  void OnDatabaseLoaded(const SnippetsLoadedCallback& callback,
+  void OnDatabaseLoaded(const SnippetsCallback& callback,
                         bool success,
                         std::unique_ptr<std::vector<SnippetProto>> entries);
   void OnDatabaseSaved(bool success);
 
-  void LoadImpl(const SnippetsLoadedCallback& callback);
+  // Callbacks for ProtoDatabase<SnippetImageProto> operations.
+  void OnImageDatabaseInited(bool success);
+  void OnImageDatabaseLoaded(const SnippetImageCallback& callback,
+                             bool success,
+                             std::unique_ptr<SnippetImageProto> entry);
+  void OnImageDatabaseSaved(bool success);
+
+  void ProcessPendingLoads();
 
-  void SaveImpl(std::unique_ptr<KeyEntryVector> entries_to_save);
+  void LoadSnippetsImpl(const SnippetsCallback& callback);
+  void SaveSnippetsImpl(std::unique_ptr<KeyEntryVector> entries_to_save);
+  void DeleteSnippetsImpl(
+      std::unique_ptr<std::vector<std::string>> keys_to_remove);
 
-  void DeleteImpl(std::unique_ptr<std::vector<std::string>> keys_to_remove);
+  void LoadImageImpl(const std::string& snippet_id,
+                     const SnippetImageCallback& callback);
+  void DeleteImagesImpl(
+      std::unique_ptr<std::vector<std::string>> keys_to_remove);
 
   std::unique_ptr<leveldb_proto::ProtoDatabase<SnippetProto>> database_;
   bool database_initialized_;
+  std::vector<SnippetsCallback> pending_snippets_callbacks_;
 
-  std::vector<SnippetsLoadedCallback> pending_load_callbacks_;
+  std::unique_ptr<leveldb_proto::ProtoDatabase<SnippetImageProto>>
+      image_database_;

[Bernhard Bauer, 2016/06/15 16:11:43]

Do we actually need to have these in the same class? It seems to me the snippets
DB and the image DB are fairly independent.

[Marc Treib, 2016/06/16 08:00:16]

Well, "need" is such a strong word ;)
I do think it's a bit easier/simpler overall this way. The client only has one
storage "thing" where it puts everything. And it avoids a bunch of possible race
conditions: We can be sure they both run on the same thread; we can wait until
both backend DBs are initialized before calling the whole thing initialized -
otherwise it could happen that e.g. we try to delete a snippet (plus its image)
before the image DB was initialized, so we'd have to queue up "save"s and
"delete"s in addition to "load"s; that kind of thing.

+  bool image_database_initialized_;
+  std::vector<std::pair<std::string, SnippetImageCallback>>
+      pending_image_callbacks_;
 
   base::WeakPtrFactory<NTPSnippetsDatabase> weak_ptr_factory_;