[Suggestions Service] Add support for expiring the SuggestionsStore

components/suggestions/suggestions_store.cc

 // Copyright 2014 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/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)
     : pref_service_(profile_prefs) {
   DCHECK(profile_prefs);
 }
 
 SuggestionsStore::~SuggestionsStore() {}
 
 bool SuggestionsStore::LoadSuggestions(SuggestionsProfile* suggestions) {
   DCHECK(suggestions);
 
   const std::string base64_suggestions_data =
       pref_service_->GetString(prefs::kSuggestionsData);
   if (base64_suggestions_data.empty()) {
     suggestions->Clear();
     return false;
   }
 
   // If the decode process fails, assume the pref value is corrupt and clear it.
   std::string suggestions_data;
   if (!base::Base64Decode(base64_suggestions_data, &suggestions_data) ||
       !suggestions->ParseFromString(suggestions_data)) {
     VLOG(1) << "Suggestions data in profile pref is corrupt, clearing it.";
     suggestions->Clear();
     ClearSuggestions();
     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;
 
   std::string base64_suggestions_data;
   base::Base64Encode(suggestions_data, &base64_suggestions_data);
 
   pref_service_->SetString(prefs::kSuggestionsData, base64_suggestions_data);
   return true;
 }
 
 void SuggestionsStore::ClearSuggestions() {
   pref_service_->ClearPref(prefs::kSuggestionsData);
 }
 
 // static
 void SuggestionsStore::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterStringPref(
       prefs::kSuggestionsData, std::string(),
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 }  // namespace suggestions