Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make shields settings persist #558

Merged
merged 4 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions browser/extensions/api/brave_shields_api_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "brave/browser/extensions/api/brave_shields_api.h"
#include "brave/common/brave_paths.h"
#include "brave/common/extensions/extension_constants.h"
#include "brave/components/brave_shields/common/brave_shield_constants.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/extensions/api/content_settings/content_settings_api.h"
#include "chrome/browser/extensions/api/content_settings/content_settings_api_constants.h"
Expand Down Expand Up @@ -268,4 +269,56 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
content_settings::SettingSource::SETTING_SOURCE_USER);
}

// Checks shields configuration is persisted across the sessions.
IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
PRE_ShieldSettingsPersistTest) {
HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
SetContentSettingDefaultScope(
kBraveURL, kBraveURL,
CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kHTTPUpgradableResources,
CONTENT_SETTING_ALLOW);

ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
GetContentSetting(kBraveURL, kBraveURL, 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(kBraveURL, kBraveURL, CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kHTTPUpgradableResources);
EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}

// Checks flash configuration isn't persisted across the sessions.
IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
PRE_FlashPersistTest) {
HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
SetContentSettingDefaultScope(
kBraveURL, kBraveURL,
CONTENT_SETTINGS_TYPE_PLUGINS,
std::string(),
CONTENT_SETTING_ALLOW);

ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
GetContentSetting(kBraveURL, kBraveURL, CONTENT_SETTINGS_TYPE_PLUGINS,
std::string());
EXPECT_EQ(setting, CONTENT_SETTING_ALLOW);
}

IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest,
FlashPersistTest) {
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(browser()->profile())->
GetContentSetting(kBraveURL, kBraveURL, CONTENT_SETTINGS_TYPE_PLUGINS,
std::string());
EXPECT_EQ(setting, CONTENT_SETTING_BLOCK);
}

} // namespace extensions
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.h"
#include "brave/components/content_settings/core/browser/brave_content_settings_pref_provider.h"

