Support manual (white|black)list, previewing and allowing after interstitial

chrome/browser/managed_mode/managed_mode.h

Index: chrome/browser/managed_mode/managed_mode.h
diff --git a/chrome/browser/managed_mode/managed_mode.h b/chrome/browser/managed_mode/managed_mode.h
index 3c4126d863b277a2b7294f548c9c89384e935d24..97a7b5c1a13ea488d076c3a2247d74f36847ea6a 100644
--- a/chrome/browser/managed_mode/managed_mode.h
+++ b/chrome/browser/managed_mode/managed_mode.h
@@ -26,6 +26,7 @@ class ManagedModeURLFilter;
 class PrefChangeRegistrar;
 class PrefService;
 class Profile;
+
 namespace policy{
 class URLBlacklist;
 }
@@ -70,15 +71,19 @@ class ManagedMode : public chrome::BrowserListObserver,
   // Called in the critical path of drawing the history UI, so needs to be fast.
   static void GetCategoryNames(CategoryList* list);
 
-  // Returns the profile whitelist.
-  static scoped_ptr<base::ListValue> GetWhitelist();
+  // Checks if the url pattern is in the manual whitelist.
+  static bool IsInManualWhitelist(const std::string& url_pattern);
+  bool IsInManualWhitelistImpl(const std::string& url_pattern);

[Bernhard Bauer, 2012/11/27 18:44:46]

Make this private please.

[Sergiu, 2012/11/28 12:53:25]

Done. I'm not sure about the order of the functions in .h though.. It makes more
sense to me to keep the non-Impl close to the Impl in the .cc but then they're
not really grouped by visibility. Any tips here?

[Bernhard Bauer, 2012/11/28 14:21:05]

You mean in the .cc file? In the .h file we don't have a choice.
I don't have a particular preference. Personally, I'm not very good at keeping
method definitions in the same order as declarations, so I would be okay with
grouping them.

 
-  // Sets the profile whitelist to |whitelist|.
-  static void SetWhitelist(base::ListValue* whitelist);
+  // Appends |whitelist| to the manual whitelist (both in URL filter and in
+  // preferences). Returns whether the last element in |whitelist| was already
+  // in the manual whitelist or not.

[Bernhard Bauer, 2012/11/27 18:44:46]

That seems… very specific. Could a client just use IsInManualWhitelist() for the
last element before adding them?

[Sergiu, 2012/11/28 12:53:25]

I'm not sure I get you here, AddToManualWhitelist adds a whole whitelist, not
just an entry. The bool is returned then to show the infobar if we didn't add
the last one.

[Bernhard Bauer, 2012/11/28 14:21:05]

Yes, it's just that an interface "add these, and tell me if the last one of them
was already on the list" is very specific to the client that calls it. So I was
asking if we could simplify the interface of this class by using other methods
(like the one we already have that tells whether a given site is on the
whitelist), which I see you did. So... 'kay.

+  static bool AddToManualWhitelist(const base::ListValue& whitelist);
+  bool AddToManualWhitelistImpl(const base::ListValue& whitelist);

[Bernhard Bauer, 2012/11/27 18:44:46]

Make this private.

[Sergiu, 2012/11/28 12:53:25]

Done.

 
-  // Adds the |url| to the manual lists.
-  static void AddStringToManualWhitelist(const std::string& url);
-  static void AddStringToManualBlacklist(const std::string& url);
+  // Adds the |url_pattern| to the manual lists in the URL filter.
+  static void AddURLPatternToManualWhitelist(const std::string& url_pattern);
+  static void AddURLPatternToManualBlacklist(const std::string& url_pattern);
 
   // Returns the profile blacklist.
   static scoped_ptr<base::ListValue> GetBlacklist();
@@ -157,6 +162,9 @@ class ManagedMode : public chrome::BrowserListObserver,
 
   void UpdateWhitelist();
 
+  // Returns a copy of the manual whitelist which is stored in each profile.
+  scoped_ptr<base::ListValue> GetWhitelist();
+
   content::NotificationRegistrar registrar_;
   scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;