Profiles: Move NetworkActionPredictor to the ProfileKeyedService framework.

Profiles: Move NetworkActionPredictor to the ProfileKeyedService framework.

BUG=77155, 103574
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114067

[Elliot Glaysher, 2011-12-09 00:07:44]

dominich: review
mirandac: fyi

[dominich, 2011-12-12 16:53:10]

On 2011/12/09 00:07:44, Elliot Glaysher wrote:
> dominich: review
> mirandac: fyi

Thanks for looking at this: Please add BUG 103574 to the CL.

[dominich, 2011-12-12 16:58:37]

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/network_action_predictor.cc (right):

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/network_action_predictor.cc:91:
db_->OnPredictorDestroyed();
This should be moved to Shutdown.

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/network_action_predictor_factory.cc (right):

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/network_action_predictor_factory.cc:32: Profile*
profile) const {
This probably shouldn't be created for incognito profiles.

[Elliot Glaysher, 2011-12-12 18:16:15]

PTAL

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/network_action_predictor.cc (right):

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/network_action_predictor.cc:91:
db_->OnPredictorDestroyed();
On 2011/12/12 16:58:39, dominich wrote:
> This should be moved to Shutdown.

Done.

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/network_action_predictor_factory.cc (right):

http://codereview.chromium.org/8872036/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/network_action_predictor_factory.cc:32: Profile*
profile) const {
On 2011/12/12 16:58:39, dominich wrote:
> This probably shouldn't be created for incognito profiles. 

It isn't. ServiceRedirectedInIncognito() and ServiceHasOwnInstanceInIncognito()
aren't overloaded.

[dominich, 2011-12-12 18:25:01]

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/network_action_predictor_unittest.cc (right):

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/network_action_predictor_unittest.cc:89: virtual
~NetworkActionPredictorTest() {
I'm not sure, but I think calling this in TearDown is a better reflection of
normal code flow.

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_dependency_manager.cc (right):

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_dependency_manager.cc:46:
NetworkActionPredictorFactory::GetInstance();
As this is the only place GetInstance is called, and it's actually used to
initialize the factory, should it be called Initialize instead?

[Elliot Glaysher, 2011-12-12 18:43:47]

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/network_action_predictor_unittest.cc (right):

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/network_action_predictor_unittest.cc:89: virtual
~NetworkActionPredictorTest() {
On 2011/12/12 18:25:01, dominich wrote:
> I'm not sure, but I think calling this in TearDown is a better reflection of
> normal code flow.

Done.

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_dependency_manager.cc (right):

http://codereview.chromium.org/8872036/diff/6001/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_dependency_manager.cc:46:
NetworkActionPredictorFactory::GetInstance();
On 2011/12/12 18:25:01, dominich wrote:
> As this is the only place GetInstance is called, and it's actually used to
> initialize the factory, should it be called Initialize instead?

This isn't the only place it's used. It's used from the GetForProfile() accessor
and, if anything ever depends on NetworkActionPredictorFactory being created
first, it will be called from a DependsOn() line.

SpeechInputExtensionManager::InitializeFactory() is wrong.

[dominich, 2011-12-12 18:45:49]

LGTM modulo trybots passing.

[commit-bot: I haz the power, 2011-12-12 18:50:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/8872036/8002

[commit-bot: I haz the power, 2011-12-12 20:08:48]

Change committed as 114067