[NTP Snippets] Don't show the bookmark section if there are no bookmarks at all

components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc

Index: components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
diff --git a/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc b/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
index a8dd863f117279ebf4c886ef4b15ff29dff31a0c..35626874a8333b054fdce775d5f113bcc14ec53f 100644
--- a/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
+++ b/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
@@ -41,6 +41,7 @@ const char* kMinBookmarksParamName = "bookmarks_min_count";
 const char* kMaxBookmarkAgeInDaysParamName = "bookmarks_max_age_in_days";
 const char* kUseCreationDateFallbackForDaysParamName =
     "bookmarks_creation_date_fallback_days";
+const char* kShowIfEmptyParamName = "bookmarks_show_if_empty";
 
 base::Time GetThresholdTime() {
   std::string age_in_days_string = variations::GetVariationParamValueByFeature(
@@ -74,7 +75,7 @@ int GetMaxCount() {
   if (base::StringToInt(max_count_string, &max_count))
     return max_count;
   if (!max_count_string.empty())
-    LOG(WARNING) << "Failed to parse max bookmarks count" << max_count_string;
+    LOG(WARNING) << "Failed to parse max bookmarks count " << max_count_string;
 
   return kMaxBookmarks;
 }
@@ -86,11 +87,23 @@ int GetMinCount() {
   if (base::StringToInt(min_count_string, &min_count))
     return min_count;
   if (!min_count_string.empty())
-    LOG(WARNING) << "Failed to parse min bookmarks count" << min_count_string;
+    LOG(WARNING) << "Failed to parse min bookmarks count " << min_count_string;
 
   return kMinBookmarks;
 }
 
+bool ShowIfEmpty() {

[Bernhard Bauer, 2016/08/25 10:07:20]

Nit: Maybe "ShouldShowIfEmpty()" to make it clear this is not an imperative?

[Marc Treib, 2016/08/25 10:15:09]

Done.

+  std::string show_if_empty = variations::GetVariationParamValueByFeature(
+      ntp_snippets::kBookmarkSuggestionsFeature, kShowIfEmptyParamName);
+  if (show_if_empty.empty() || show_if_empty == "0")

[Bernhard Bauer, 2016/08/25 10:07:20]

If the variation parameters are strings, why not just use "true" and "false"?

[Marc Treib, 2016/08/25 10:15:09]

No reason :)  Done.

+    return false;
+  if (show_if_empty == "1")
+    return true;
+
+  LOG(WARNING) << "Failed to parse show if empty " << show_if_empty;
+  return false;
+}
+
 }  // namespace
 
 namespace ntp_snippets {
@@ -151,7 +164,7 @@ CategoryInfo BookmarkSuggestionsProvider::GetCategoryInfo(Category category) {
       l10n_util::GetStringUTF16(IDS_NTP_BOOKMARK_SUGGESTIONS_SECTION_HEADER),
       ContentSuggestionsCardLayout::MINIMAL_CARD,
       /* has_more_button */ true,
-      /* show_if_empty */ true);
+      /* show_if_empty */ ShowIfEmpty() && bookmark_model_->HasBookmarks());
   // TODO(treib): Setting show_if_empty to true is a temporary hack, see
   // crbug.com/640568.
 }