Bookmark Suggestions: Make 'creation date fallback' time configurable

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 21 matching lines @@
 namespace {
 
 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";
 
 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);
 }
 
+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;
+}
+
 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;
@@ skipping 33 matching lines @@
   if (pref_service->HasPrefPath(prefs::kBookmarksFirstM54Start)) {
     first_m54_start = base::Time::FromInternalValue(
         pref_service->GetInt64(prefs::kBookmarksFirstM54Start));
   } else {
     first_m54_start = now;
     pref_service->SetInt64(prefs::kBookmarksFirstM54Start,
                            first_m54_start.ToInternalValue());
   }
   base::TimeDelta time_since_first_m54_start = now - first_m54_start;
   creation_date_fallback_ =
-      time_since_first_m54_start.InDays() < kUseCreationDateFallbackForDays;
+      time_since_first_m54_start.InDays() < UseCreationDateFallbackForDays();

[sfiera, 2016/08/23 15:31:14]

Seems like setting "0" (or negative) would have the effect of disabling
fallback. Intentional? Do you want to document that?

[Marc Treib, 2016/08/23 15:35:19]

That's exactly the behavior you'd expect, no? Use the fallback for 0 days?

[sfiera, 2016/08/23 15:42:04]

I might expect that "0" would mean "use the compiled default".

[Marc Treib, 2016/08/23 16:09:38]

I'd expect a comment if it meant that!
Anyway, comment added.

   bookmark_model_->AddObserver(this);
   FetchBookmarks();
 }
 
 BookmarkSuggestionsProvider::~BookmarkSuggestionsProvider() {
   bookmark_model_->RemoveObserver(this);
 }
 
 // static
 void BookmarkSuggestionsProvider::RegisterProfilePrefs(
@@ skipping 152 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