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..d125dab4fd742e8abcbfe25adf9c37b1538038ed 100644
--- a/components/content_settings/core/browser/host_content_settings_map.cc
+++ b/components/content_settings/core/browser/host_content_settings_map.cc
@@ -23,6 +23,7 @@
 #include "components/content_settings/core/browser/content_settings_registry.h"
 #include "components/content_settings/core/browser/content_settings_rule.h"
 #include "components/content_settings/core/browser/content_settings_utils.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/content_settings_pattern.h"
 #include "components/content_settings/core/common/pref_names.h"
@@ -146,8 +147,16 @@ ContentSetting HostContentSettingsMap::GetDefaultContentSettingFromProvider(
 ContentSetting HostContentSettingsMap::GetDefaultContentSetting(
     ContentSettingsType content_type,
     std::string* provider_id) const {
+  using content_settings::WebsiteSettingsInfo;

[msramek, 2015/11/27 16:30:04]

Nit: Is this allowed? I have seen it in tests and classes, but never inside
methods... In any case, there is just one callsite, so I would inline it
completely.

[raymes, 2015/11/30 00:28:46]

This was a suggestion from Michael. Either way is ok (it's really just style
preference), but I'd tend to side with msramek in this case..

[johnme, 2015/11/30 14:56:58]

Done.

[johnme, 2015/11/30 14:56:58]

Done.

+  using content_settings::WebsiteSettingsRegistry;
   UsedContentSettingsProviders();
 
+  const WebsiteSettingsRegistry* reg = WebsiteSettingsRegistry::GetInstance();
+  bool do_not_inherit_allow =
+      is_off_the_record_ &&
+      reg->Get(content_type)->incognito_behavior()
+          == WebsiteSettingsInfo::INHERIT_IN_INCOGNITO_EXCEPT_ALLOW;
+
   // Iterate through the list of providers and return the first non-NULL value
   // that matches |primary_url| and |secondary_url|.
   for (ConstProviderIterator provider = content_settings_providers_.begin();
@@ -157,6 +166,8 @@ ContentSetting HostContentSettingsMap::GetDefaultContentSetting(
       continue;
     ContentSetting default_setting =
         GetDefaultContentSettingFromProvider(content_type, provider->second);
+    if (do_not_inherit_allow && default_setting == CONTENT_SETTING_ALLOW)
+      default_setting = CONTENT_SETTING_ASK;

[msramek, 2015/11/27 16:30:04]

DCHECK ContentSettingsInfo::IsSettingValid(ASK)

Similarly in ContentSettingsRegistry, DCHECK(...) << "If
INHERIT_IN_INCOGNITO_EXCEPT_ALLOW is set, ASK must be listed as a valid
setting."

Alternatively, we could fall back to the default setting, but that can be ALLOW
as well, so that would be more complicated.

[raymes, 2015/11/30 00:28:46]

+1

[johnme, 2015/11/30 14:56:58]

Done.

[johnme, 2015/11/30 14:56:58]

Done.

     if (default_setting != CONTENT_SETTING_DEFAULT) {
       if (provider_id)
         *provider_id = kProviderNamesSourceMap[provider->first].provider_name;
@@ -654,7 +665,7 @@ scoped_ptr<base::Value> HostContentSettingsMap::GetWebsiteSettingInternal(
        provider != content_settings_providers_.end();
        ++provider) {
 
-    scoped_ptr<base::Value> value(
+    scoped_ptr<base::Value> value =
         content_settings::GetContentSettingValueAndPatterns(provider->second,
                                                             primary_url,
                                                             secondary_url,
@@ -662,7 +673,7 @@ scoped_ptr<base::Value> HostContentSettingsMap::GetWebsiteSettingInternal(
                                                             resource_identifier,
                                                             is_off_the_record_,
                                                             primary_pattern,
-                                                            secondary_pattern));
+                                                            secondary_pattern);
     if (value) {
       if (info)
         info->source = kProviderNamesSourceMap[provider->first].provider_source;