#define EphemeralProvider BraveEphemeralProvider
#define PrefProvider BravePrefProvider
#include "../../../../../../components/content_settings/core/browser/host_content_settings_map.cc"
#undef EphemeralProvider
#undef PrefProvider
4 changes: 4 additions & 0 deletions components/content_settings/core/browser/BUILD.gn
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
source_set("browser") {
sources = [
"brave_content_settings_ephemeral_provider.cc",
"brave_content_settings_ephemeral_provider.h",
"brave_content_settings_pref_provider.cc",
"brave_content_settings_pref_provider.h",
"brave_cookie_settings.cc",
"brave_cookie_settings.h",
"brave_host_content_settings_map.cc",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.h"

#include "brave/components/brave_shields/common/brave_shield_constants.h"

namespace {

bool IsShieldsResourceID(
const content_settings::ResourceIdentifier& resource_identifier) {
if (resource_identifier == brave_shields::kAds ||
resource_identifier == brave_shields::kTrackers ||
resource_identifier == brave_shields::kHTTPUpgradableResources ||
resource_identifier == brave_shields::kJavaScript ||
resource_identifier == brave_shields::kFingerprinting ||
resource_identifier == brave_shields::kBraveShields ||
resource_identifier == brave_shields::kReferrers ||
resource_identifier == brave_shields::kCookies) {
return true;
} else {
return false;
}
}

}

namespace content_settings {

bool BraveEphemeralProvider::SetWebsiteSetting(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern,
ContentSettingsType content_type,
const ResourceIdentifier& resource_identifier,
base::Value* in_value) {
// Prevent this handle shields configuration.
if (content_type == CONTENT_SETTINGS_TYPE_PLUGINS &&
IsShieldsResourceID(resource_identifier)) {
return false;
}

Copy link
Member

@bbondy bbondy Oct 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have a DCHECK here to ensure resource_identifier is empty?
I think that will prevent a future bug when we add a new content setting type and forget to add it in the list of IsShieldsResourceID

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// Only flash plugin setting can be reached here.
DCHECK(resource_identifier.empty());

return EphemeralProvider::SetWebsiteSetting(
primary_pattern, secondary_pattern,
content_type, resource_identifier, in_value);
}

} // namespace content_settings
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_CONTENT_SETTINGS_EPHEMERAL_PROVIDER_H_
#define BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_CONTENT_SETTINGS_EPHEMERAL_PROVIDER_H_

#include "components/content_settings/core/browser/content_settings_ephemeral_provider.h"

namespace content_settings {

// See the comments of BravePrefProvider.
class BraveEphemeralProvider : public EphemeralProvider {
public:
using EphemeralProvider::EphemeralProvider;
~BraveEphemeralProvider() override {}

private:
// EphemeralProvider overrides:
bool SetWebsiteSetting(const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern,
ContentSettingsType content_type,
const ResourceIdentifier& resource_identifier,
base::Value* value) override;

DISALLOW_COPY_AND_ASSIGN(BraveEphemeralProvider);
};

} // namespace content_settings

#endif // BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_CONTENT_SETTINGS_EPHEMERAL_PROVIDER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/content_settings/core/browser/brave_content_settings_pref_provider.h"

#include "base/bind.h"
#include "components/content_settings/core/browser/content_settings_pref.h"
#include "components/content_settings/core/browser/website_settings_registry.h"

namespace content_settings {

BravePrefProvider::BravePrefProvider(PrefService* prefs,
bool incognito,
bool store_last_modified)
: PrefProvider(prefs, incognito, store_last_modified) {
brave_pref_change_registrar_.Init(prefs_);

WebsiteSettingsRegistry* website_settings =
WebsiteSettingsRegistry::GetInstance();
// Makes BravePrefProvder handle plugin type.
for (const WebsiteSettingsInfo* info : *website_settings) {
if (info->type() == CONTENT_SETTINGS_TYPE_PLUGINS) {
content_settings_prefs_.insert(std::make_pair(
info->type(),
std::make_unique<ContentSettingsPref>(
info->type(), prefs_, &brave_pref_change_registrar_,
info->pref_name(),
is_incognito_,
base::Bind(&PrefProvider::Notify, base::Unretained(this)))));
return;
}
}
}

void BravePrefProvider::ShutdownOnUIThread() {
brave_pref_change_registrar_.RemoveAll();
PrefProvider::ShutdownOnUIThread();
}

bool BravePrefProvider::SetWebsiteSetting(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern,
ContentSettingsType content_type,
const ResourceIdentifier& resource_identifier,
base::Value* in_value) {
// Flash's setting shouldn't be reached here.
// Its content type is plugin and id is empty string.
// One excpetion is default setting. It can be persisted.
if (content_type == CONTENT_SETTINGS_TYPE_PLUGINS &&
resource_identifier.empty()) {
DCHECK(primary_pattern == ContentSettingsPattern::Wildcard() &&
secondary_pattern == ContentSettingsPattern::Wildcard());
}

return PrefProvider::SetWebsiteSetting(
primary_pattern, secondary_pattern,
content_type, resource_identifier, in_value);
}

} // namespace content_settings
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_BRAVE_CONTENT_SETTINGS_PREF_PROVIDER_H_
#define BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_BRAVE_CONTENT_SETTINGS_PREF_PROVIDER_H_

#include "components/content_settings/core/browser/content_settings_pref_provider.h"
#include "components/prefs/pref_change_registrar.h"

namespace content_settings {

// With this subclass, shields configuration is persisted across sessions.
// Its content type is |CONTENT_SETTINGS_TYPE_PLUGIN| and its storage option is
// ephemeral because chromium want that flash configuration shouldn't be
// persisted. (Maybe chromium assumes flash is the only one of this type).
// Because of this reasion, shields configuration was also ephemeral.
// However, we want shilelds configuration persisted. To do this, we make
// EphemeralProvider ignore shields type and this class handles.
class BravePrefProvider : public PrefProvider {
public:
BravePrefProvider(
PrefService* prefs, bool incognito, bool store_last_modified);
~BravePrefProvider() override {}

private:
// content_settings::PrefProvider overrides:
void ShutdownOnUIThread() override;
bool SetWebsiteSetting(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern,
ContentSettingsType content_type,
const ResourceIdentifier& resource_identifier,
base::Value* value) override;

// PrefProvider::pref_change_registrar_ alreay has plugin type.
PrefChangeRegistrar brave_pref_change_registrar_;

DISALLOW_COPY_AND_ASSIGN(BravePrefProvider);
};

} // namespace content_settings

#endif // BRAVE_COMPONENTS_CONTENT_SETTINGS_CORE_BROWSER_BRAVE_CONTENT_SETTINGS_PREF_PROVIDER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
diff --git a/components/content_settings/core/browser/content_settings_pref_provider.h b/components/content_settings/core/browser/content_settings_pref_provider.h
index 2dcdfbacda452ca8519f40d9db74d202b76dc425..915591404eb06bc04696d61522c087ea1a95c729 100644
--- a/components/content_settings/core/browser/content_settings_pref_provider.h
+++ b/components/content_settings/core/browser/content_settings_pref_provider.h
@@ -64,6 +64,7 @@ class PrefProvider : public UserModifiableProvider {
void SetClockForTesting(base::Clock* clock);

private:
+ friend class BravePrefProvider;
friend class DeadlockCheckerObserver; // For testing.

void Notify(const ContentSettingsPattern& primary_pattern,