SuggestionsService undo blacklist functionality.

components/suggestions/blacklist_store.h

Index: components/suggestions/blacklist_store.h
diff --git a/components/suggestions/blacklist_store.h b/components/suggestions/blacklist_store.h
index 072e3f69c0e2b3120b847c9c3d733f0113401686..6b811d8202881baf2542dca01d89b69d4f5725e2 100644
--- a/components/suggestions/blacklist_store.h
+++ b/components/suggestions/blacklist_store.h
@@ -5,7 +5,11 @@
 #ifndef COMPONENTS_SUGGESTIONS_BLACKLIST_STORE_H_
 #define COMPONENTS_SUGGESTIONS_BLACKLIST_STORE_H_
 
+#include <map>
+#include <string>
+
 #include "base/macros.h"
+#include "base/time/time.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "url/gurl.h"
 
@@ -17,23 +21,41 @@ class PrefRegistrySyncable;
 
 namespace suggestions {
 
-// A helper class for reading, writing and modifying a small blacklist stored
-// in the Profile preferences. It also handles SuggestionsProfile
-// filtering based on the stored blacklist.
+// A helper class for reading, writing, modifying and applying a small URL
+// blacklist, pending upload to the server. The class has a concept of time
+// duration before which a blacklisted URL becomes candidate for upload to the
+// server. Keep in mind most of the operations involve interaction with the disk
+// (the profile's preferences).
 class BlacklistStore {
  public:
-  explicit BlacklistStore(PrefService* profile_prefs);
+  BlacklistStore(
+      PrefService* profile_prefs,
+      const base::TimeDelta& upload_delay = base::TimeDelta::FromSeconds(15));
   virtual ~BlacklistStore();
 
-  // Returns true if successful or |url| was already in the blacklist.
+  // Returns true if successful or |url| was already in the blacklist. If |url|
+  // was already in the blacklist, its blacklisting timestamp gets updated.
   virtual bool BlacklistUrl(const GURL& url);
 
-  // Sets |url| to the first URL from the blacklist. Returns false if the
+  // Returns true if the blacklist is empty.
+  virtual bool IsEmpty();

[Mathieu, 2014/12/04 18:53:32]

does this need to be public? I can't really see how it's used.

[manzagop (departed), 2014/12/05 15:13:22]

Unused. Got rid of the function.

+
+  // Gets the time until any URL is ready for upload. Returns false if the
   // blacklist is empty.
-  virtual bool GetFirstUrlFromBlacklist(GURL* url);
+  virtual bool GetTimeUntilReadyForUpload(base::TimeDelta* delta);
+
+  // Gets the time until |url| is ready for upload. Returns false if |url| is
+  // not part of the blacklist.
+  virtual bool GetTimeUntilReadyForUpload(const GURL& url,

[Mathieu, 2014/12/04 18:53:32]

I get easily confused by two methods with identical name.

Could we have GetTimeUntilURLReadyForUpload?

[manzagop (departed), 2014/12/05 15:13:22]

Done.

+                                          base::TimeDelta* delta);
 
-  // Removes |url| from the stored blacklist. Returns true if successful or if
-  // |url| is not in the blacklist.
+  // Sets |url| to a URL from the blacklist that is candidate for upload.
+  // Returns false if there is no candidate for upload.
+  virtual bool GetCandidateForUpload(GURL* url);
+
+  // Removes |url| from the stored blacklist. Returns true if successful, false
+  // on failure or if |url| was not in the blacklist. Note that this function
+  // does not enforce a minimum time since blacklist before removal.
   virtual bool RemoveUrl(const GURL& url);
 
   // Applies the blacklist to |suggestions|.
@@ -63,6 +85,15 @@ class BlacklistStore {
   // The pref service used to persist the suggestions blacklist.
   PrefService* pref_service_;
 
+  // Delay after which a URL becomes candidate for upload, measured from the
+  // last time the URL was added.
+  base::TimeDelta upload_delay_;
+
+  // The times at which URLs were blacklisted. Used to determine when a URL is
+  // valid for server upload. Guaranteed to contain URLs that are not ready for
+  // upload. Might not contain URLs that are ready for upload.
+  std::map<std::string, base::TimeTicks> blacklist_times_;
+
   DISALLOW_COPY_AND_ASSIGN(BlacklistStore);
 };