Moving FaviconService to a ProfileKeyedService.

Moving FaviconService to a ProfileKeyedService.

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

Reverted, was not the cause of problem. Resubmitting.

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

[rpetterson, 2012-08-10 20:34:15]

All small one-line-ish changes, but need a lot of owners approvals.

atwilson - sync/glue
estade - ui/webui/*
groby - intents, ui/intents
mirandac - importer, profiles
pkasting - ui/search_engines, ui/toolbar
sail - ui/cocoa
sky - bookmarks, favicon, jumplist_win.cc, chrome_browser.gypi
sreeram - instant

[Andrew T Wilson (Slow), 2012-08-10 20:36:41]

sync/glue LGTM

[sail, 2012-08-10 20:38:52]

LGTM
CCing erg

[Miranda Callahan, 2012-08-10 20:40:22]

On 2012/08/10 20:38:52, sail wrote:
> LGTM
> CCing erg

importer/ & profiles/ LGTM

[groby-ooo-7-16, 2012-08-10 20:42:50]

intents lgtm

[sreeram, 2012-08-10 22:10:59]

LGTM for chrome/browser/instant/...

[sky, 2012-08-10 22:41:41]

LGTM

[Evan Stade, 2012-08-11 01:57:05]

webui lgtm. For mechanical changes like this you can TBR owners reviews.

[Peter Kasting, 2012-08-11 02:52:34]

LGTM (and agree on it being OK to TBR though there has historically been
disagreement about this)

[Elliot Glaysher, 2012-08-13 17:44:25]

profiles lgtm

[andreip3000, 2012-08-17 10:11:26]

http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
File chrome/browser/favicon/favicon_service.h (right):

http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
chrome/browser/favicon/favicon_service.h:110: Profile* profile_;
I think this dependency must go. You can just inject the HistoryService into the
FaviconService at construction time. Sadly, there is one other use of profile_
in favicon_service.cc that will take a while to untangle (that's why I left a
TODO in my CL).

http://codereview.chromium.org/10857037/

[rpetterson, 2012-08-19 01:10:01]

andreip, PTAL.

http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
File chrome/browser/favicon/favicon_service.h (right):

http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
chrome/browser/favicon/favicon_service.h:110: Profile* profile_;
I think I've gotten rid of all the profile dependencies. PTAL.

On 2012/08/17 10:11:26, andreip3000 wrote:
> I think this dependency must go. You can just inject the HistoryService into
the
> FaviconService at construction time. Sadly, there is one other use of profile_
> in favicon_service.cc that will take a while to untangle (that's why I left a
> TODO in my CL).
> 
> http://codereview.chromium.org/10857037/

[rpetterson, 2012-08-21 21:18:57]

ping andreip?

On 2012/08/19 01:10:01, rpetterson wrote:
> andreip, PTAL.
> 
>
http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
> File chrome/browser/favicon/favicon_service.h (right):
> 
>
http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
> chrome/browser/favicon/favicon_service.h:110: Profile* profile_;
> I think I've gotten rid of all the profile dependencies. PTAL.
> 
> On 2012/08/17 10:11:26, andreip3000 wrote:
> > I think this dependency must go. You can just inject the HistoryService into
> the
> > FaviconService at construction time. Sadly, there is one other use of
profile_
> > in favicon_service.cc that will take a while to untangle (that's why I left
a
> > TODO in my CL).
> > 
> > http://codereview.chromium.org/10857037/

[andreip3000, 2012-08-21 21:26:46]

Sorry, LGTM! 

Thanks!


On 2012/08/21 21:18:57, rpetterson wrote:
> ping andreip?
> 
> On 2012/08/19 01:10:01, rpetterson wrote:
> > andreip, PTAL.
> > 
> >
>
http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
> > File chrome/browser/favicon/favicon_service.h (right):
> > 
> >
>
http://codereview.chromium.org/10828263/diff/1067/chrome/browser/favicon/favi...
> > chrome/browser/favicon/favicon_service.h:110: Profile* profile_;
> > I think I've gotten rid of all the profile dependencies. PTAL.
> > 
> > On 2012/08/17 10:11:26, andreip3000 wrote:
> > > I think this dependency must go. You can just inject the HistoryService
into
> > the
> > > FaviconService at construction time. Sadly, there is one other use of
> profile_
> > > in favicon_service.cc that will take a while to untangle (that's why I
left
> a
> > > TODO in my CL).
> > > 
> > > http://codereview.chromium.org/10857037/

[commit-bot: I haz the power, 2012-08-22 22:46:59]

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

[commit-bot: I haz the power, 2012-08-23 01:08:28]

Change committed as 152904

[commit-bot: I haz the power, 2012-08-23 18:20:12]

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

[commit-bot: I haz the power, 2012-08-23 22:28:02]

Change committed as 153087