Moving to a ProfileKeyedService world
Elliot Glaysher (Chromium)
If you have an object hanging off of Profile, accessed like this:
profile_->GetMyThing(), please read! We need help changing all these
getters into a new style that should fix existing bugs, and let us
move faster!
You can easily move to this new world where dependency management,
startup ordering and shutdown ordering are all managed for you when
you:
1) Make your existing FooService derive from ProfileKeyedService.
2) If possible, make your FooService no longer refcounted. Most of the
refcounted objects that hang off of Profile appear to be that way
because they don't use ScopedRunnableMethodFactory<> instead of
needing to run on multiple threads.
3) Build a simple FooServiceFactory derived from
ProfileKeyedServiceFactory and override the template methods like
ServiceRedirectedInIncognito() or ServiceIsCreatedWithProfile() to
control your service's lifecycle and incognito binding.
4) Add your service to the AssertFactoriesBuilt() list in
profile_dependency_manager.cc.
5) Make sure your service drops its weak reference to the profile in
BlahService::Shutdown().
6) Change each instance of "profile_->GetFooService()" to
"FooServiceFactory::GetForProfile(profile_);"
Here are some example patches of previous conversions:
http://codereview.chromium.org/6820065/ and
http://codereview.chromium.org/6792047.
Q: So why do it this way?
A: Our shutdown order is wrong today. There are infrequent race
conditions and crashes due to either explicit shutdown order being
subtly wrong as stated in the code or due to weird refcounting issues.
Our shutdown is manually ordered and subtle. We're not sure of all the
issues there. We have also fixed a few use of Profile after free
errors here so far and expect to find more.
A big part of the motivation is to state a service's dependencies
explicitly in code and than have the framework figure out the startup
and shutdown dependency ordering for you. In your constructor for your
ServiceFactory subclass, you say something like:
DependsOn(HistoryServiceFactory::GetInstance());
And now the framework will make sure the HistoryService is created
before your service is created and will ensure your service receives a
Shutdown() call before HistoryService::Shutdown(), and will make sure
your service is deleted before HistoryService is deleted. (For
historical reasons, we need to have a two pass shutdown. It's
complex.)
Q: Why care about what was just a crash at shutdown? Our shutdown is
crashy in other places as well and the user probably doesn't care!
A: In a multiprofile world, where we're shutting down Profiles during
normal chrome usage instead of the end, crashes will be much more
noticeable.
We also want to be able to start moving to a world where we can
compile in each feature.
-- Elliot