[Policy] HostContentSettingsMap: Add AreUserExceptionsAllowedForType() and tests.

chrome/browser/content_settings/host_content_settings_map_unittest.cc

Index: chrome/browser/content_settings/host_content_settings_map_unittest.cc
diff --git a/chrome/browser/content_settings/host_content_settings_map_unittest.cc b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
index 945eabc17cfb4ef048a6050b7a16709cb3e9e8e7..ed9cd70ed5434a708d61cb27ca5a220a2c543767 100644
--- a/chrome/browser/content_settings/host_content_settings_map_unittest.cc
+++ b/chrome/browser/content_settings/host_content_settings_map_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
@@ -19,6 +21,7 @@
 #include "components/content_settings/core/browser/website_settings_info.h"
 #include "components/content_settings/core/browser/website_settings_registry.h"
 #include "components/content_settings/core/common/pref_names.h"
+#include "components/content_settings/core/test/content_settings_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/syncable_prefs/testing_pref_service_syncable.h"
@@ -47,6 +50,76 @@ class HostContentSettingsMapTest : public testing::Test {
   content::TestBrowserThread ui_thread_;
 };
 
+// Helper that wrap test boilerplates, and maintains a "current content type"

[huangs, 2016/04/11 21:37:33]

Adding this layer of abstraction makes test much smaller, but also makes them
less "hands on". If you'd prefer that I don't do this, can I use the lambda
approach as seen in
https://codereview.chromium.org/1865803002/diff/140001/chrome/browser/content...
?

+// state so caller needs not repeatedly specify content type.
+class HostContentSettingsMapTester {

[raymes, 2016/04/12 04:52:47]

Since these are specific to your tests, how about calling this
PolicyDefaultTester (or something similar)?

You could also make this a test class that your tests inherit from, similar to
HostContentSettingsMapTest above.

[huangs, 2016/04/13 17:36:18]

Changing this to just a wrapper:
- Renaming to TesterForType.
- TestingProfile will be owned by caller.
- Type is injected at construction.

+ public:
+  HostContentSettingsMapTester() {
+    profile_.reset(new TestingProfile);
+    prefs_ = profile_->GetTestingPrefService();
+    host_content_settings_map_ =
+        HostContentSettingsMapFactory::GetForProfile(profile_.get());
+  }
+
+  // Chainable.
+  HostContentSettingsMapTester& SelectType(ContentSettingsType content_type) {

[raymes, 2016/04/12 04:52:47]

nit: How about removing this function (and not tracking the "cur" state in this
class). Instead make ClearPolicyDefault and SetPolicyDefault take a
ContentSettingsType and lookup the pref based on that type.

[huangs, 2016/04/12 14:19:13]

The ability to select "cur" state once to establish context, and avoid
repetition was the main purpose of this helper class.  But if the preferred
style is to explicitly state |content_type| then I can remove this (need
feedback).

+    cur_content_type_ = content_type;
+    switch (cur_content_type_) {
+      case CONTENT_SETTINGS_TYPE_COOKIES:
+        cur_policy_default_setting_ = prefs::kManagedDefaultCookiesSetting;
+        break;
+      case CONTENT_SETTINGS_TYPE_POPUPS:
+        cur_policy_default_setting_ = prefs::kManagedDefaultPopupsSetting;
+        break;
+      default:
+        // Add support as needed.
+        NOTREACHED();
+    }
+    return *this;
+  }
+
+  void ClearPolicyDefault() {
+    prefs_->RemoveManagedPref(cur_policy_default_setting_);
+  }
+
+  void SetPolicyDefault(ContentSetting setting) {
+    prefs_->SetManagedPref(cur_policy_default_setting_,
+                           new base::FundamentalValue(setting));
+  }
+
+  bool AreUserExceptionsAllowed() {
+    return host_content_settings_map_->AreUserExceptionsAllowedForType(
+        cur_content_type_);
+  }
+
+  void AddUserException(std::string exception,
+                        ContentSetting content_settings) {
+    content_settings::TestUtils::AddUserException(exception, cur_content_type_,
+        content_settings, host_content_settings_map_);
+  }
+
+  // Wrapper to query GetWebsiteSetting(), and only return the source.
+  content_settings::SettingSource SourceMatch(

[raymes, 2016/04/12 04:52:47]

nit: GetSettingSourceForURL?

[huangs, 2016/04/13 17:36:18]

Done.

+      const std::string& url_str) {
+    GURL host(url_str);

[Bernhard Bauer, 2016/04/12 09:30:05]

Should this be |url|?

[huangs, 2016/04/13 17:36:18]

Done.

+    content_settings::SettingInfo setting_info;
+    scoped_ptr<base::Value> result =
+        host_content_settings_map_->GetWebsiteSetting(
+            host, host, cur_content_type_, std::string(),
+            &setting_info);
+    return setting_info.source;
+  };
+
+ private:
+  std::unique_ptr<TestingProfile> profile_;
+  syncable_prefs::TestingPrefServiceSyncable* prefs_ = nullptr;
+  HostContentSettingsMap* host_content_settings_map_ = nullptr;
+  ContentSettingsType cur_content_type_ = CONTENT_SETTINGS_TYPE_DEFAULT;
+  const char* cur_policy_default_setting_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostContentSettingsMapTester);
+};
+
 TEST_F(HostContentSettingsMapTest, DefaultValues) {
   TestingProfile profile;
   HostContentSettingsMap* host_content_settings_map =
@@ -773,6 +846,67 @@ TEST_F(HostContentSettingsMapTest, OffTheRecordDontInheritSetting) {
                          std::string(), nullptr));
 }
 
