Stop inheriting push notification permissions from regular to incognito

components/content_settings/core/browser/content_settings_default_provider.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 "components/content_settings/core/browser/content_settings_default_provider.h"
 
 #include <string>
 #include <vector>
 
 #include "base/auto_reset.h"
@@ skipping 242 matching lines @@
                   content_type,
                   ResourceIdentifier());
 
   return true;
 }
 
 scoped_ptr<RuleIterator> DefaultProvider::GetRuleIterator(
     ContentSettingsType content_type,
     const ResourceIdentifier& resource_identifier,
     bool incognito) const {
+  // The default provider never has incognito-specific settings.

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

This is wrong, please remove it.

With this change, when HostContentSettingsMap asks DefaultProvider what is the
default setting for incognito, it will respond "I don't know", and
HostContentSettingsMap will use CONTENT_SETTING_DEFAULT.

That is incorrect. The setting set by DefaultProvider (i.e. what you see in the
radio groups in chrome://settings/content) is valid for both the regular and
incognito mode (which is exactly why the incognito parameter is ignored here).

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

I think this is actually correct. GetRuleIterator is only called from three
places:
1. HostContentSettingsMap::GetDefaultContentSettingFromProvider
2. GetContentSettingValueAndPatterns (content_settings_utils.cc)
3. HostContentSettingsMap::AddSettingsForOneType, which is in turn only called
by HostContentSettingsMap::GetSettingsForOneType.

Of these, #1 only ever passes false for |incognito| (so this change has no
effect), and both #2 and #3 either only pass false, or pass true immediately
followed by false. In the latter case, it'll still correctly inherit the default
setting into incognito.

And semantically, this change aligns DefaultProvider with the comment on
ProviderInterface::GetRuleIterator, which says "If |incognito| is true, the
iterator returns only the content settings which are applicable to the incognito
mode and differ from the normal mode. Otherwise, it returns the content settings
for the normal mode."

[msramek, 2015/12/01 13:41:43]

Ahhh! Somehow I have never noticed this; I have always been thinking of this
method as returning the same settings for regular mode and incognito. But for
incognito, it should really only return the incognito-*specific* settings, which
there are none.

This change is a no-op due to the way how we use the iterator, but you're right
that only now is it consistent with the description, so I agree that this is a
positive change :-)

[johnme, 2015/12/02 15:13:16]

Acknowledged.

+  if (incognito)
+    return scoped_ptr<RuleIterator>(new EmptyRuleIterator());
+
   base::AutoLock lock(lock_);
   if (resource_identifier.empty()) {
     auto it(default_settings_.find(content_type));
     if (it != default_settings_.end())
       return scoped_ptr<RuleIterator>(new DefaultRuleIterator(it->second));
     NOTREACHED();
   }
   return scoped_ptr<RuleIterator>(new EmptyRuleIterator());
 }
 
@@ skipping 99 matching lines @@
 }
 
 void DefaultProvider::DiscardObsoletePreferences() {
   prefs_->ClearPref(kObsoleteDefaultContentSettings);
   prefs_->ClearPref(kObsoleteMigratedDefaultContentSettings);
   prefs_->ClearPref(kObsoleteMigratedDefaultMediaStreamSetting);
   prefs_->ClearPref(kObsoleteMetroSwitchToDesktopSetting);
 }
 
 }  // namespace content_settings