[subresource_filter] Swap ALLOW and BLOCK meanings in the content setting

components/content_settings/core/browser/content_settings_registry.cc

 // Copyright 2015 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_registry.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/macros.h"
@@ skipping 257 matching lines @@
 
   Register(CONTENT_SETTINGS_TYPE_AUTOPLAY, "autoplay", CONTENT_SETTING_ALLOW,
            WebsiteSettingsInfo::UNSYNCABLE, WhitelistedSchemes(),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
            WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
            WebsiteSettingsRegistry::DESKTOP |
                WebsiteSettingsRegistry::PLATFORM_ANDROID,
            ContentSettingsInfo::INHERIT_IF_LESS_PERMISSIVE);
 
   Register(CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER, "subresource-filter",
-           CONTENT_SETTING_ALLOW,
-           WebsiteSettingsInfo::UNSYNCABLE, WhitelistedSchemes(),
+           CONTENT_SETTING_BLOCK, WebsiteSettingsInfo::UNSYNCABLE,

[engedy, 2017/05/11 10:39:37]

Changing the default value for a content setting is safe and supported, right?

[Charlie Harrison, 2017/05/11 14:05:44]

AFAICT this is safe since default values are not persisted on disk (anywhere I
can see).

raymes: can you confirm?

[msramek, 2017/05/11 14:29:20]

The default provider currently doesn't distinguish between users who had ALLOW
because they never touched it, and those who had ALLOW explicitly (i.e. by
switching to BLOCK and back).

So after this change, everyone who had ALLOW will have BLOCK, and everyone who
had BLOCK will also have BLOCK.

But it won't crash anything.

If you care about users who might have already started using this, you should
add a migration code to PrefProvider (default BLOCK->ALLOW), and PrefProvider
(mirror all exceptions).

[Charlie Harrison, 2017/05/11 15:50:11]

Thanks for the clarification. As discussed offline, we probably don't need a
migration path since this isn't being used outside of our own dev builds.

+           WhitelistedSchemes(),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
            WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
            WebsiteSettingsRegistry::DESKTOP |
                WebsiteSettingsRegistry::PLATFORM_ANDROID,
            ContentSettingsInfo::INHERIT_IF_LESS_PERMISSIVE);
 
   // Content settings that aren't used to store any data. TODO(raymes): use a
   // different mechanism rather than content settings to represent these.
   // Since nothing is stored in them, there is no real point in them being a
   // content setting.
@@ skipping 46 matching lines @@
   if (!website_settings_info)
     return;
 
   DCHECK(!base::ContainsKey(content_settings_info_, type));
   content_settings_info_[type] = base::MakeUnique<ContentSettingsInfo>(
       website_settings_info, whitelisted_schemes, valid_settings,
       incognito_behavior);
 }
 
 }  // namespace content_settings