NTPSnippetsService: Garbage collect orphaned images at startup.

components/ntp_snippets/remote/ntp_snippets_service.cc

Index: components/ntp_snippets/remote/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/remote/ntp_snippets_service.cc b/components/ntp_snippets/remote/ntp_snippets_service.cc
index 94a8f893a9ca5bc162fc147b7bb2bf45fd5fafa3..a73ce3898f8694c7145e5db4075146530f68f1be 100644
--- a/components/ntp_snippets/remote/ntp_snippets_service.cc
+++ b/components/ntp_snippets/remote/ntp_snippets_service.cc
@@ -10,6 +10,7 @@
 
 #include "base/command_line.h"
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
@@ -492,6 +493,9 @@ void NTPSnippetsService::OnDatabaseLoaded(NTPSnippet::PtrVector snippets) {
   DCHECK_EQ(1u, categories_.size());  // Only articles category, so far.
   DCHECK(categories_.find(articles_category_) != categories_.end());
 
+  ClearExpiredDismissedSnippets();

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

This won't do anything, since |categories_| is still empty at this point.

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

are you sure? we have a DCHECK() above verifying it includes the articles
category...

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

Ah okay, correction: The category itself will be there, but it won't have any
suggestions - those are added just below. So my point "this won't do anything"
still stands :)

If you make a set of IDs anyway, you might as well collect them from
categories_[articles_category_]->snippets and ->dismissed directly in
ClearOrphanedImages instead of passing in |snippets|, and move these two lines
down again. Or am I missing something?

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

nope -- you're not :-)
added a TODO() to increase test coverage as no test was failing.
thanks again for the catch!

+  ClearOrphanedImages(snippets);
+
   // TODO(sfiera): support non-article categories in database.
   CategoryContent* content = &categories_[articles_category_];
   for (std::unique_ptr<NTPSnippet>& snippet : snippets) {
@@ -507,8 +511,6 @@ void NTPSnippetsService::OnDatabaseLoaded(NTPSnippet::PtrVector snippets) {
               return lhs->score() > rhs->score();
             });
 
-  ClearExpiredDismissedSnippets();
-  ClearOrphanedImages();
   FinishInitialization();
 }
 
@@ -593,7 +595,6 @@ void NTPSnippetsService::OnFetchFinished(
         UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticlesFetched",
                                     fetched_category.snippets.size());
       }
-
       ReplaceSnippets(category, std::move(fetched_category.snippets));
     }
   }
@@ -767,8 +768,13 @@ void NTPSnippetsService::ClearExpiredDismissedSnippets() {
   }
 }
 
-void NTPSnippetsService::ClearOrphanedImages() {
-  // TODO(jkrcal): Implement. crbug.com/649009
+void NTPSnippetsService::ClearOrphanedImages(
+    const NTPSnippet::PtrVector& snippets) {
+  auto alive_snippets = base::MakeUnique<std::set<std::string>>();
+  for (const auto& snippet_ptr : snippets) {
+    alive_snippets->insert(snippet_ptr->id());
+  }
+  database_->GarbageCollectImages(std::move(alive_snippets));
 }
 
 void NTPSnippetsService::NukeAllSnippets() {