Bypass the favicon_service when importing favicons.

Bypass the favicon_service when importing favicons.

BUG=391795
TBR=gab

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

[jif, 2014-07-07 17:07:48]

ptal

[blundell, 2014-07-08 09:19:45]

https://codereview.chromium.org/375613002/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_service.h (right):

https://codereview.chromium.org/375613002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.h:545: friend class ProfileWriter;
The SetImportedFavicons method should just be made public.

[jif-google, 2014-07-08 13:33:27]

https://codereview.chromium.org/375613002/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_service.h (right):

https://codereview.chromium.org/375613002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.h:545: friend class ProfileWriter;
On 2014/07/08 09:19:45, blundell wrote:
> The SetImportedFavicons method should just be made public.

Done.

[blundell, 2014-07-08 13:48:27]

lgtm

[jif-google, 2014-07-08 13:48:56]

The CQ bit was checked by jif@google.com

[commit-bot: I haz the power, 2014-07-08 13:49:49]

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

[commit-bot: I haz the power, 2014-07-08 17:23:04]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[commit-bot: I haz the power, 2014-07-08 17:26:48]

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

[commit-bot: I haz the power, 2014-07-08 17:26:49]

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

[jif, 2014-07-18 09:42:46]

The CQ bit was checked by jif@chromium.org

[jif, 2014-07-18 09:43:03]

The CQ bit was unchecked by jif@chromium.org

[jif, 2014-07-18 09:46:19]

+gab for OWNERS of chrome/browser/importer/profile_writer.cc

[jif, 2014-07-18 09:47:40]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2014-07-18 09:49:06]

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

[commit-bot: I haz the power, 2014-07-18 09:52:57]

Change committed as 284045

[gab, 2014-07-18 13:47:32]

Why would the importer have to know that this ends up in History? Feels like the
fact that FaviconService was using the HistoryService as a backing-store was an
implementation detail... what is gained from doing this apart from
short-circuiting an abstraction layer?

https://codereview.chromium.org/375613002/diff/40001/chrome/browser/importer/...
File chrome/browser/importer/profile_writer.cc (right):

https://codereview.chromium.org/375613002/diff/40001/chrome/browser/importer/...
chrome/browser/importer/profile_writer.cc:19: #include
"chrome/browser/favicon/favicon_service_factory.h"
rm the above 2 includes

[jif, 2014-07-18 14:14:19]

On 2014/07/18 13:47:32, gab wrote:
> Why would the importer have to know that this ends up in History? Feels like
the
> fact that FaviconService was using the HistoryService as a backing-store was
an
> implementation detail... what is gained from doing this apart from
> short-circuiting an abstraction layer?
> 
>
https://codereview.chromium.org/375613002/diff/40001/chrome/browser/importer/...
> File chrome/browser/importer/profile_writer.cc (right):
> 
>
https://codereview.chromium.org/375613002/diff/40001/chrome/browser/importer/...
> chrome/browser/importer/profile_writer.cc:19: #include
> "chrome/browser/favicon/favicon_service_factory.h"
> rm the above 2 includes

Ok.
Going to revert and go with a different approach.

[jif, 2014-07-18 14:15:48]

A revert of this CL has been created in
https://codereview.chromium.org/407513002/ by jif@chromium.org.

The reason for reverting is: There's a better approach..