Profiles: Mark the factories as NonThreadSafe and allow services to be deleted on certain threads.

BUG=116892, 116887


Review URL: http://codereview.chromium.org/9617023

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

5 files changed, 79 insertions, 4 deletions

diff --git a/chrome/browser/profiles/profile_keyed_base_factory.cc b/chrome/browser/profiles/profile_keyed_base_factory.cc
index 61087e5..1b63cb9 100644
--- a/chrome/browser/profiles/profile_keyed_base_factory.cc
+++ b/chrome/browser/profiles/profile_keyed_base_factory.cc
@@ -50,7 +50,6 @@ ProfileKeyedBaseFactory::~ProfileKeyedBaseFactory() {
   dependency_manager_->RemoveComponent(this);
 }
 
-
 void ProfileKeyedBaseFactory::DependsOn(ProfileKeyedBaseFactory* rhs) {
   dependency_manager_->AddEdge(rhs, this);
 }
@@ -109,6 +108,8 @@ bool ProfileKeyedBaseFactory::ServiceIsNULLWhileTesting() {
 ProfileKeyedBase* ProfileKeyedBaseFactory::GetBaseForProfile(
     Profile* profile,
     bool create) {
+  DCHECK(CalledOnValidThread());
+
 #ifndef NDEBUG
   dependency_manager_->AssertProfileWasntDestroyed(profile);
 #endif
@@ -157,6 +158,10 @@ ProfileKeyedBase* ProfileKeyedBaseFactory::GetBaseForProfile(
 }
 
 void ProfileKeyedBaseFactory::ProfileDestroyed(Profile* profile) {
+  // While object destruction can be customized in ways where the object is
+  // only dereferenced, this still must run on the UI thread.
+  DCHECK(CalledOnValidThread());
+
   // For unit tests, we also remove the factory function both so we don't
   // maintain a big map of dead pointers, but also since we may have a second
   // object that lives at the same address (see other comments about unit tests
diff --git a/chrome/browser/profiles/profile_keyed_base_factory.h b/chrome/browser/profiles/profile_keyed_base_factory.h
index 846b396..efd1edd 100644
--- a/chrome/browser/profiles/profile_keyed_base_factory.h
+++ b/chrome/browser/profiles/profile_keyed_base_factory.h
@@ -8,6 +8,8 @@
 #include <map>
 #include <set>
 
+#include "base/threading/non_thread_safe.h"
+
 class PrefService;
 class Profile;
 class ProfileKeyedBase;
@@ -20,7 +22,7 @@ class ProfileDependencyManager;
 // RefcountedProfileKeyedServiceFactory. This object describes general profile
 // dependency management; subclasses react to lifecycle events and implement
 // memory management.
-class ProfileKeyedBaseFactory {
+class ProfileKeyedBaseFactory : public base::NonThreadSafe {
  public:
   // A function that replaces the (possibly internal) object used by this
   // factory. For the majority of cases, this is the object returned to users.
diff --git a/chrome/browser/profiles/refcounted_profile_keyed_service.cc b/chrome/browser/profiles/refcounted_profile_keyed_service.cc
index b9e757c..46b8d96 100644
--- a/chrome/browser/profiles/refcounted_profile_keyed_service.cc
+++ b/chrome/browser/profiles/refcounted_profile_keyed_service.cc
@@ -4,5 +4,34 @@
 
 #include "chrome/browser/profiles/refcounted_profile_keyed_service.h"
 
+namespace impl {
+
+// static
+void RefcountedProfileKeyedServiceTraits::Destruct(
+    const RefcountedProfileKeyedService* obj) {
+  if (obj->requires_destruction_on_thread_) {
+    if (content::BrowserThread::CurrentlyOn(obj->thread_id_)) {
+      delete obj;
+    } else {
+      content::BrowserThread::DeleteSoon(obj->thread_id_, FROM_HERE, obj);
+    }
+  } else {
+    delete obj;
+  }
+}
+
+}  // namespace impl
+
+RefcountedProfileKeyedService::RefcountedProfileKeyedService()
+    : requires_destruction_on_thread_(false),
+      thread_id_(content::BrowserThread::UI) {
+}
+
+RefcountedProfileKeyedService::RefcountedProfileKeyedService(
+    const content::BrowserThread::ID thread_id)
+    : requires_destruction_on_thread_(true),
+      thread_id_(thread_id) {
+}
+
 RefcountedProfileKeyedService::~RefcountedProfileKeyedService() {}
 
diff --git a/chrome/browser/profiles/refcounted_profile_keyed_service.h b/chrome/browser/profiles/refcounted_profile_keyed_service.h
index 562e113..c010118 100644
--- a/chrome/browser/profiles/refcounted_profile_keyed_service.h
+++ b/chrome/browser/profiles/refcounted_profile_keyed_service.h
@@ -7,6 +7,17 @@
 
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/profiles/profile_keyed_base.h"
+#include "content/public/browser/browser_thread.h"
+
+class RefcountedProfileKeyedService;
+
+namespace impl {
+
+struct RefcountedProfileKeyedServiceTraits {
+  static void Destruct(const RefcountedProfileKeyedService* obj);
+};
+
+}  // namespace impl
 
 // Base class for refcounted objects that hang off the Profile.
 //
@@ -15,9 +26,16 @@
 // threads. ShutdownOnUIThread() will be called on the UI thread, and then the
 // destructor will run when the last reference is dropped, which may or may not
 // be after the corresponding Profile has been destroyed.
+//
+// Optionally, if you initialize your service with the constructor that takes a
+// thread ID, your service will be deleted on that thread. We can't use
+// content::DeleteOnThread<> directly because RefcountedProfileKeyedService
+// must be one type that RefcountedProfileKeyedServiceFactory can use.
 class RefcountedProfileKeyedService
     : public ProfileKeyedBase,
-      public base::RefCountedThreadSafe<RefcountedProfileKeyedService> {
+      public base::RefCountedThreadSafe<
+          RefcountedProfileKeyedService,
+          impl::RefcountedProfileKeyedServiceTraits> {
  public:
   // Unlike ProfileKeyedService, ShutdownOnUI is not optional. You must do
   // something to drop references during the first pass Shutdown() because this
@@ -26,8 +44,26 @@ class RefcountedProfileKeyedService
   // the UI thread; you do not need to check for that yourself.
   virtual void ShutdownOnUIThread() = 0;
 
-  // This second pass can happen anywhere.
+  // The second pass destruction can happen anywhere unless you specify which
+  // thread this service must be destroyed on by using the second constructor.
   virtual ~RefcountedProfileKeyedService();
+
+ protected:
+  // If you want your service deleted wherever, use the default constructor.
+  RefcountedProfileKeyedService();
+
+  // If you need your service to be deleted on a specific thread (for example,
+  // you're converting a service that used content::DeleteOnThread<IO>), then
+  // use this constructor with the ID of the thread.
+  explicit RefcountedProfileKeyedService(
+      const content::BrowserThread::ID thread_id);
+
+ private:
+  friend struct impl::RefcountedProfileKeyedServiceTraits;
+
+  // Do we have to delete this object on a specific thread?
+  bool requires_destruction_on_thread_;
+  content::BrowserThread::ID thread_id_;
 };
 
 #endif  // CHROME_BROWSER_PROFILES_REFCOUNTED_PROFILE_KEYED_SERVICE_H_
diff --git a/chrome/browser/profiles/refcounted_profile_keyed_service_factory.h b/chrome/browser/profiles/refcounted_profile_keyed_service_factory.h
index 93e1d47..4e9c1b5 100644
--- a/chrome/browser/profiles/refcounted_profile_keyed_service_factory.h
+++ b/chrome/browser/profiles/refcounted_profile_keyed_service_factory.h
@@ -17,6 +17,9 @@ class RefcountedProfileKeyedService;
 // A specialized ProfileKeyedServiceFactory that manages a
 // RefcountedThreadSafe<>.
 //
+// While the factory returns RefcountedThreadSafe<>s, the factory itself is a
+// base::NotThreadSafe. Only call methods on this object on the UI thread.
+//
 // Implementers of RefcountedProfileKeyedService should note that we guarantee
 // that ShutdownOnUIThread() is called on the UI thread, but actual object
 // destruction can happen anywhere.