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

chrome/browser/content_settings/host_content_settings_map_unittest.cc

 // Copyright (c) 2012 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 <memory>
+
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/content_settings/content_settings_mock_observer.h"
 #include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/mock_settings_observer.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/content_settings_details.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #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"
 #include "content/public/test/test_browser_thread.h"
 #include "net/base/static_cookie_policy.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 
 using ::testing::_;
 
 class HostContentSettingsMapTest : public testing::Test {
  public:
   HostContentSettingsMapTest() : ui_thread_(BrowserThread::UI, &message_loop_) {
   }
 
  protected:
   const std::string& GetPrefName(ContentSettingsType type) {
     return content_settings::WebsiteSettingsRegistry::GetInstance()
         ->Get(type)
         ->pref_name();
   }
 
   base::MessageLoop message_loop_;
   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 =
       HostContentSettingsMapFactory::GetForProfile(&profile);
 
   // Check setting defaults.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetDefaultContentSetting(
                 CONTENT_SETTINGS_TYPE_JAVASCRIPT, NULL));
   host_content_settings_map->SetDefaultContentSetting(
@@ skipping 706 matching lines @@
   scoped_ptr<base::Value> stored_value =
       host_content_settings_map->GetWebsiteSetting(
           host, host, CONTENT_SETTINGS_TYPE_USB_CHOOSER_DATA, std::string(),
           nullptr);
   EXPECT_TRUE(stored_value && stored_value->Equals(&test_value));
   EXPECT_EQ(nullptr, otr_map->GetWebsiteSetting(
                          host, host, CONTENT_SETTINGS_TYPE_USB_CHOOSER_DATA,
                          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) {
   TestingProfile profile;
   PrefService* prefs = profile.GetPrefs();
 
   // Set utf-8 data.
   {
     DictionaryPrefUpdate update(prefs,
                                 GetPrefName(CONTENT_SETTINGS_TYPE_PLUGINS));
@@ skipping 398 matching lines @@
                 host, host, CONTENT_SETTINGS_TYPE_KEYGEN, std::string()));
 
   // After migrating old settings, changes to the setting works.
   host_content_settings_map->SetContentSettingDefaultScope(
       host, GURL(), CONTENT_SETTINGS_TYPE_KEYGEN, std::string(),
       CONTENT_SETTING_BLOCK);
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
                 host, host, CONTENT_SETTINGS_TYPE_KEYGEN, std::string()));
 }