Shield Settings are not retained across sessions

[srirambv]

Description

Global Shield Settings are not retained across sessions

Steps to Reproduce

1. Clean profile on Beta/Dev
2. Change Default Global Shields settings for Adblocker/Cookie/Fingerprint
3. Close browser window and relaunch, all settings reverts back to default values

Actual result:

Global Shield Settings are not retained across sessions

Expected result:

Global shield settings should be retained if changed from default value

Reproduces how often:

Easy

Brave version (chrome://version info)

Brave	0.55.10 Chromium: 70.0.3538.22 (Official Build) beta (64-bit)
Revision	ac9418ba9c3bd7f6baaffa0b055dfe147e0f8364-refs/branch-heads/3538@{#468}
OS	Windows

Reproducible on current release:

No. B-l retains the settings once its changed

Website problems only:

• Does the issue resolve itself when disabling Brave Shields?
• Is the issue reproducible on the latest version of Chrome?

Additional Information

Multiple reports on community
cc: @simonhong @bbondy

[bbondy]

This is also reproducing with just Brave shield settings. I think it regressed with brave/brave-core@28e1ef0

[yrliou]

Thought they're not needed since we don't use ContentSettingsStore anymore, I'll check it.

[simonhong]

Should we consider similar fixes done by BraveContentSettingsStore to HostContentSettingsMap?
default shields and brave shields api all uses HostContentSettingsMap instead of ContentSettingsStore to store configuration.

[simonhong]

Only script setting is persisted.

[bbondy]

Should we consider similar fixes done by BraveContentSettingsStore to HostContentSettingsMap?

yes.

[simonhong]

Below test is failed. It seems plugin type isn't persisted.
However, javascript type is persisted.
Curious this is upstream bug or intended behavior.
Need more debugging...

IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
                       PRE_ShieldSettingsPersistTest) {
  HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
      SetContentSettingCustomScope(
        ContentSettingsPattern::Wildcard(),
        ContentSettingsPattern::Wildcard(),
        CONTENT_SETTINGS_TYPE_PLUGINS,
        brave_shields::kHTTPUpgradableResources,
        CONTENT_SETTING_ALLOW);

  ContentSetting setting =
      HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
          GetContentSetting(GURL(), GURL(), CONTENT_SETTINGS_TYPE_PLUGINS,
                            brave_shields::kHTTPUpgradableResources);
  EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}

IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
                       ShieldSettingsPersistTest) {
  ContentSetting setting =
      HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
          GetContentSetting(GURL(), GURL(), CONTENT_SETTINGS_TYPE_PLUGINS,
                            brave_shields::kHTTPUpgradableResources);
  EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}

[simonhong]

plugin type isn't persisted by default.
Chromium determines this configuration by their plugin strategy.
When PERSISTENT is used, this issue is fixed.
However, it changes flash permission to persistent.
Can we change this from EPHEMERAL to PERSISTENT or should we need more finer control? @bbondy

  Register(
      CONTENT_SETTINGS_TYPE_PLUGINS, "plugins",
      CONTENT_SETTING_DETECT_IMPORTANT_CONTENT, WebsiteSettingsInfo::SYNCABLE,
      WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
      ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
                    CONTENT_SETTING_ASK,
                    CONTENT_SETTING_DETECT_IMPORTANT_CONTENT),
      WebsiteSettingsInfo::SINGLE_ORIGIN_WITH_EMBEDDED_EXCEPTIONS_SCOPE,
      WebsiteSettingsRegistry::DESKTOP,
      ContentSettingsInfo::INHERIT_IF_LESS_PERMISSIVE,
      base::FeatureList::IsEnabled(features::kEnableEphemeralFlashPermission)
          ? ContentSettingsInfo::EPHEMERAL
          : ContentSettingsInfo::PERSISTENT);

[bbondy]

We want to persist except for the case with Flash, we don't want this to regress:
#241

[srirambv]

Cookie and Fingerprint settings are not retained across session. Logged #1524. Needs re-verification after #1524 is fixed

[srirambv]

Verification Passed on

Brave	0.55.17 Chromium: 70.0.3538.67 (Official Build) (64-bit)
Revision	9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS	Linux

• Verified Fingerprint settings are retained on each startup

Verified passed on

Brave	0.55.17 Chromium: 70.0.3538.67 (Official Build) (64-bit)
Revision	9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS	Mac OS X

• Verified Fingerprint settings are retained on each startup

Verification Passed on

Brave	0.55.17 Chromium: 70.0.3538.67 (Official Build) (32-bit)
Revision	9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS	Windows

• Verified Fingerprint settings are retained on each startup

[markwylde]

Is it possible this has regressed?

The following steps occur in Brave "Version 0.64.76 Chromium: 74.0.3729.157 (Official Build) (64-bit)"

1. Navigate to "brave://settings/"
2. Scroll down to "Brave shields defaults"
3. See "Fingerprinting protection" is set to "Block 3rd party fingerprinting"
4. Change "Fingerprinting protection" to "Block all fingerprinting"
5. Restart Brave
6. Navigate to "brave://settings/"
7. Scroll down to "Brave shields defaults"
8. [UNEXPECTED] See "Fingerprinting protection" is set to "Block 3rd party fingerprinting"

Step 8 should have been set to "Block all fingerprinting"