Various NTPSnippetsService cleanups:

components/ntp_snippets/ntp_snippets_service.cc

 // Copyright 2015 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_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
@@ skipping 108 matching lines @@
   for (int i = 0; i < suggestions.suggestions_size(); ++i) {
     const ChromeSuggestion& suggestion = suggestions.suggestions(i);
     GURL url(suggestion.url());
     if (url.is_valid())
       hosts.insert(url.host());
   }
   return hosts;
 }
 
 std::unique_ptr<base::ListValue> SnippetsToListValue(
-    const NTPSnippetsService::NTPSnippetStorage& snippets) {
+    const NTPSnippet::PtrVector& snippets) {
   std::unique_ptr<base::ListValue> list(new base::ListValue);
   for (const auto& snippet : snippets) {
     std::unique_ptr<base::DictionaryValue> dict = snippet->ToDictionary();
     list->Append(std::move(dict));
   }
   return list;
 }
 
-bool ContainsSnippet(const NTPSnippetsService::NTPSnippetStorage& haystack,
+bool ContainsSnippet(const NTPSnippet::PtrVector& haystack,
                      const std::unique_ptr<NTPSnippet>& needle) {
   const std::string& id = needle->id();
   return std::find_if(haystack.begin(), haystack.end(),
                       [&id](const std::unique_ptr<NTPSnippet>& snippet) {
                         return snippet->id() == id;
                       }) != haystack.end();
 }
 
 void WrapImageFetchedCallback(
     const NTPSnippetsService::ImageFetchedCallback& callback,
     const GURL& snippet_id_url,
     const SkBitmap* bitmap) {
   callback.Run(snippet_id_url.spec(), bitmap);
 }
 
 }  // namespace
 
 NTPSnippetsService::NTPSnippetsService(
     PrefService* pref_service,
     SuggestionsService* suggestions_service,
     scoped_refptr<base::SequencedTaskRunner> file_task_runner,
     const std::string& application_language_code,
     NTPSnippetsScheduler* scheduler,
     std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher,
     std::unique_ptr<ImageFetcher> image_fetcher)
-    : enabled_(false),
+    : state_(State::NOT_INITED),
+      enabled_(false),
       pref_service_(pref_service),
       suggestions_service_(suggestions_service),
       file_task_runner_(file_task_runner),
       application_language_code_(application_language_code),
       scheduler_(scheduler),
       snippets_fetcher_(std::move(snippets_fetcher)),
       image_fetcher_(std::move(image_fetcher)) {
   snippets_fetcher_->SetCallback(base::Bind(
       &NTPSnippetsService::OnFetchFinished, base::Unretained(this)));
 }
 
-NTPSnippetsService::~NTPSnippetsService() {}
+NTPSnippetsService::~NTPSnippetsService() {
+  DCHECK(state_ == State::NOT_INITED || state_ == State::SHUT_DOWN);
+}
 
 // static
 void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(prefs::kSnippets);
   registry->RegisterListPref(prefs::kDiscardedSnippets);
   registry->RegisterListPref(prefs::kSnippetHosts);
 }
 
 void NTPSnippetsService::Init(bool enabled) {
+  DCHECK(state_ == State::NOT_INITED);

[Marc Treib, 2016/05/17 11:21:18]

DCHECK_EQ etc don't work with enum class :(
(I guess we could cast everything to int, but at that point it's way less ugly
to just use regular DCHECK IMO.)

[Bernhard Bauer, 2016/05/17 12:38:13]

Yuck. TBH, I'm less and less convinced that enum classes are All That.

Oh, and I guess appending |state_| to the DCHECK to log it doesn't work either,
because operator<< isn't defined, right?

[Marc Treib, 2016/05/17 12:46:34]

Yup, the missing operator<< is the reason DCHECK_EQ doesn't work.
Would you prefer a non-class enum? Or alternatively, it should be possible to
explicitly overload operator<< (and cast to int there). WDYT?

+  state_ = State::INITED;
+
   enabled_ = enabled;
   if (enabled_) {
     // |suggestions_service_| can be null in tests.
     if (suggestions_service_) {
       suggestions_service_subscription_ = suggestions_service_->AddCallback(
           base::Bind(&NTPSnippetsService::OnSuggestionsChanged,
                      base::Unretained(this)));
     }
 
     // Get any existing snippets immediately from prefs.
     LoadDiscardedSnippetsFromPrefs();
     LoadSnippetsFromPrefs();
 
     // If we don't have any snippets yet, start a fetch.
     if (snippets_.empty())
       FetchSnippets();
   }
 
   RescheduleFetching();
 }
 
 void NTPSnippetsService::Shutdown() {
+  DCHECK(state_ == State::INITED);
+  state_ = State::SHUT_DOWN;
+
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceShutdown());
+  suggestions_service_subscription_.reset();
   enabled_ = false;
 }
 
 void NTPSnippetsService::FetchSnippets() {
   FetchSnippetsFromHosts(GetSuggestionsHosts());
 }
 
 void NTPSnippetsService::FetchSnippetsFromHosts(
     const std::set<std::string>& hosts) {
   snippets_fetcher_->FetchSnippetsFromHosts(hosts, application_language_code_,
@@ skipping 123 matching lines @@
     // Sparse histogram used because the number of snippets is small (bound by
     // kMaxSnippetCount).
     DCHECK_LE(snippets->size(), static_cast<size_t>(kMaxSnippetCount));
     UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticlesFetched",
                                 snippets->size());
     MergeSnippets(std::move(*snippets));
   }
   LoadingSnippetsFinished();
 }
 
-void NTPSnippetsService::MergeSnippets(NTPSnippetStorage new_snippets) {
+void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
   // Remove new snippets that we already have, or that have been discarded.
   new_snippets.erase(
       std::remove_if(new_snippets.begin(), new_snippets.end(),
                      [this](const std::unique_ptr<NTPSnippet>& snippet) {
                        return ContainsSnippet(discarded_snippets_, snippet) ||
                               ContainsSnippet(snippets_, snippet);
                      }),
       new_snippets.end());
 
   // Fill in default publish/expiry dates where required.
@@ skipping 29 matching lines @@
     }
   }
 
   // Insert the new snippets at the front.
   snippets_.insert(snippets_.begin(),
                    std::make_move_iterator(new_snippets.begin()),
                    std::make_move_iterator(new_snippets.end()));
 }
 
 void NTPSnippetsService::LoadSnippetsFromPrefs() {
-  NTPSnippetStorage prefs_snippets;
+  NTPSnippet::PtrVector prefs_snippets;
   bool success = NTPSnippet::AddFromListValue(
       *pref_service_->GetList(prefs::kSnippets), &prefs_snippets);
   DCHECK(success) << "Failed to parse snippets from prefs";
   MergeSnippets(std::move(prefs_snippets));
   LoadingSnippetsFinished();
 }
 
 void NTPSnippetsService::StoreSnippetsToPrefs() {
   pref_service_->Set(prefs::kSnippets, *SnippetsToListValue(snippets_));
 }
@@ skipping 79 matching lines @@
     if (snippet->expiry_date() < next_expiry)
       next_expiry = snippet->expiry_date();
   }
   DCHECK_GT(next_expiry, expiry);
   expiry_timer_.Start(FROM_HERE, next_expiry - expiry,
                       base::Bind(&NTPSnippetsService::LoadingSnippetsFinished,
                                  base::Unretained(this)));
 }
 
 }  // namespace ntp_snippets