+TEST_F(HostContentSettingsMapTest, AreUserExceptionsAllowedForType) {
+  ContentSettingsType kContentTypesToTest[] = {
+    CONTENT_SETTINGS_TYPE_COOKIES,
+    CONTENT_SETTINGS_TYPE_POPUPS,
+  };
+
+  HostContentSettingsMapTester tester;

[Bernhard Bauer, 2016/04/12 09:30:05]

Move this into the loop to limit its scope?

[huangs, 2016/04/13 17:36:18]

Done.  Note that I moved TestingProfile outside, and now the tester is just a
wrapper.

+
+  for (ContentSettingsType type : kContentTypesToTest) {
+    tester.SelectType(type);
+
+    // No settings: Yes.
+    tester.ClearPolicyDefault();
+    EXPECT_TRUE(tester.AreUserExceptionsAllowed());
+
+    // Policy enforces default value: No.
+    tester.SetPolicyDefault(CONTENT_SETTING_ALLOW);
+    EXPECT_FALSE(tester.AreUserExceptionsAllowed());
+    tester.SetPolicyDefault(CONTENT_SETTING_BLOCK);
+    EXPECT_FALSE(tester.AreUserExceptionsAllowed());
+
+    // Not calling tester.ClearPolicyDefault(); let it linger.

[raymes, 2016/04/12 04:52:47]

Should we clean up the state?

[huangs, 2016/04/13 17:36:18]

Done.

+  }
+}
+
+TEST_F(HostContentSettingsMapTest, PrefExceptionsOperation) {
+  using content_settings::SETTING_SOURCE_POLICY;
+  using content_settings::SETTING_SOURCE_USER;
+
+  const char kUrl1[] = "http://user_exception_allow.com";
+  const char kUrl2[] = "http://user_exception_block.com";
+  const char kUrl3[] = "http://non_exception.com";
+
+  HostContentSettingsMapTester tester;
+
+  // Arbitrarily using cookies as content type to test.
+  tester.SelectType(CONTENT_SETTINGS_TYPE_COOKIES);
+
+  // Add |kUrl1| and |kUrl2| only.
+  tester.AddUserException(kUrl1, CONTENT_SETTING_ALLOW);
+  tester.AddUserException(kUrl2, CONTENT_SETTING_BLOCK);
+
+  // No policy setting: follow users settings.
+  tester.ClearPolicyDefault();
+  // User exceptions.
+  EXPECT_EQ(SETTING_SOURCE_USER, tester.SourceMatch(kUrl1));
+  EXPECT_EQ(SETTING_SOURCE_USER, tester.SourceMatch(kUrl2));
+  // User default.
+  EXPECT_EQ(SETTING_SOURCE_USER, tester.SourceMatch(kUrl3));
+
+  // Policy overrides users always.
+  tester.SetPolicyDefault(CONTENT_SETTING_ALLOW);
+  EXPECT_EQ(SETTING_SOURCE_POLICY, tester.SourceMatch(kUrl1));
+  EXPECT_EQ(SETTING_SOURCE_POLICY, tester.SourceMatch(kUrl2));
+  EXPECT_EQ(SETTING_SOURCE_POLICY, tester.SourceMatch(kUrl3));
+  tester.SetPolicyDefault(CONTENT_SETTING_BLOCK);
+  EXPECT_EQ(SETTING_SOURCE_POLICY, tester.SourceMatch(kUrl1));
+  EXPECT_EQ(SETTING_SOURCE_POLICY, tester.SourceMatch(kUrl2));
+  EXPECT_EQ(SETTING_SOURCE_POLICY, tester.SourceMatch(kUrl3));
+}
+
 // For a single Unicode encoded pattern, check if it gets converted to punycode
 // and old pattern gets deleted.
 TEST_F(HostContentSettingsMapTest, CanonicalizeExceptionsUnicodeOnly) {