RemoteContentSuggestions: Stores the time of the last successful background fetch in a pref

components/ntp_snippets/remote/ntp_snippets_fetcher.h

Index: components/ntp_snippets/remote/ntp_snippets_fetcher.h
diff --git a/components/ntp_snippets/remote/ntp_snippets_fetcher.h b/components/ntp_snippets/remote/ntp_snippets_fetcher.h
index 62437c99096287262a34c2022b1411af6ca86f7f..c3dd5ca91c1cd0f77992ad0db3d909663db5141f 100644
--- a/components/ntp_snippets/remote/ntp_snippets_fetcher.h
+++ b/components/ntp_snippets/remote/ntp_snippets_fetcher.h
@@ -80,12 +80,6 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
   using FetchedCategoriesVector = std::vector<FetchedCategory>;
   using OptionalFetchedCategories = base::Optional<FetchedCategoriesVector>;
 
-  // |snippets| contains parsed snippets if a fetch succeeded. If problems
-  // occur, |snippets| contains no value (no actual vector in base::Optional).
-  // Error details can be retrieved using last_status().
-  using SnippetsAvailableCallback =
-      base::OnceCallback<void(OptionalFetchedCategories fetched_categories)>;
-
   // Enumeration listing all possible outcomes for fetch attempts. Used for UMA
   // histograms, so do not change existing values. Insert new values at the end,
   // and update the histogram definition.
@@ -101,6 +95,12 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
     NON_INTERACTIVE_QUOTA_ERROR,
     RESULT_MAX
   };
+  // |snippets| contains parsed snippets if a fetch succeeded. If problems

[Marc Treib, 2016/12/07 14:42:13]

nit: Empty line before.

[markusheintz_, 2016/12/08 09:47:41]

Done.

+  // occur, |snippets| contains no value (no actual vector in base::Optional).
+  // Error details can be retrieved using last_status().
+  using SnippetsAvailableCallback =
+      base::OnceCallback<void(OptionalFetchedCategories fetched_categories,
+                              FetchResult fetch_result)>;

[Marc Treib, 2016/12/07 14:42:13]

nit: I'd find it a bit more natural to have the FetchResult first, and the
(optional) data second. WDYT?

[markusheintz_, 2016/12/08 09:47:41]

I have absolutely no hard feeling on this. But I guess I would pass on this one 
:-). If seen it both ways and I guess it does not really matter. 
while naturally you would want to check first the result status and then look at
the results.

But on a related note the name fetch_result is kind of not expressing that this
is actually a success status. So I guess I can make you happy by changing the
order, when finding a better name for the param?
=> Followup CL?

[Marc Treib, 2016/12/08 10:33:48]

I think if we are gonna change the order at all, then we should do it in this CL
which introduces the new param. Otherwise it's just more churn.
So, I'd still prefer to have the result code first, but it's not important
enough to fight over it ;)

For the name, FetchResultCode maybe, to make clear that it's just a status code
rather than the actual result data? (This is definitely for a separate CL
though)

[markusheintz_, 2016/12/08 11:37:18]

Since Christmas is close here is my present for you :-)
done

[Marc Treib, 2016/12/08 12:15:40]

Woohoo! :D

 
   // Enumeration listing all possible variants of dealing with personalization.
   enum class Personalization { kPersonal, kNonPersonal, kBoth };