[Sync] Initial implementation of foreign sessions suggestions provider.

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 11 matching lines @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
 
 using bookmarks::BookmarkModel;
 using bookmarks::BookmarkNode;
 
+namespace ntp_snippets {
+
 namespace {
 
 const int kMaxBookmarks = 10;
 const int kMinBookmarks = 3;
 const int kMaxBookmarkAgeInDays = 42;
 const int kUseCreationDateFallbackForDays = 0;
 
 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";
 
+// Any bookmark created or visited after this time will be considered recent.
+// Note that bookmarks can be shown that do not meet this threshold.
 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;
-  }
-  return base::Time::Now() - base::TimeDelta::FromDays(age_in_days);
+  return base::Time::Now() -
+         base::TimeDelta::FromDays(GetParamAsInt(
+             ntp_snippets::kBookmarkSuggestionsFeature,
+             kMaxBookmarkAgeInDaysParamName, kMaxBookmarkAgeInDays));
 }
 
+// Should the creation timestamp of a bookmark be used as a fallback if no last
+// visited timestamp is present.

[Marc Treib, 2016/09/16 12:26:48]

nit: This sounds like it would return a bool.

[skym, 2016/09/16 18:18:47]

This value is confusing. Updated.

 int UseCreationDateFallbackForDays() {
-  std::string days_string = variations::GetVariationParamValueByFeature(
-      ntp_snippets::kBookmarkSuggestionsFeature,
-      kUseCreationDateFallbackForDaysParamName);
-  int days = 0;
-  if (!base::StringToInt(days_string, &days)) {
-    if (!days_string.empty())
-      LOG(WARNING) << "Failed to parse bookmark fallback days " << days_string;
-    days = kUseCreationDateFallbackForDays;
-  }
-  return days;
+  return GetParamAsInt(ntp_snippets::kBookmarkSuggestionsFeature,
+                       kUseCreationDateFallbackForDaysParamName,
+                       kUseCreationDateFallbackForDays);
 }
 
+// The maximum number of suggestions ever provided.
 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;
-
-  return kMaxBookmarks;
+  return GetParamAsInt(ntp_snippets::kBookmarkSuggestionsFeature,
+                       kMaxBookmarksParamName, kMaxBookmarks);
 }
 
+// The minimum number of suggestions ever provided. If there are not enough
+// recent bookmarks to meet this minimum, then bookmarks more stale than the
+// threshold are used.

[Marc Treib, 2016/09/16 12:26:48]

I don't think this quite matches the behavior (which is super confusing and has
way too many modes, due to last-minute changes to the product design...).
This value only matters at all if the creation date fallback is active.

[skym, 2016/09/16 18:18:47]

Okay, I've updated this, let me know if you thinks its okay now.

 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;
-
-  return kMinBookmarks;
+  return GetParamAsInt(ntp_snippets::kBookmarkSuggestionsFeature,
+                       kMinBookmarksParamName, kMinBookmarks);
 }
 
 bool ShouldShowIfEmpty() {
   std::string show_if_empty = variations::GetVariationParamValueByFeature(
       ntp_snippets::kBookmarkSuggestionsFeature, kShowIfEmptyParamName);
   if (show_if_empty.empty() || show_if_empty == "false")
     return false;
   if (show_if_empty == "true")
     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),
       category_status_(CategoryStatus::AVAILABLE_LOADING),
       provided_category_(
           category_factory->FromKnownCategory(KnownCategories::BOOKMARKS)),
       bookmark_model_(bookmark_model),
@@ skipping 200 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