Add favicon integration tests for FaviconDriverImpl

Add favicon integration tests for FaviconDriverImpl

The goal is to have high-level integration tests that use the actual
backends (real FaviconService, real HistoryService), and make sure the
caching of favicons works as expected for popular configurations (for
which, rather popular sites have been used as examples).

While doing this, a few unused methods are removed from FaviconDriver.

BUG=694312

[mastiz, 2017-03-03 13:06:31]

Description was changed from

==========
Add favicon integration tests for FaviconDriverImpl

The goal is to have high-level integration tests that use the actual
backends (real FaviconService, real HistoryService), and make sure the
caching of favicons works as expected for popular configurations (for
which, rather popular sites have been used as examples).

BUG=694312
==========

to

==========
Add favicon integration tests for FaviconDriverImpl

The goal is to have high-level integration tests that use the actual
backends (real FaviconService, real HistoryService), and make sure the
caching of favicons works as expected for popular configurations (for
which, rather popular sites have been used as examples).

While doing this, a few unused methods are removed from FaviconDriver.

BUG=694312
==========

[mastiz, 2017-03-03 13:40:12]

mastiz@chromium.org changed reviewers:
+ pkotwicz@chromium.org

[mastiz, 2017-03-03 13:40:48]

pkotwicz@: I'd like to have high-level integration tests of this kind somewhere.
One option is here and another one would be ContentFaviconDriver, WDYT?

[pkotwicz, 2017-03-08 06:11:11]

Adding extra tests does not hurt. However, since none of the tests put anything
in the favicon database for the initial state, the tests really are:
- duplicating the tests in favicon_handler_unittest.cc
- testing FaviconService::SetFavicons(). There are a lot of tests already for
FaviconService::SetFavicons() already in history_backend_unittest.cc

I am not a big proponent of adding tests which have stuff in the database prior
to FaviconHandler running. It is likely that only the simple scenarios will be
tested because these tests are more onerous to write than the corresponding
favicon_handler_unittest.cc and history_backend_unittest.cc tests. There is
perhaps value in testing how the logic in HasExpiredOrIncompleteResult() deals
with weird database states.

[mastiz, 2017-09-04 09:25:02]

On 2017/03/08 06:11:11, pkotwicz wrote:
> Adding extra tests does not hurt. However, since none of the tests put
anything
> in the favicon database for the initial state, the tests really are:
> - duplicating the tests in favicon_handler_unittest.cc
> - testing FaviconService::SetFavicons(). There are a lot of tests already for
> FaviconService::SetFavicons() already in history_backend_unittest.cc
> 
> I am not a big proponent of adding tests which have stuff in the database
prior
> to FaviconHandler running. It is likely that only the simple scenarios will be
> tested because these tests are more onerous to write than the corresponding
> favicon_handler_unittest.cc and history_backend_unittest.cc tests. There is
> perhaps value in testing how the logic in HasExpiredOrIncompleteResult() deals
> with weird database states.

Since there wasn't much interest in these tests, and we currently have more
browser tests, I'm gonna drop this patch.