[Suggestions Service] Add support for expiring the SuggestionsStore

components/suggestions/suggestions_store.cc

Index: components/suggestions/suggestions_store.cc
diff --git a/components/suggestions/suggestions_store.cc b/components/suggestions/suggestions_store.cc
index 1d1147b342c12650cbc15f65bbd86cb5c32bb16d..3249d8881446d7f1aae2feee199ceb3c3e06a8a8 100644
--- a/components/suggestions/suggestions_store.cc
+++ b/components/suggestions/suggestions_store.cc
@@ -4,13 +4,16 @@
 
 #include "components/suggestions/suggestions_store.h"
 
+#include <ctime>

[manzagop (departed), 2014/07/31 15:31:51]

Used?

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Removed

 #include <string>
 
 #include "base/base64.h"
 #include "base/prefs/pref_service.h"
+#include "base/time/time.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/suggestions/suggestions_pref_names.h"
 
+

[manzagop (departed), 2014/07/31 15:31:51]

No need for this line.

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Done.

 namespace suggestions {
 
 SuggestionsStore::SuggestionsStore(PrefService* profile_prefs)
@@ -40,9 +43,44 @@ bool SuggestionsStore::LoadSuggestions(SuggestionsProfile* suggestions) {
     return false;
   }
 
+  // save number of suggestions before filtering expired suggestions

[manzagop (departed), 2014/07/31 15:31:51]

Capitalize Save

[gayane -on leave until 09-2017, 2014/08/04 13:46:30]

Done.

+  int unfiltered_size = suggestions->suggestions_size();
+  FilterExpiredSuggestions(suggestions);
+
+  // if any suggestion was expired update stored suggestions.
+  if (suggestions->suggestions_size() != unfiltered_size){
+    StoreSuggestions(*suggestions);
+  }
+
+  // if all suggestions were expired return false.

[manzagop (departed), 2014/07/31 15:31:51]

These 3 code blocks don't need to be seperated by empty lines and you could have
a single comment for all of them, eg:

// Filter expired suggestions and update the stored suggestions if at least one
was filtered.

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Done.

+  if (suggestions->suggestions_size() == 0){
+    suggestions->Clear();
+    return false;
+  }
+
   return true;
 }
 
+// private

[manzagop (departed), 2014/07/31 15:31:51]

I think I've only seen this kind of comment for static functions, so I'd remove
it.

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Done.

+void SuggestionsStore::FilterExpiredSuggestions(
+    SuggestionsProfile* suggestions) {
+  SuggestionsProfile* filteredSuggestions = new SuggestionsProfile();
+  int64 now = (base::Time::NowFromSystemTime()
+              -base::Time::UnixEpoch()).ToInternalValue();

[manzagop (departed), 2014/07/31 15:31:50]

Indentation. See suggestions_source for details.

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Done.

+
+  for (int i = 0; i < suggestions->suggestions_size(); i++) {

[manzagop (departed), 2014/07/31 15:31:50]

Even though not mandatory in this case, I believe preincrementing i is more
common in this case:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Vertic...

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Done.

+    auto* suggestion = suggestions->mutable_suggestions(i);

[manzagop (departed), 2014/07/31 15:31:50]

AFAIK auto is not allowed yet since chromium hasn't moved to 11 yet.

[gayane -on leave until 09-2017, 2014/08/04 13:46:31]

Done.

+
+    // copy if valid

[manzagop (departed), 2014/07/31 15:31:51]

Comment does not add much so I'd remove it. You're also not copying.

[gayane -on leave until 09-2017, 2014/08/04 13:46:30]

Done.

+    if (!suggestion->has_expiry_ts() || suggestion->expiry_ts() > now) {
+        ChromeSuggestion* tmp = filteredSuggestions->add_suggestions();
+        tmp->Swap(suggestion);
+    }
+  }
+
+  suggestions->Swap(filteredSuggestions);
+}
+
 bool SuggestionsStore::StoreSuggestions(const SuggestionsProfile& suggestions) {
   std::string suggestions_data;
   if (!suggestions.SerializeToString(&suggestions_data)) return false;