Make WebDataService no longer depend on ProfileKeyedService

Make WebDataService no longer depend on ProfileKeyedService

TBR=sky@chromium.org,akalin@chromium.org 
(sky@ for testing_profile changes, akalin@ for sync).
BUG=181277
COLLABORATOR=joi@chromium.org
Patch originally by caitkp@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=189234

[Cait (Slow), 2013-03-15 01:06:13]

Hi Joi, in light of having to revert and whatnot, it seemed like it made more
sense to abandon the first CL and start over. Here is a much better thought out
attempt at breaking WebDataService's dependency on PKS. 

(This change depends on: https://codereview.chromium.org/12871006/)

[Jói, 2013-03-15 17:40:59]

Overall this looks good to me, see a few comments below.

Cheers,
Jói

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
File chrome/browser/webdata/web_data_service.h (right):

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
chrome/browser/webdata/web_data_service.h:106: : public
base::RefCountedThreadSafe<WebDataService,
This base will have to move to WebDataServiceBase, but that can (and probably
should) happen in a later change.

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
File chrome/browser/webdata/web_data_service_factory.h (right):

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
chrome/browser/webdata/web_data_service_factory.h:35: // Singleton that owns all
PersonalDataManagers and associates them with
PersonalDataManager?  (Here and three lines down).

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
chrome/browser/webdata/web_data_service_factory.h:44: static
scoped_refptr<WebDataService> GetForProfile(
This approach of hiding the wrapper class will work for as long as
WebDataService is refcounted. Fine for now since we're not changing the
ownership model yet, but we may have to change this later, to something more
like the WebDataManager object you had in a different change that would own the
WebDatabaseService and each XyzWebDataService.

[Cait (Slow), 2013-03-17 18:50:56]

David, PTAL at c/b/webdata/ 

This change breaks WebData's direct dependency on PKS so we can move WebData
into its own component (it relies on https://codereview.chromium.org/12871006).

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
File chrome/browser/webdata/web_data_service_factory.h (right):

https://codereview.chromium.org/12491017/diff/1/chrome/browser/webdata/web_da...
chrome/browser/webdata/web_data_service_factory.h:35: // Singleton that owns all
PersonalDataManagers and associates them with
On 2013/03/15 17:40:59, Jói wrote:
> PersonalDataManager?  (Here and three lines down).

Done.

[dhollowa, 2013-03-18 18:50:59]

LGTM w/ nits.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
File chrome/browser/webdata/web_data_service_factory.cc (right):

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:20: web_data_service_ = NULL;
nit: not needed

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:24:
web_data_service_->ShutdownOnUIThread();
There may be advantage to: web_data_service_ = NULL; here though.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:79: // |service| can be NULL
in Incognito mode.
|service|?  or |wrapper|?

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:95: // |service| can be NULL
in Incognito mode.
ditto

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
File chrome/browser/webdata/web_data_service_factory.h (right):

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.h:32: };
DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.h:62: virtual bool
ServiceIsNULLWhileTesting() const OVERRIDE;
DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/12491017/diff/9002/components/autofill/browse...
File components/autofill/browser/autocomplete_history_manager_unittest.cc
(right):

https://codereview.chromium.org/12491017/diff/9002/components/autofill/browse...
components/autofill/browser/autocomplete_history_manager_unittest.cc:75: 
nit: remove empty line.

https://codereview.chromium.org/12491017/diff/9002/components/autofill/browse...
components/autofill/browser/autocomplete_history_manager_unittest.cc:76: };
DISALLOW_COPY_AND_ASSIGN

[Cait (Slow), 2013-03-18 19:10:34]

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
File chrome/browser/webdata/web_data_service_factory.cc (right):

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:20: web_data_service_ = NULL;
On 2013/03/18 18:50:59, dhollowa wrote:
> nit: not needed

Done.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:24:
web_data_service_->ShutdownOnUIThread();
On 2013/03/18 18:50:59, dhollowa wrote:
> There may be advantage to: web_data_service_ = NULL; here though.

Done.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:79: // |service| can be NULL
in Incognito mode.
On 2013/03/18 18:50:59, dhollowa wrote:
> |service|?  or |wrapper|?

Done.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.cc:95: // |service| can be NULL
in Incognito mode.
On 2013/03/18 18:50:59, dhollowa wrote:
> ditto

Done.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
File chrome/browser/webdata/web_data_service_factory.h (right):

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.h:32: };
On 2013/03/18 18:50:59, dhollowa wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/12491017/diff/9002/chrome/browser/webdata/web...
chrome/browser/webdata/web_data_service_factory.h:62: virtual bool
ServiceIsNULLWhileTesting() const OVERRIDE;
On 2013/03/18 18:50:59, dhollowa wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/12491017/diff/9002/components/autofill/browse...
File components/autofill/browser/autocomplete_history_manager_unittest.cc
(right):

https://codereview.chromium.org/12491017/diff/9002/components/autofill/browse...
components/autofill/browser/autocomplete_history_manager_unittest.cc:75: 
On 2013/03/18 18:50:59, dhollowa wrote:
> nit: remove empty line.

Done.

https://codereview.chromium.org/12491017/diff/9002/components/autofill/browse...
components/autofill/browser/autocomplete_history_manager_unittest.cc:76: };
On 2013/03/18 18:50:59, dhollowa wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

[commit-bot: I haz the power, 2013-03-19 04:22:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caitkp@chromium.org/12491017/28001

[commit-bot: I haz the power, 2013-03-19 04:24:46]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-03-19 04:57:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caitkp@chromium.org/12491017/28001

[commit-bot: I haz the power, 2013-03-19 05:10:34]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[commit-bot: I haz the power, 2013-03-20 00:48:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caitkp@chromium.org/12491017/89001

[commit-bot: I haz the power, 2013-03-20 09:43:20]

Change committed as 189234