Change NTPSnippetsBridge to read from ContentSuggestionsService

components/ntp_snippets/content_suggestions_category_status.h

Index: components/ntp_snippets/content_suggestions_category_status.h
diff --git a/components/ntp_snippets/content_suggestions_category_status.h b/components/ntp_snippets/content_suggestions_category_status.h
index 16b65af028bac7483e0e0bba18283b115bcffaa2..83b5fa2c6d85df5bbfcadb9393b92eba3e5c66f2 100644
--- a/components/ntp_snippets/content_suggestions_category_status.h
+++ b/components/ntp_snippets/content_suggestions_category_status.h
@@ -50,6 +50,12 @@ enum class ContentSuggestionsCategoryStatus {
 bool IsContentSuggestionsCategoryStatusAvailable(
     ContentSuggestionsCategoryStatus status);
 
+// Determines whether the given status is INITIALIZING or one of the AVAILABLE
+// statuses. All of these statuses have in common that there is or will soon be
+// content.
+bool IsContentSuggestionsCategoryStatusInitOrAvailable(
+    ContentSuggestionsCategoryStatus status);

[Marc Treib, 2016/07/22 11:44:42]

Hm, this almost seems so specific that callers might just as well hardcode the
individual values. Can we find some better name for this? Or maybe for its
negation?

[Philipp Keck, 2016/07/22 11:50:42]

Is..StatusNonError
Is..StatusNotErroneous

Alternative: Don't hardcode all the values (because there might be more
AVAILABLE_X values in the future and we want to avoid forgetting places to
update those and want to avoid verbose code). But we could instead use "status
== INITIALIZING || Is..StatusAvailable(status)" everywhere we use this function.

[Marc Treib, 2016/07/22 12:05:33]

I'd like to avoid negated functions like that, they tend to make conditions very
hard to read.
Something like Is...Error might work, though I wouldn't really consider all of
these statuses errors. IsNotAvailable would be more accurate, but then you'd
expect it to be the opposite of Available, which is also not true... argh.
Essentially it's a tri-state grouping: Available, Not Available, Initializing
(i.e. Unknown).
That might be okay, though it also looks a bit weird.

[Philipp Keck, 2016/07/22 12:12:17]

Exactly. And if those were A, B and C, at least I would prefer
if (A || B)
over
if (!C)

And the equivalent of A||B is "InitOrAvailable" ...
It may sound a bit specific, but it's just one of the interesting cases. Of
course, we shouldn't be making specific helper functions for all combinations.
We could introduce Is..Unknown or Is..Initializing that just checks for the one
value (at the moment) to make the "A||B" call look more symmetric.

[Marc Treib, 2016/07/22 12:23:34]

Eh, depends on the context... if we just want to check that it's not in a "bad"
state, then (!C) would be the right thing.
Anyway, just to be clear: I'm not blocking the CL over this; mostly just
bikeshedding here :)

+
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_CATEGORY_STATUS_H_