Profiles: Really fix refcounted services.

chrome/browser/profiles/profile_keyed_service_factory.h

Index: chrome/browser/profiles/profile_keyed_service_factory.h
diff --git a/chrome/browser/profiles/profile_keyed_service_factory.h b/chrome/browser/profiles/profile_keyed_service_factory.h
index 16c39ade9f431ab1e94aa07e1ce36305f881ad0c..6678a3440d3988d16504d4e494ae50af899a11d7 100644
--- a/chrome/browser/profiles/profile_keyed_service_factory.h
+++ b/chrome/browser/profiles/profile_keyed_service_factory.h
@@ -8,6 +8,7 @@
 #include <map>
 
 #include "base/compiler_specific.h"
+#include "base/basictypes.h"

[Miranda Callahan, 2012/03/15 15:31:07]

nit: b before c except after e. actually, also after e. 

 #include "chrome/browser/profiles/profile_keyed_base_factory.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 
@@ -24,6 +25,26 @@ class ProfileKeyedService;
 // shutdown/destruction order. In each derived classes' constructors, the
 // implementors must explicitly state which services are depended on.
 class ProfileKeyedServiceFactory : public ProfileKeyedBaseFactory {
+ 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.
+  typedef ProfileKeyedService* (*FactoryFunction)(Profile* profile);

[Miranda Callahan, 2012/03/15 15:31:07]

Shouldn't FactoryFunction be described as "A function that takes a Profile* and
returns the PKS associated with that Profile"? I'm not sure what you mean by
"replaces the (possibly internal) object used by this factory" wrt this function
definition.

+
+  // Associates |factory| with |profile| so that |factory| is used to create
+  // the ProfileKeyedService when requested.
+  //

[Miranda Callahan, 2012/03/15 15:31:07]

nit: remove blank line.

+  // |factory| can be NULL to signal that ProfileKeyedService should be NULL. A
+  // second call to SetTestingFactory() is allowed. If the FactoryFunction is

[Miranda Callahan, 2012/03/15 15:31:07]

instead of "a second call", maybe "multiple calls ... are allowed." 

+  // changed AND an instance of the PKSF already exists for |profile|, that
+  // service is destroyed.
+  void SetTestingFactory(Profile* profile, FactoryFunction factory);
+
+  // Associates |factory| with |profile| and immediately returns the created
+  // ProfileKeyedService. Since the factory will be used immediately, it may
+  // not be NULL;

[Miranda Callahan, 2012/03/15 15:31:07]

nit: s/;/.

+  ProfileKeyedService* SetTestingFactoryAndUse(Profile* profile,
+                                               FactoryFunction factory);
+
  protected:
   // ProfileKeyedServiceFactories must communicate with a
   // ProfileDependencyManager. For all non-test code, write your subclass
@@ -45,18 +66,14 @@ class ProfileKeyedServiceFactory : public ProfileKeyedBaseFactory {
   // BuildServiceInstanceFor() if it doesn't already exist.
   ProfileKeyedService* GetServiceForProfile(Profile* profile, bool create);
 
+  // Maps |profile| to |provider| with debug checks to prevent duplication.

[Miranda Callahan, 2012/03/15 15:31:07]

s/|provider|/|service|/

+  void Associate(Profile* profile, ProfileKeyedService* service);
+
   // All subclasses of ProfileKeyedServiceFactory must return a
   // ProfileKeyedService instead of just a ProfileKeyedBase.
   virtual ProfileKeyedService* BuildServiceInstanceFor(
       Profile* profile) const = 0;
 
-  // Maps |profile| to |provider| with debug checks to prevent duplication.
-  virtual void Associate(Profile* profile, ProfileKeyedBase* base) OVERRIDE;
-
-  // Returns the previously associated |base| for |profile|, or NULL.
-  virtual bool GetAssociation(Profile* profile,
-                              ProfileKeyedBase** out) const OVERRIDE;
-
   // A helper object actually listens for notifications about Profile
   // destruction, calculates the order in which things are destroyed and then
   // does a two pass shutdown.
@@ -71,12 +88,20 @@ class ProfileKeyedServiceFactory : public ProfileKeyedBaseFactory {
   virtual void ProfileShutdown(Profile* profile) OVERRIDE;
   virtual void ProfileDestroyed(Profile* profile) OVERRIDE;
 
+  virtual void SetEmptyTestingFactory(Profile* profile) OVERRIDE;
+  virtual void CreateServiceNow(Profile* profile) OVERRIDE;
+
  private:
   friend class ProfileDependencyManager;
   friend class ProfileDependencyManagerUnittests;
 
   // The mapping between a Profile and its service.
   std::map<Profile*, ProfileKeyedService*> mapping_;
+
+  // The mapping between a Profile and its overridden FactoryFunction.
+  std::map<Profile*, FactoryFunction> factories_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProfileKeyedServiceFactory);
 };
 
 #endif  // CHROME_BROWSER_PROFILES_PROFILE_KEYED_SERVICE_FACTORY_H_