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..55c1cd8cd7cfb7eeff89ff39811edd589949f55e 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,16 @@ 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);
 
-  // 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).

[Pam (message me for reviews), 2012/12/06 15:11:56]

Please describe what |whitelist| is (presumably, a list of URL patterns).

Also, the separation between the whitelist in the prefs and the whitelist in the
URL filter (which is really just a parsed/cached copy of the one in the prefs)
is pretty hard to follow. I'd suggest requiring that the two are always in sync
(that is, not having public AddURLPatternToManual*List methods that only do one
part). Am I missing a use case for that?

[Sergiu, 2013/01/04 15:44:07]

Added a paragraph describing whitelists and url_patterns and made
AddURLPatternToManualList a private function.

+  static void AddToManualWhitelist(const base::ListValue& whitelist);
 
-  // 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 +159,13 @@ class ManagedMode : public chrome::BrowserListObserver,
 
   void UpdateWhitelist();
 
+  // Returns a copy of the manual whitelist which is stored in each profile.

[Pam (message me for reviews), 2012/12/06 15:11:56]

Meaning the one from the prefs? Also, what format is the list in (again, I
suppose it's a list of std::string URL patterns)? If possible, define a type for
the list, so it's self-evident what to expect.

[Sergiu, 2013/01/04 15:44:07]

I've typedefed base::ListValue to URLPatternList, hopefully it is clearer now.

[Bernhard Bauer, 2013/01/07 12:34:19]

I'm sorry, but I disagree. When I see a base::ListValue type, I at least know
the class, just not the type of the list contents. With URLPatternList I have to
look up the definition of the type (and how do I know where it's defined?), and
you don't even gain anything in type safety. The comment is great, but I don't
think we should have the typedef.

+  scoped_ptr<base::ListValue> GetWhitelist();
+
+  void AddToManualWhitelistImpl(const base::ListValue& whitelist);
+
+  bool IsInManualWhitelistImpl(const std::string& url_pattern);
+
   content::NotificationRegistrar registrar_;
   scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;