Refactor (Refcounted)BrowserContextKeyedServiceFactory

Refactor (Refcounted)BrowserContextKeyedServiceFactory

Introduce (Refcounted)KeyedServiceFactory classes that deal with the
lifetime management of (Refcounted)KeyedService. Port the classes
(Refcounted)BrowserContextKeyedServiceFactory to inherit from them.

BUG=419366
Committed: https://crrev.com/28acc0e482c0a4b3b852487bf0bcdd339f830e54
Cr-Commit-Position: refs/heads/master@{#302054}

[sdefresne, 2014-10-28 16:22:53]

sdefresne@chromium.org changed reviewers:
+ blundell@chromium.org, erg@chromium.org

[sdefresne, 2014-10-28 16:22:54]

erg: please take a look

blundell: FYI

[Elliot Glaysher, 2014-10-28 17:46:29]

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
File components/keyed_service/content/browser_context_keyed_base_factory.cc
(left):

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
components/keyed_service/content/browser_context_keyed_base_factory.cc:83: //
out.
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?

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
File components/keyed_service/content/browser_context_keyed_service_factory.h
(right):

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
components/keyed_service/content/browser_context_keyed_service_factory.h:99:
virtual bool ServiceIsNULLWhileTesting() const override;
No virtual on override.

[sdefresne, 2014-10-29 08:52:48]

Please take another look.

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
File components/keyed_service/content/browser_context_keyed_base_factory.cc
(left):

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
components/keyed_service/content/browser_context_keyed_base_factory.cc:83: //
out.
On 2014/10/28 17:46:28, Elliot Glaysher wrote:
> 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?

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).

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
File components/keyed_service/content/browser_context_keyed_service_factory.h
(right):

https://codereview.chromium.org/654753010/diff/1/components/keyed_service/con...
components/keyed_service/content/browser_context_keyed_service_factory.h:99:
virtual bool ServiceIsNULLWhileTesting() const override;
On 2014/10/28 17:46:28, Elliot Glaysher wrote:
> No virtual on override.

Done.

[Elliot Glaysher, 2014-10-29 22:43:48]

lgtm

[sdefresne, 2014-10-30 08:11:34]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-10-30 08:12:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/654753010/20001

[commit-bot: I haz the power, 2014-10-30 08:52:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-30 08:53:13]

Patchset 2 (id:??) landed as
https://crrev.com/28acc0e482c0a4b3b852487bf0bcdd339f830e54
Cr-Commit-Position: refs/heads/master@{#302054}