Taking over issue 10006037: Moved WebDataService to ProfileKeyedService

Taking over issue 10006037.

Moved WebDataService to ProfileKeyedService 

James: 
chrome\browser\ui\intents 
Peter: 
chrome\browser\ui\search_engines 
chrome\browser\search_engines 
Nicolas: 
chrome\browser\sync 
Rachel/Elliot: 
chrome\browser\profiles and the whole cl 

BUG=112234
TEST=unit-tests 

TBR=jhawkins@chromium.org,pkasting@chromium.org,zea@chromium.org,erg@chromium.org,isherman@chromium.org

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

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

[commit-bot: I haz the power, 2012-04-25 01:53:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/28005

[commit-bot: I haz the power, 2012-04-25 02:55:31]

Try job failure for 10185008-28005 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-25 07:11:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/16235

[commit-bot: I haz the power, 2012-04-25 08:14:44]

Try job failure for 10185008-16235 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-25 08:30:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/16235

[commit-bot: I haz the power, 2012-04-25 09:29:23]

Try job failure for 10185008-16235 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-25 15:49:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/16235

[commit-bot: I haz the power, 2012-04-25 17:06:08]

Try job failure for 10185008-16235 (retry) (retry) on mac_rel for step
"unit_tests" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-25 17:36:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/43009

[commit-bot: I haz the power, 2012-04-25 18:37:17]

Try job failure for 10185008-43009 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-05-11 02:24:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/65114

[commit-bot: I haz the power, 2012-05-11 03:54:27]

Change committed as 136499

[Lei Zhang, 2012-05-11 04:02:55]

Erm, I don't think the TBR list ever got this email...

[rpetterson, 2012-05-11 17:13:48]

On 2012/05/11 04:02:55, Lei Zhang wrote:
> Erm, I don't think the TBR list ever got this email...

This a CL which has simply taken a real long time to get through (~ 2 months).
It has all the LGTMs on the previous versions:
http://codereview.chromium.org/10006037/ (sky's LGTM)
http://codereview.chromium.org/9949024/
http://codereview.chromium.org/9834056/ (all other LGTMs)

If you think it should go through another round of reviews, let me know.

(It was reverted, so I have to fix something so I'll be trying again anyhow.)

[Peter Kasting, 2012-05-11 22:29:32]

chrome/browser/search_engines LGTM

http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
File chrome/browser/search_engines/template_url_service_test_util.cc (right):

http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
chrome/browser/search_engines/template_url_service_test_util.cc:155: return
test_profile->service_;
Nit: Should we move the temp dir/service init from SetUp() to here?  I'm not
sure this would work... I'm just looking for ways to make the ownership and
construction patterns here "more typical"

[rpetterson, 2012-05-11 23:09:10]

http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
File chrome/browser/search_engines/template_url_service_test_util.cc (right):

http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
chrome/browser/search_engines/template_url_service_test_util.cc:155: return
test_profile->service_;
On 2012/05/11 22:29:32, Peter Kasting wrote:
> Nit: Should we move the temp dir/service init from SetUp() to here?  I'm not
> sure this would work... I'm just looking for ways to make the ownership and
> construction patterns here "more typical"

Yes, the way you describe makes more logical sense. However, they are doing some
unique things with the creation and deletion of the service that would require
more invasive changes to fix it. Seeing as this CL is already huge, I'd rather
wait to make those changes in a separate CL.

[Peter Kasting, 2012-05-11 23:15:02]

On 2012/05/11 23:09:10, rpetterson wrote:
>
http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
> File chrome/browser/search_engines/template_url_service_test_util.cc (right):
> 
>
http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
> chrome/browser/search_engines/template_url_service_test_util.cc:155: return
> test_profile->service_;
> On 2012/05/11 22:29:32, Peter Kasting wrote:
> > Nit: Should we move the temp dir/service init from SetUp() to here?  I'm not
> > sure this would work... I'm just looking for ways to make the ownership and
> > construction patterns here "more typical"
> 
> Yes, the way you describe makes more logical sense. However, they are doing
some
> unique things with the creation and deletion of the service that would require
> more invasive changes to fix it. Seeing as this CL is already huge, I'd rather
> wait to make those changes in a separate CL.

No problem.  If you think this is worth doing please do file a bug so it won't
get lost :)

[rpetterson, 2012-05-11 23:35:48]

On 2012/05/11 23:15:02, Peter Kasting wrote:
> On 2012/05/11 23:09:10, rpetterson wrote:
> >
>
http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
> > File chrome/browser/search_engines/template_url_service_test_util.cc
(right):
> > 
> >
>
http://codereview.chromium.org/10185008/diff/65114/chrome/browser/search_engi...
> > chrome/browser/search_engines/template_url_service_test_util.cc:155: return
> > test_profile->service_;
> > On 2012/05/11 22:29:32, Peter Kasting wrote:
> > > Nit: Should we move the temp dir/service init from SetUp() to here?  I'm
not
> > > sure this would work... I'm just looking for ways to make the ownership
and
> > > construction patterns here "more typical"
> > 
> > Yes, the way you describe makes more logical sense. However, they are doing
> some
> > unique things with the creation and deletion of the service that would
require
> > more invasive changes to fix it. Seeing as this CL is already huge, I'd
rather
> > wait to make those changes in a separate CL.
> 
> No problem.  If you think this is worth doing please do file a bug so it won't
> get lost :)

Done: crbug.com/127884

[commit-bot: I haz the power, 2012-05-14 20:26:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10185008/57064

[commit-bot: I haz the power, 2012-05-14 23:49:50]

Change committed as 137014