Add back a DCHECK on threading behavior in HostContentSettingsMap.

HostContentSettingsMap::RegisterExtensionService is the final piece of
initialization in this class, after which content_settings_providers_
is immutable, and so can be used from multiple threads without
locking. This assert verifies that the member is only accessed from a
single thread before initialization completes. The previous assert
(documented in the bug) checked that it was never read before
initialization, which was incorrect, it does get read but only by the
same thread that constructs and initializes it.

BUG=176315

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

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

2 files changed, 19 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 14c251d..8749d6f 100644
--- a/chrome/browser/content_settings/host_content_settings_map.cc
+++ b/chrome/browser/content_settings/host_content_settings_map.cc
@@ -79,7 +79,7 @@ HostContentSettingsMap::HostContentSettingsMap(
     PrefService* prefs,
     bool incognito) :
 #ifndef NDEBUG
-      used_content_settings_providers_(false),
+      used_from_thread_id_(base::PlatformThread::CurrentId()),
 #endif
       prefs_(prefs),
       is_off_the_record_(incognito) {
@@ -108,10 +108,6 @@ HostContentSettingsMap::HostContentSettingsMap(
 void HostContentSettingsMap::RegisterExtensionService(
     ExtensionService* extension_service) {
   DCHECK(extension_service);
-  // http://crbug.com/176315
-  // #ifndef NDEBUG
-  //   DCHECK(!used_content_settings_providers_);
-  // #endif
   DCHECK(!content_settings_providers_[INTERNAL_EXTENSION_PROVIDER]);
   DCHECK(!content_settings_providers_[CUSTOM_EXTENSION_PROVIDER]);
 
@@ -129,6 +125,11 @@ void HostContentSettingsMap::RegisterExtensionService(
   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,
@@ -512,7 +513,11 @@ void HostContentSettingsMap::AddSettingsForOneType(
 
 void HostContentSettingsMap::UsedContentSettingsProviders() const {
 #ifndef NDEBUG
-  used_content_settings_providers_ = true;
+  if (used_from_thread_id_ == base::kInvalidThreadId)
+    return;
+
+  if (base::PlatformThread::CurrentId() != used_from_thread_id_)
+    used_from_thread_id_ = base::kInvalidThreadId;
 #endif
 }
 
diff --git a/chrome/browser/content_settings/host_content_settings_map.h b/chrome/browser/content_settings/host_content_settings_map.h
index 46f62ff..1f1637f 100644
--- a/chrome/browser/content_settings/host_content_settings_map.h
+++ b/chrome/browser/content_settings/host_content_settings_map.h
@@ -15,6 +15,7 @@
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/public/pref_change_registrar.h"
+#include "base/threading/platform_thread.h"
 #include "base/tuple.h"
 #include "chrome/browser/content_settings/content_settings_observer.h"
 #include "chrome/common/content_settings.h"
@@ -236,7 +237,13 @@ class HostContentSettingsMap
   void UsedContentSettingsProviders() const;
 
 #ifndef NDEBUG
-  mutable bool used_content_settings_providers_;
+  // This starts as the thread ID of the thread that constructs this
+  // object, and remains until used by a different thread, at which
+  // point it is set to base::kInvalidThreadId. This allows us to
+  // DCHECK on unsafe usage of content_settings_providers_ (they
+  // should be set up on a single thread, after which they are
+  // immutable).
+  mutable base::PlatformThreadId used_from_thread_id_;
 #endif
 
   // Weak; owned by the Profile.