Stop inheriting push notification permissions from regular to incognito

components/content_settings/core/browser/host_content_settings_map.cc

Index: components/content_settings/core/browser/host_content_settings_map.cc
diff --git a/components/content_settings/core/browser/host_content_settings_map.cc b/components/content_settings/core/browser/host_content_settings_map.cc
index 3887bb5f009c70b618a978e6969b8fc4723eba76..70c0d052c412da967a69f4a092e8091fef7b833d 100644
--- a/components/content_settings/core/browser/host_content_settings_map.cc
+++ b/components/content_settings/core/browser/host_content_settings_map.cc
@@ -157,6 +157,10 @@ ContentSetting HostContentSettingsMap::GetDefaultContentSetting(
       continue;
     ContentSetting default_setting =
         GetDefaultContentSettingFromProvider(content_type, provider->second);
+    if (is_off_the_record_) {
+      default_setting =
+          CoerceSettingInheritedToIncognito(content_type, default_setting);
+    }
     if (default_setting != CONTENT_SETTING_DEFAULT) {
       if (provider_id)
         *provider_id = kProviderNamesSourceMap[provider->first].provider_name;
@@ -653,16 +657,10 @@ scoped_ptr<base::Value> HostContentSettingsMap::GetWebsiteSettingInternal(
   for (ConstProviderIterator provider = content_settings_providers_.begin();
        provider != content_settings_providers_.end();
        ++provider) {
-
-    scoped_ptr<base::Value> value(
-        content_settings::GetContentSettingValueAndPatterns(provider->second,
-                                                            primary_url,
-                                                            secondary_url,
-                                                            content_type,
-                                                            resource_identifier,
-                                                            is_off_the_record_,
-                                                            primary_pattern,
-                                                            secondary_pattern));
+    scoped_ptr<base::Value> value = GetContentSettingValueAndPatternsInternal(
+        provider->second, primary_url, secondary_url, content_type,
+        resource_identifier, is_off_the_record_, primary_pattern,
+        secondary_pattern);
     if (value) {
       if (info)
         info->source = kProviderNamesSourceMap[provider->first].provider_source;
@@ -677,3 +675,81 @@ scoped_ptr<base::Value> HostContentSettingsMap::GetWebsiteSettingInternal(
   }
   return scoped_ptr<base::Value>();
 }
+
+// static
+scoped_ptr<base::Value>
+HostContentSettingsMap::GetContentSettingValueAndPatternsInternal(
+    const content_settings::ProviderInterface* provider,
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsType content_type,
+    const std::string& resource_identifier,
+    bool include_incognito,
+    ContentSettingsPattern* primary_pattern,
+    ContentSettingsPattern* secondary_pattern) {
+  if (include_incognito) {
+    // Check incognito-only specific settings. It's essential that the
+    // |RuleIterator| gets out of scope before we get a rule iterator for the
+    // normal mode.
+    scoped_ptr<content_settings::RuleIterator> incognito_rule_iterator(
+        provider->GetRuleIterator(content_type, resource_identifier,
+                                  true /* incognito */));
+    scoped_ptr<base::Value> value = GetContentSettingValueAndPatternsInternal(
+        incognito_rule_iterator.get(), primary_url, secondary_url,
+        primary_pattern, secondary_pattern);
+    if (value)
+      return value;
+  }
+  // No settings from the incognito; use the normal mode.
+  scoped_ptr<content_settings::RuleIterator> rule_iterator(
+      provider->GetRuleIterator(content_type, resource_identifier,
+                                false /* incognito */));
+  scoped_ptr<base::Value> value = GetContentSettingValueAndPatternsInternal(
+      rule_iterator.get(), primary_url, secondary_url, primary_pattern,
+      secondary_pattern);
+  if (value && include_incognito) {
+    value = content_settings::ContentSettingToValue(

[raymes, 2015/12/03 01:56:49]

This isn't going to give the right answer for website settings which don't
necessarily have the value of allow/block/etc (|value| here could be anything,
like a string or dictionary). It's going to coerce it into some content setting
value and then convert it back to a base::Value. 

I think you either need to check that the ContentSettingsType is in the
ContentSettingsRegistry before calling Coerce or make Coerce take and return a
base::Value. I'd suggest the latter because it saves an additional map lookup in
ContentSettingsRegistry.

[johnme, 2015/12/03 19:06:40]

Done.

+        CoerceSettingInheritedToIncognito(
+            content_type,
+            content_settings::ValueToContentSetting(value.get())));
+  }
+  return value;
+}
+
+// static
+scoped_ptr<base::Value>
+HostContentSettingsMap::GetContentSettingValueAndPatternsInternal(
+    content_settings::RuleIterator* rule_iterator,
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsPattern* primary_pattern,
+    ContentSettingsPattern* secondary_pattern) {
+  while (rule_iterator->HasNext()) {
+    const content_settings::Rule& rule = rule_iterator->Next();
+    if (rule.primary_pattern.Matches(primary_url) &&
+        rule.secondary_pattern.Matches(secondary_url)) {
+      if (primary_pattern)
+        *primary_pattern = rule.primary_pattern;
+      if (secondary_pattern)
+        *secondary_pattern = rule.secondary_pattern;
+      return make_scoped_ptr(rule.value.get()->DeepCopy());
+    }
+  }
+  return scoped_ptr<base::Value>();
+}
+
+// static
+ContentSetting HostContentSettingsMap::CoerceSettingInheritedToIncognito(

[raymes, 2015/12/03 01:56:49]

This doesn't need to be a static function, it could just be a standalone
function in the anonymous namespace in this file.

[johnme, 2015/12/03 19:06:40]

Done.

+    ContentSettingsType content_type,
+    ContentSetting setting) {
+  if (setting != CONTENT_SETTING_ALLOW)
+    return setting;
+  const content_settings::ContentSettingsInfo* info =
+      content_settings::ContentSettingsRegistry::GetInstance()->Get(
+          content_type);
+  if (info->incognito_behavior() !=
+      content_settings::ContentSettingsInfo::INHERIT_IN_INCOGNITO_EXCEPT_ALLOW)
+    return setting;
+  DCHECK(info->IsSettingValid(CONTENT_SETTING_ASK));
+  return CONTENT_SETTING_ASK;
+}