NTPSnippetsService: Garbage collect orphaned images at startup.

components/ntp_snippets/remote/ntp_snippets_database.h

 // 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.
 
 #ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_DATABASE_H_
 #define COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_DATABASE_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "components/leveldb_proto/proto_database.h"
@@ skipping 48 matching lines @@
   void LoadImage(const std::string& snippet_id,
                  const SnippetImageCallback& callback);
 
   // 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);
   // Deletes the image data for the given snippets (identified by their IDs).
   void DeleteImages(const NTPSnippet::PtrVector& snippets);
+  // Deletes all images which are not associated with any of the provided
+  // snippets.
+  void GarbageCollectImages(
+      std::unique_ptr<std::set<std::string>> alive_snippets);

[Marc Treib, 2016/10/06 10:21:30]

Any particular reason for using std::set here? I'd pass in a const
NTPSnippet::PtrVector&, to be consistent with the other methods (and build the
set inside).

[tschumann, 2016/10/06 10:56:10]

hmmm... it feels a bit weird that the function call wants a whole snippet if the
snippet-id is sufficient. I'd  actually vote for changing DeleteImages() to take
a vector of ids as well.
if we want to go that route (i'm happy to write the patch), then passing the
data in the data structure we actually need seems more plausible.

If you feel strongly, I can change that of course ;-)

[Marc Treib, 2016/10/06 11:37:20]

I think the main reason for doing it that way was laziness :)  The caller had a
vector of snippets anyway, so this saved some copying.
Not feeling too strongly either way, I just like consistency. If you promise to
do a followup that makes things consistent (either way), then I'm fine with this
for now.

[tschumann, 2016/10/06 12:21:44]

These methods were not called in many places. Just updated within this CL. PTAL.

 
  private:
   friend class NTPSnippetsDatabaseTest;
 
   using KeyEntryVector =
       leveldb_proto::ProtoDatabase<SnippetProto>::KeyEntryVector;
 
   using ImageKeyEntryVector =
       leveldb_proto::ProtoDatabase<SnippetImageProto>::KeyEntryVector;
 
@@ skipping 17 matching lines @@
 
   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 LoadImageImpl(const std::string& snippet_id,
                      const SnippetImageCallback& callback);
   void DeleteImagesImpl(
       std::unique_ptr<std::vector<std::string>> keys_to_remove);
+  void DeleteUnreferencedImages(
+      std::unique_ptr<std::set<std::string>> references,
+      bool success,

[Marc Treib, 2016/10/06 10:21:30]

nit: load_keys_success? Took me a while to understand what that bool was doing
in the middle there.

[tschumann, 2016/10/06 10:56:10]

Done.

+      std::unique_ptr<std::vector<std::string>> keys);
 
   std::unique_ptr<leveldb_proto::ProtoDatabase<SnippetProto>> database_;
   bool database_initialized_;
   std::vector<SnippetsCallback> pending_snippets_callbacks_;
 
   std::unique_ptr<leveldb_proto::ProtoDatabase<SnippetImageProto>>
       image_database_;
   bool image_database_initialized_;
   std::vector<std::pair<std::string, SnippetImageCallback>>
       pending_image_callbacks_;
 
   base::Closure error_callback_;
 
   base::WeakPtrFactory<NTPSnippetsDatabase> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NTPSnippetsDatabase);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_DATABASE_H_