Pass an instance of FaviconClient to FaviconService.

Pass an instance of FaviconClient to FaviconService.

This is an intermediary CL where the FaviconClient is not used by the FaviconService.
Subsequent CLs will start using the FaviconClient in the FaviconService to abstract usage of chrome/ code.

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

[jif-google, 2014-05-27 18:45:43]

PTAL

[blundell, 2014-05-27 18:48:39]

LGTM

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

https://codereview.chromium.org/302433010/diff/20001/chrome/browser/favicon/f...
chrome/browser/favicon/favicon_service.h:29: explicit FaviconService(Profile*
profile, FaviconClient* favicon_client);
nit: get rid of explicit

https://codereview.chromium.org/302433010/diff/20001/chrome/browser/favicon/f...
chrome/browser/favicon/favicon_service.h:29: explicit FaviconService(Profile*
profile, FaviconClient* favicon_client);
Add a comment on these parameters, including their expected lifetimes. Add a
TODO(jif) to eliminate the Profile parameter with a bug link.

[jif, 2014-05-28 09:05:40]

+phajdan.jr for OWNER of chrome/test/base/testing_profile.cc

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

https://codereview.chromium.org/302433010/diff/20001/chrome/browser/favicon/f...
chrome/browser/favicon/favicon_service.h:29: explicit FaviconService(Profile*
profile, FaviconClient* favicon_client);
On 2014/05/27 18:48:40, blundell wrote:
> nit: get rid of explicit

Done.

[Paweł Hajdan Jr., 2014-05-28 12:24:32]

chrome/test LGTM

[jif, 2014-06-10 15:13:40]

The CQ bit was checked by jif@chromium.org

[jif, 2014-06-10 15:13:47]

The CQ bit was unchecked by jif@chromium.org

[jif, 2014-06-12 13:39:20]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2014-06-12 13:40:56]

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

[commit-bot: I haz the power, 2014-06-12 18:05:41]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[jif-google, 2014-06-12 18:05:58]

The CQ bit was unchecked by jif@google.com

[jif, 2014-06-24 09:33:49]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2014-06-24 09:36:14]

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

[commit-bot: I haz the power, 2014-06-24 11:16:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)

[commit-bot: I haz the power, 2014-06-24 11:26:03]

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

[commit-bot: I haz the power, 2014-06-24 11:26:04]

Try jobs failed on following builders:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)

[jif-google, 2014-07-03 12:04:44]

The CQ bit was checked by jif@google.com

[commit-bot: I haz the power, 2014-07-03 12:05:51]

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

[jif-google, 2014-07-03 12:06:51]

The CQ bit was unchecked by jif@google.com

[jif-google, 2014-07-03 12:10:31]

The CQ bit was checked by jif@google.com

[commit-bot: I haz the power, 2014-07-03 12:11:56]

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

[commit-bot: I haz the power, 2014-07-03 15:01:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-03 15:10:26]

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

[commit-bot: I haz the power, 2014-07-03 15:10:27]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[jif-google, 2014-07-03 15:31:51]

The CQ bit was checked by jif@google.com

[commit-bot: I haz the power, 2014-07-03 15:32:40]

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

[commit-bot: I haz the power, 2014-07-03 18:41:54]

Change committed as 281304

[michaelbai, 2014-10-15 23:54:20]

On 2014/07/03 18:41:54, I haz the power (commit-bot) wrote:
> Change committed as 281304

Is there upcoming patch? This is landed 4 months ago, but FaviconClient is not
used yet.

[blundell, 2014-10-16 06:25:05]

On 2014/10/15 23:54:20, michaelbai wrote:
> On 2014/07/03 18:41:54, I haz the power (commit-bot) wrote:
> > Change committed as 281304
> 
> Is there upcoming patch? This is landed 4 months ago, but FaviconClient is not
> used yet.

Thanks for checking in. The completion of the favicon componentization has been
blocked by the history componentization, which has been a tremendous amount of
work. J-F, maybe you (or someone else) could complete the componentization
in-place in //chrome/browser, and then the code could be moved to the component
once the //chrome/browser/history dependencies are transformed to
//components/history dependencies?

[blundell, 2014-10-16 06:26:05]

On 2014/10/16 06:25:05, blundell wrote:
> On 2014/10/15 23:54:20, michaelbai wrote:
> > On 2014/07/03 18:41:54, I haz the power (commit-bot) wrote:
> > > Change committed as 281304
> > 
> > Is there upcoming patch? This is landed 4 months ago, but FaviconClient is
not
> > used yet.
> 
> Thanks for checking in. The completion of the favicon componentization has
been
> blocked by the history componentization, which has been a tremendous amount of
> work. J-F, maybe you (or someone else) could complete the componentization
> in-place in //chrome/browser, and then the code could be moved to the
component
> once the //chrome/browser/history dependencies are transformed to
> //components/history dependencies?

+sdefresne@

To be clear, the suggestion is to complete the *favicon* componentization in
place.

[jif-google, 2014-10-16 16:21:13]

On 2014/10/16 06:26:05, blundell wrote:
> On 2014/10/16 06:25:05, blundell wrote:
> > On 2014/10/15 23:54:20, michaelbai wrote:
> > > On 2014/07/03 18:41:54, I haz the power (commit-bot) wrote:
> > > > Change committed as 281304
> > > 
> > > Is there upcoming patch? This is landed 4 months ago, but FaviconClient is
> not
> > > used yet.
> > 
> > Thanks for checking in. The completion of the favicon componentization has
> been
> > blocked by the history componentization, which has been a tremendous amount
of
> > work. J-F, maybe you (or someone else) could complete the componentization
> > in-place in //chrome/browser, and then the code could be moved to the
> component
> > once the //chrome/browser/history dependencies are transformed to
> > //components/history dependencies?
> 
> +sdefresne@
> 
> To be clear, the suggestion is to complete the *favicon* componentization in
> place.

I'm currently busy with Material Design stuff for M40, but in 2 weeks I'm going
to get back and finish the favicon componentization.
I'm going to inform sdesfresne of what's blocking the work so that he can
prioritize his work on history.