Detach the dependency from host_content_settings_map to extension.
In order to componentize host_content_settings_map, one big obstacle
is the extension system. This CL changes the direction of the
dependency.
old: host_content_settings_map register extension system
new: host_content_settings_map accepts providers which allows
extension system to register itself.
I believe this is a better design.
BUG=384869
[email protected], [email protected], [email protected]
[email protected], [email protected]
TEST=no regression
Review URL: https://codereview.chromium.org/545413002
Cr-Commit-Position: refs/heads/master@{#294877}
diff --git a/chrome/browser/content_settings/host_content_settings_map.cc b/chrome/browser/content_settings/host_content_settings_map.cc
index 40b6fe1b..c632dbcc 100644
--- a/chrome/browser/content_settings/host_content_settings_map.cc
+++ b/chrome/browser/content_settings/host_content_settings_map.cc
@@ -13,39 +13,29 @@
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/clock.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/content_settings/content_settings_custom_extension_provider.h"
#include "chrome/browser/content_settings/content_settings_default_provider.h"
#include "chrome/browser/content_settings/content_settings_details.h"
-#include "chrome/browser/content_settings/content_settings_internal_extension_provider.h"
#include "chrome/browser/content_settings/content_settings_observable_provider.h"
#include "chrome/browser/content_settings/content_settings_policy_provider.h"
#include "chrome/browser/content_settings/content_settings_pref_provider.h"
#include "chrome/browser/content_settings/content_settings_provider.h"
#include "chrome/browser/content_settings/content_settings_rule.h"
#include "chrome/browser/content_settings/content_settings_utils.h"
-#include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_source.h"
-#include "content/public/browser/user_metrics.h"
#include "content/public/common/content_switches.h"
#include "net/base/net_errors.h"
#include "net/base/static_cookie_policy.h"
#include "url/gurl.h"
#if defined(ENABLE_EXTENSIONS)
-#include "chrome/browser/extensions/extension_service.h"
-#include "extensions/browser/extension_prefs.h"
#include "extensions/common/constants.h"
#endif
-using base::UserMetricsAction;
using content::BrowserThread;
namespace {
@@ -111,40 +101,6 @@
}
}
-#if defined(ENABLE_EXTENSIONS)
-void HostContentSettingsMap::RegisterExtensionService(
- ExtensionService* extension_service) {
- DCHECK(extension_service);
- DCHECK(!content_settings_providers_[INTERNAL_EXTENSION_PROVIDER]);
- DCHECK(!content_settings_providers_[CUSTOM_EXTENSION_PROVIDER]);
-
- content_settings::InternalExtensionProvider* internal_extension_provider =
- new content_settings::InternalExtensionProvider(extension_service);
- internal_extension_provider->AddObserver(this);
- content_settings_providers_[INTERNAL_EXTENSION_PROVIDER] =
- internal_extension_provider;
-
- content_settings::ObservableProvider* custom_extension_provider =
- new content_settings::CustomExtensionProvider(
- extensions::ContentSettingsService::Get(
- extension_service->GetBrowserContext())->content_settings_store(),
- is_off_the_record_);
- custom_extension_provider->AddObserver(this);
- content_settings_providers_[CUSTOM_EXTENSION_PROVIDER] =
- custom_extension_provider;
-
-#ifndef NDEBUG
- DCHECK(used_from_thread_id_ != base::kInvalidThreadId)
- << "Used from multiple threads before initialization complete.";
-#endif
-
- OnContentSettingChanged(ContentSettingsPattern(),
- ContentSettingsPattern(),
- CONTENT_SETTINGS_TYPE_DEFAULT,
- std::string());
-}
-#endif
-
// static
void HostContentSettingsMap::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
@@ -163,6 +119,24 @@
content_settings::PolicyProvider::RegisterProfilePrefs(registry);
}
+void HostContentSettingsMap::RegisterProvider(
+ ProviderType type,
+ scoped_ptr<content_settings::ObservableProvider> provider) {
+ DCHECK(!content_settings_providers_[type]);
+ provider->AddObserver(this);
+ content_settings_providers_[type] = provider.release();
+
+#ifndef NDEBUG
+ DCHECK_NE(used_from_thread_id_, base::kInvalidThreadId)
+ << "Used from multiple threads before initialization complete.";
+#endif
+
+ OnContentSettingChanged(ContentSettingsPattern(),
+ ContentSettingsPattern(),
+ CONTENT_SETTINGS_TYPE_DEFAULT,
+ std::string());
+}
+
ContentSetting HostContentSettingsMap::GetDefaultContentSettingFromProvider(
ContentSettingsType content_type,
content_settings::ProviderInterface* provider) const {
diff --git a/chrome/browser/content_settings/host_content_settings_map.h b/chrome/browser/content_settings/host_content_settings_map.h
index 1b05880..09d8ebd 100644
--- a/chrome/browser/content_settings/host_content_settings_map.h
+++ b/chrome/browser/content_settings/host_content_settings_map.h
@@ -33,6 +33,7 @@
}
namespace content_settings {
+class ObservableProvider;
class ProviderInterface;
class PrefProvider;
}
@@ -46,6 +47,9 @@
public base::RefCountedThreadSafe<HostContentSettingsMap> {
public:
enum ProviderType {
+ // EXTENSION names is a layering violation when this class will move to
+ // components.
+ // TODO(mukai): find the solution.
INTERNAL_EXTENSION_PROVIDER = 0,
POLICY_PROVIDER,
CUSTOM_EXTENSION_PROVIDER,
@@ -56,15 +60,13 @@
HostContentSettingsMap(PrefService* prefs, bool incognito);
-#if defined(ENABLE_EXTENSIONS)
- // In some cases, the ExtensionService is not available at the time the
- // HostContentSettingsMap is constructed. In these cases, we register the
- // service once it's available.
- void RegisterExtensionService(ExtensionService* extension_service);
-#endif
-
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+ // Adds a new provider for |type|.
+ void RegisterProvider(
+ ProviderType type,
+ scoped_ptr<content_settings::ObservableProvider> provider);
+
// Returns the default setting for a particular content type. If |provider_id|
// is not NULL, the id of the provider which provided the default setting is
// assigned to it.
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 776e7eb2..7f6e68d 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -18,6 +18,10 @@
#include "base/time/time.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/content_settings/content_settings_custom_extension_provider.h"
+#include "chrome/browser/content_settings/content_settings_internal_extension_provider.h"
+#include "chrome/browser/content_settings/host_content_settings_map.h"
+#include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
#include "chrome/browser/extensions/component_loader.h"
#include "chrome/browser/extensions/crx_installer.h"
#include "chrome/browser/extensions/data_deleter.h"
@@ -1915,6 +1919,22 @@
return delayed_installs_.GetByID(id);
}
+void ExtensionService::RegisterContentSettings(
+ HostContentSettingsMap* host_content_settings_map) {
+ host_content_settings_map->RegisterProvider(
+ HostContentSettingsMap::INTERNAL_EXTENSION_PROVIDER,
+ scoped_ptr<content_settings::ObservableProvider>(
+ new content_settings::InternalExtensionProvider(this)));
+
+ host_content_settings_map->RegisterProvider(
+ HostContentSettingsMap::CUSTOM_EXTENSION_PROVIDER,
+ scoped_ptr<content_settings::ObservableProvider>(
+ new content_settings::CustomExtensionProvider(
+ extensions::ContentSettingsService::Get(
+ profile_)->content_settings_store(),
+ profile_->GetOriginalProfile() != profile_)));
+}
+
void ExtensionService::TrackTerminatedExtension(const Extension* extension) {
// No need to check for duplicates; inserting a duplicate is a no-op.
registry_->AddTerminated(make_scoped_refptr(extension));
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index c239beb..55954a6 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -33,6 +33,7 @@
class ExtensionSyncService;
class GURL;
+class HostContentSettingsMap;
class Profile;
namespace base {
@@ -365,6 +366,10 @@
// notification is desired the calling code is responsible for doing that.
void TerminateExtension(const std::string& extension_id);
+ // Register self and content settings API with the specified map.
+ void RegisterContentSettings(
+ HostContentSettingsMap* host_content_settings_map);
+
// Adds/Removes update observers.
void AddUpdateObserver(extensions::UpdateObserver* observer);
void RemoveUpdateObserver(extensions::UpdateObserver* observer);
diff --git a/chrome/browser/profiles/off_the_record_profile_impl.cc b/chrome/browser/profiles/off_the_record_profile_impl.cc
index 2913ca82..ffbfbb9 100644
--- a/chrome/browser/profiles/off_the_record_profile_impl.cc
+++ b/chrome/browser/profiles/off_the_record_profile_impl.cc
@@ -20,6 +20,7 @@
#include "chrome/browser/download/chrome_download_manager_delegate.h"
#include "chrome/browser/download/download_service.h"
#include "chrome/browser/download/download_service_factory.h"
+#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_special_storage_policy.h"
#include "chrome/browser/io_thread.h"
#include "chrome/browser/net/chrome_url_request_context_getter.h"
@@ -357,8 +358,10 @@
#if defined(ENABLE_EXTENSIONS)
ExtensionService* extension_service =
extensions::ExtensionSystem::Get(this)->extension_service();
- if (extension_service)
- host_content_settings_map_->RegisterExtensionService(extension_service);
+ if (extension_service) {
+ extension_service->RegisterContentSettings(
+ host_content_settings_map_.get());
+ }
#endif
}
return host_content_settings_map_.get();
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index 20cb9d5..db5affd 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -993,8 +993,8 @@
// During tests, when |profile| is an instance of TestingProfile,
// ExtensionSystem might not create an ExtensionService.
if (extensions::ExtensionSystem::Get(profile)->extension_service()) {
- profile->GetHostContentSettingsMap()->RegisterExtensionService(
- extensions::ExtensionSystem::Get(profile)->extension_service());
+ extensions::ExtensionSystem::Get(profile)->extension_service()->
+ RegisterContentSettings(profile->GetHostContentSettingsMap());
}
#endif
#if defined(ENABLE_MANAGED_USERS) && !defined(OS_ANDROID)
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc
index 0bd0568b..5fa8cba 100644
--- a/chrome/test/base/testing_profile.cc
+++ b/chrome/test/base/testing_profile.cc
@@ -19,6 +19,7 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/content_settings/host_content_settings_map.h"
+#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/favicon/chrome_favicon_client_factory.h"
#include "chrome/browser/favicon/favicon_service.h"
#include "chrome/browser/favicon/favicon_service_factory.h"
@@ -811,8 +812,10 @@
#if defined(ENABLE_EXTENSIONS)
ExtensionService* extension_service =
extensions::ExtensionSystem::Get(this)->extension_service();
- if (extension_service)
- host_content_settings_map_->RegisterExtensionService(extension_service);
+ if (extension_service) {
+ extension_service->RegisterContentSettings(
+ host_content_settings_map_.get());
+ }
#endif
}
return host_content_settings_map_.get();
