Remove unused lock, add verification of intended usage.

There used to be locking in this class, but many small changes had contributed to removing all use of the lock. The lock does not appear to actually be needed, as long as the class continues to be used as it currently is used, with the deferred initialization via RegisterExtensionService always happening before the providers map owned by the object is iterated over.

This change removes the |lock_| member and adds debug-only code that verifies that this is how the class continues to be used.

BUG=none

Review URL: https://codereview.chromium.org/12223052

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181930 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 33 insertions, 7 deletions

diff --git a/chrome/browser/content_settings/host_content_settings_map.cc b/chrome/browser/content_settings/host_content_settings_map.cc
index 421f0c1..50606ad 100644
--- a/chrome/browser/content_settings/host_content_settings_map.cc
+++ b/chrome/browser/content_settings/host_content_settings_map.cc
@@ -77,8 +77,11 @@ bool SupportsResourceIdentifier(ContentSettingsType content_type) {
 
 HostContentSettingsMap::HostContentSettingsMap(
     PrefService* prefs,
-    bool incognito)
-    : prefs_(prefs),
+    bool incognito) :
+#ifndef NDEBUG
+      used_content_settings_providers_(false),
+#endif
+      prefs_(prefs),
       is_off_the_record_(incognito) {
   content_settings::ObservableProvider* policy_provider =
       new content_settings::PolicyProvider(prefs_);
@@ -105,6 +108,9 @@ HostContentSettingsMap::HostContentSettingsMap(
 void HostContentSettingsMap::RegisterExtensionService(
     ExtensionService* extension_service) {
   DCHECK(extension_service);
+#ifndef NDEBUG
+  DCHECK(!used_content_settings_providers_);
+#endif
   DCHECK(!content_settings_providers_[INTERNAL_EXTENSION_PROVIDER]);
   DCHECK(!content_settings_providers_[CUSTOM_EXTENSION_PROVIDER]);
 
@@ -167,6 +173,8 @@ ContentSetting HostContentSettingsMap::GetDefaultContentSettingFromProvider(
 ContentSetting HostContentSettingsMap::GetDefaultContentSetting(
     ContentSettingsType content_type,
     std::string* provider_id) const {
+  UsedContentSettingsProviders();
+
   // Iterate through the list of providers and return the first non-NULL value
   // that matches |primary_url| and |secondary_url|.
   for (ConstProviderIterator provider = content_settings_providers_.begin();
@@ -208,6 +216,7 @@ void HostContentSettingsMap::GetSettingsForOneType(
   DCHECK(SupportsResourceIdentifier(content_type) ||
          resource_identifier.empty());
   DCHECK(settings);
+  UsedContentSettingsProviders();
 
   settings->clear();
   for (ConstProviderIterator provider = content_settings_providers_.begin();
@@ -257,6 +266,8 @@ void HostContentSettingsMap::SetWebsiteSetting(
   DCHECK(IsValueAllowedForType(prefs_, value, content_type));
   DCHECK(SupportsResourceIdentifier(content_type) ||
          resource_identifier.empty());
+  UsedContentSettingsProviders();
+
   for (ProviderIterator provider = content_settings_providers_.begin();
        provider != content_settings_providers_.end();
        ++provider) {
@@ -316,6 +327,7 @@ void HostContentSettingsMap::AddExceptionForURL(
 
 void HostContentSettingsMap::ClearSettingsForOneType(
     ContentSettingsType content_type) {
+  UsedContentSettingsProviders();
   for (ProviderIterator provider = content_settings_providers_.begin();
        provider != content_settings_providers_.end();
        ++provider) {
@@ -497,6 +509,12 @@ void HostContentSettingsMap::AddSettingsForOneType(
   }
 }
 
+void HostContentSettingsMap::UsedContentSettingsProviders() const {
+#ifndef NDEBUG
+  used_content_settings_providers_ = true;
+#endif
+}
+
 bool HostContentSettingsMap::ShouldAllowAllContent(
     const GURL& primary_url,
     const GURL& secondary_url,
diff --git a/chrome/browser/content_settings/host_content_settings_map.h b/chrome/browser/content_settings/host_content_settings_map.h
index 635a3db..46f62ff 100644
--- a/chrome/browser/content_settings/host_content_settings_map.h
+++ b/chrome/browser/content_settings/host_content_settings_map.h
@@ -15,7 +15,6 @@
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/public/pref_change_registrar.h"
-#include "base/synchronization/lock.h"
 #include "base/tuple.h"
 #include "chrome/browser/content_settings/content_settings_observer.h"
 #include "chrome/common/content_settings.h"
@@ -230,18 +229,27 @@ class HostContentSettingsMap
       ContentSettingsForOneType* settings,
       bool incognito) const;
 
+  // Call UsedContentSettingsProviders() whenever you access
+  // content_settings_providers_ (apart from initialization and
+  // teardown), so that we can DCHECK in RegisterExtensionService that
+  // it is not being called too late.
+  void UsedContentSettingsProviders() const;
+
+#ifndef NDEBUG
+  mutable bool used_content_settings_providers_;
+#endif
+
   // Weak; owned by the Profile.
   PrefService* prefs_;
 
   // Whether this settings map is for an OTR session.
   bool is_off_the_record_;
 
-  // Content setting providers.
+  // Content setting providers. This is only modified at construction
+  // time and by RegisterExtensionService, both of which should happen
+  // before any other uses of it.
   ProviderMap content_settings_providers_;
 
-  // Used around accesses to the following objects to guarantee thread safety.
-  mutable base::Lock lock_;
-
   DISALLOW_COPY_AND_ASSIGN(HostContentSettingsMap);
 };