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

components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc

 // Copyright 2016 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/bookmarks/bookmark_suggestions_provider.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 23 matching lines @@
 const int kMaxBookmarks = 10;
 const int kMinBookmarks = 3;
 const int kMaxBookmarkAgeInDays = 42;
 const int kUseCreationDateFallbackForDays = 42;
 
 const char* kMaxBookmarksParamName = "bookmarks_max_count";
 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(
     ntp_snippets::kBookmarkSuggestionsFeature, kMaxBookmarkAgeInDaysParamName);
   int age_in_days = 0;
   if (!base::StringToInt(age_in_days_string, &age_in_days)) {
     if (!age_in_days_string.empty())
       LOG(WARNING) << "Failed to parse bookmark age " << age_in_days_string;
     age_in_days = kMaxBookmarkAgeInDays;
   }
@@ skipping 13 matching lines @@
   return days;
 }
 
 int GetMaxCount() {
   std::string max_count_string = variations::GetVariationParamValueByFeature(
       ntp_snippets::kBookmarkSuggestionsFeature, kMaxBookmarksParamName);
   int max_count = 0;
   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;
 }
 
 int GetMinCount() {
   std::string min_count_string = variations::GetVariationParamValueByFeature(
       ntp_snippets::kBookmarkSuggestionsFeature, kMinBookmarksParamName);
   int min_count = 0;
   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 {
 
 BookmarkSuggestionsProvider::BookmarkSuggestionsProvider(
     ContentSuggestionsProvider::Observer* observer,
     CategoryFactory* category_factory,
     bookmarks::BookmarkModel* bookmark_model,
     PrefService* pref_service)
     : ContentSuggestionsProvider(observer, category_factory),
@@ skipping 40 matching lines @@
     Category category) {
   DCHECK_EQ(category, provided_category_);
   return category_status_;
 }
 
 CategoryInfo BookmarkSuggestionsProvider::GetCategoryInfo(Category category) {
   return CategoryInfo(
       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.
 }
 
 void BookmarkSuggestionsProvider::DismissSuggestion(
     const std::string& suggestion_id) {
   DCHECK(bookmark_model_->loaded());
   GURL url(GetWithinCategoryIDFromUniqueID(suggestion_id));
   MarkBookmarksDismissed(bookmark_model_, url);
 }
@@ skipping 124 matching lines @@
 
 void BookmarkSuggestionsProvider::NotifyStatusChanged(
     CategoryStatus new_status) {
   if (category_status_ == new_status)
     return;
   category_status_ = new_status;
   observer()->OnCategoryStatusChanged(this, provided_category_, new_status);
 }
 
 }  // namespace ntp_snippets