Moves knowledge of Profile out of FaviconHandler, into FaviconTabHelper.

Moves knowledge of Profile out of FaviconHandler, into FaviconTabHelper.

This CL introduces an interface that will be used to access all of chrome/ API.
It is implemented by the FaviconTabHelper, and allows moving the
GetFaviconService() method out of the FaviconHandler.

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

[jif, 2014-04-08 16:11:49]

can you TAL?
The favicon.gypi diff is bizarre because I messed the father CL
(https://codereview.chromium.org/227153007/)

[blundell, 2014-04-09 09:17:21]

LGTM with nits

I would add something about introducing the FaviconClient interface to the CL
description

https://codereview.chromium.org/228783002/diff/20001/chrome/browser/favicon/f...
File chrome/browser/favicon/favicon_tab_helper.h (right):

https://codereview.chromium.org/228783002/diff/20001/chrome/browser/favicon/f...
chrome/browser/favicon/favicon_tab_helper.h:85: // Returns a FaviconService.
Don't need to comment overriden methods. Just say "FaviconClient
implementation:"

https://codereview.chromium.org/228783002/diff/20001/components/favicon/core/...
File components/favicon/core/browser/favicon_client.h (right):

https://codereview.chromium.org/228783002/diff/20001/components/favicon/core/...
components/favicon/core/browser/favicon_client.h:10: // This class abstracts
operations that depend on embedders environment, e.g.
the embedder's

[jif, 2014-04-09 11:22:48]

Thanks.

+stevenjb for an extra pair of eyes.

PS: Unless you want to be a reviewer on the rest of the favicon componentization
CLs, I'm going to stop adding you as a reviewer, and only rely on blundell for
the LGTMs.

https://codereview.chromium.org/228783002/diff/20001/chrome/browser/favicon/f...
File chrome/browser/favicon/favicon_tab_helper.h (right):

https://codereview.chromium.org/228783002/diff/20001/chrome/browser/favicon/f...
chrome/browser/favicon/favicon_tab_helper.h:85: // Returns a FaviconService.
On 2014/04/09 09:17:21, blundell wrote:
> Don't need to comment overriden methods. Just say "FaviconClient
> implementation:"

Done.

https://codereview.chromium.org/228783002/diff/20001/components/favicon/core/...
File components/favicon/core/browser/favicon_client.h (right):

https://codereview.chromium.org/228783002/diff/20001/components/favicon/core/...
components/favicon/core/browser/favicon_client.h:10: // This class abstracts
operations that depend on embedders environment, e.g.
On 2014/04/09 09:17:21, blundell wrote:
> the embedder's

Done.

[blundell, 2014-04-09 11:24:27]

On future CLs, you can CC stevenjb and sky just so they're aware of the work
that's going on.

[stevenjb, 2014-04-09 16:35:37]

lgtm

[jif, 2014-04-09 19:15:19]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2014-04-09 19:15:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jif@chromium.org/228783002/80001

[commit-bot: I haz the power, 2014-04-09 19:49:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-09 19:49:43]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg
  tryserver.chromium on linux_chromium_chromeos_rel

[jif, 2014-04-10 08:44:13]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2014-04-10 08:44:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jif@chromium.org/228783002/80001

[commit-bot: I haz the power, 2014-04-10 13:24:36]

Change committed as 262960