Making structure for ContentSettings and its corresponding strings.

components/content_settings/core/browser/content_settings_utils_unittest.cc

Index: components/content_settings/core/browser/content_settings_utils_unittest.cc
diff --git a/components/content_settings/core/browser/content_settings_utils_unittest.cc b/components/content_settings/core/browser/content_settings_utils_unittest.cc
index f89b45b42f9f4ad3eda913c973a263da655d0eca..6227296db20fb64912e1b970b30892ebd892e8d1 100644
--- a/components/content_settings/core/browser/content_settings_utils_unittest.cc
+++ b/components/content_settings/core/browser/content_settings_utils_unittest.cc
@@ -44,3 +44,33 @@ TEST(ContentSettingsUtilsTest, ParsePatternString) {
   EXPECT_FALSE(pattern_pair.first.IsValid());
   EXPECT_FALSE(pattern_pair.second.IsValid());
 }
+
+TEST(ContentSettingsUtilsTest, ContentSettingsStringMap) {
+  // Checking content_settings::ContentSettingToString() functionality.

[Bernhard Bauer, 2015/10/06 15:37:25]

Nit: Remove this comment, the code tell you pretty much the same thing ☺

[Deepak, 2015/10/07 04:39:43]

Done.

+  EXPECT_EQ("default",

[AKV, 2015/10/06 16:01:38]

If we define constant variable instead of hard coding it would be good, as we
use same string in 2 places;
const char* kContentSettingAllow = "allow"; for individual string

or

we can define an array like as it was in content_settings_helpers.cc and loop
through and EXPECT_EQ with proper indexes.
This will reduce number of LOCs in test code.

[Deepak, 2015/10/07 04:39:43]

Done.

+            content_settings::ContentSettingToString(CONTENT_SETTING_DEFAULT));
+  EXPECT_EQ("allow",
+            content_settings::ContentSettingToString(CONTENT_SETTING_ALLOW));
+  EXPECT_EQ("block",
+            content_settings::ContentSettingToString(CONTENT_SETTING_BLOCK));
+  EXPECT_EQ("ask",
+            content_settings::ContentSettingToString(CONTENT_SETTING_ASK));
+  EXPECT_EQ("session_only", content_settings::ContentSettingToString(
+                                CONTENT_SETTING_SESSION_ONLY));
+  EXPECT_EQ("detect_important_content",

[AKV, 2015/10/06 16:01:38]

Can we add a test case, for failure case by passing either empty string or
invalid string, to double ensure validities ?

[Deepak, 2015/10/07 04:39:43]

We can add negative test case for content_settings::ContentSettingFromString(),
but negative test for 
content_settings::ContentSettingToString() failed due to DCHECK() in
ContentSettingToString().

[Bernhard Bauer, 2015/10/07 09:32:03]

If we remove that DCHECK (see my other comment), we can add a negative test
case.

+            content_settings::ContentSettingToString(
+                CONTENT_SETTING_DETECT_IMPORTANT_CONTENT));
+
+  // Checking content_settings::ContentSettingFromString() functionality.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,

[AKV, 2015/10/06 16:01:38]

ditto

[Deepak, 2015/10/07 04:39:43]

Done.

+            content_settings::ContentSettingFromString("allow"));
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            content_settings::ContentSettingFromString("block"));
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            content_settings::ContentSettingFromString("ask"));
+  EXPECT_EQ(CONTENT_SETTING_SESSION_ONLY,
+            content_settings::ContentSettingFromString("session_only"));
+  EXPECT_EQ(
+      CONTENT_SETTING_DETECT_IMPORTANT_CONTENT,
+      content_settings::ContentSettingFromString("detect_important_content"));
+}