Refactor (Refcounted)BrowserContextKeyedServiceFactory

components/keyed_service/content/browser_context_keyed_base_factory.cc

Index: components/keyed_service/content/browser_context_keyed_base_factory.cc
diff --git a/components/keyed_service/content/browser_context_keyed_base_factory.cc b/components/keyed_service/content/browser_context_keyed_base_factory.cc
index f2f09b41adcf2f59f6a695330223d8ed0d8f3ff3..726c8efec551aeda44ccc6ca7131bc755e14526e 100644
--- a/components/keyed_service/content/browser_context_keyed_base_factory.cc
+++ b/components/keyed_service/content/browser_context_keyed_base_factory.cc
@@ -56,31 +56,6 @@ void BrowserContextKeyedBaseFactory::BrowserContextDestroyed(
 user_prefs::PrefRegistrySyncable*
 BrowserContextKeyedBaseFactory::GetAssociatedPrefRegistry(
     base::SupportsUserData* context) const {
-  // Safe timing for pref registration is hard. Previously, we made
-  // BrowserContext responsible for all pref registration on every service
-  // that used BrowserContext. Now we don't and there are timing issues.
-  //
-  // With normal contexts, prefs can simply be registered at
-  // BrowserContextDependencyManager::RegisterProfilePrefsForServices time.
-  // With incognito contexts, we just never register since incognito contexts
-  // share the same pref services with their parent contexts.
-  //
-  // TestingBrowserContexts throw a wrench into the mix, in that some tests will
-  // swap out the PrefService after we've registered user prefs on the original
-  // PrefService. Test code that does this is responsible for either manually
-  // invoking RegisterProfilePrefs() on the appropriate
-  // BrowserContextKeyedServiceFactory associated with the prefs they need,
-  // or they can use SetTestingFactory() and create a service (since service
-  // creation with a factory method causes registration to happen at
-  // TestingProfile creation time).
-  //
-  // Now that services are responsible for declaring their preferences, we have
-  // to enforce a uniquenes check here because some tests create one context and
-  // multiple services of the same type attached to that context (serially, not
-  // parallel) and we don't want to register multiple times on the same context.
-  // This is the purpose of RegisterProfilePrefsIfNecessary() which could be
-  // replaced directly by RegisterProfilePrefs() if this method is ever phased
-  // out.

[Elliot Glaysher, 2014/10/28 17:46:28]

So, the method with this comment seems to have moved into
browser_context_keyed_service_factory.cc. Is there a reason to duplicate the
implementation in the base here?

[sdefresne, 2014/10/29 08:52:47]

BrowserContextKeyedServiceFactory does not inherit from
BrowserContextKeyedBaseFactory. In fact I'd like to remove
BrowserContextKeyedBaseFactory were it not for the fact that four factories
(SchemaRegistryServiceFactory, ProfilePolicyConnectorFactory,
UserCloudPolicyManagerFactory, UserCloudPolicyManagerFactoryChromeOS) inherits
from BrowserContextKeyedBaseFactory instead of BrowserContextKeyedServiceFactory
(with reference to http://crbug.com/131843 and http://crbug.com/131844).

BrowserContextKeyedServiceFactory cannot inherit from
BrowserContextKeyedBaseFactory (that inherit from KeyedServiceBaseFactory) and
from KeyedServiceFactory (since it also inherit from KeyedServiceBaseFactory).
So for the moment I have to duplicate the method. I removed the comment here
since it is already present in KeyedServiceBaseFactory before the call to the
virtual method GetAssociatedPrefRegistry().

I've added a comment at the top of browser_context_keyed_base_factory.h
indicating that this class should no longer be used and is there only for legacy
reason (and pointed to the two bugs mentioned above).

   PrefService* prefs = user_prefs::UserPrefs::Get(
       static_cast<content::BrowserContext*>(context));
   user_prefs::PrefRegistrySyncable* registry =