[Remote suggestions] Fetch favicon via ContentSuggestionsService

[Remote suggestions] Fetch favicon via ContentSuggestionsService

This CL follows up on 2781583002 and implements java code (behind a
feature) that fetches publisher's icons via content suggestions service.

BUG=692975
Review-Url: https://codereview.chromium.org/2790703003
Cr-Commit-Position: refs/heads/master@{#461412}
Committed: https://chromium.googlesource.com/chromium/src/+/af528b15fa5c278f063d7fbc5450ff187b8afdfb

[jkrcal, 2017-03-31 13:25:48]

jkrcal@chromium.org changed reviewers:
+ bauerb@chromium.org

[jkrcal, 2017-03-31 13:25:49]

Berhnard, could you PTAL?

[jkrcal, 2017-03-31 13:26:34]

The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 13:26:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 14:03:43]

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

[commit-bot: I haz the power, 2017-03-31 14:03:43]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-03-31 16:35:28]

lgtm

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
(right):

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java:252:
/** @return whether a snippet is a remote suggestion. */
Nit: Move this up, say after setRecentTabData(), or before isDownload()?

[dgn, 2017-03-31 17:48:47]

dgn@chromium.org changed reviewers:
+ dgn@chromium.org

[dgn, 2017-03-31 17:48:47]

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
(right):

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:216:
return;
not the old code path as that would be skipping the 2 calls after the catch
block.

Extract the fetchSuggestionFavicon call like we have done for the old code path
and just use a `else`?

[jkrcal, 2017-04-03 12:07:13]

The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 12:07:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jkrcal, 2017-04-03 12:07:31]

Thanks for the reviews!

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
(right):

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java:252:
/** @return whether a snippet is a remote suggestion. */
On 2017/03/31 16:35:28, Bernhard (slow until Apr 6) wrote:
> Nit: Move this up, say after setRecentTabData(), or before isDownload()?

Done.

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
(right):

https://codereview.chromium.org/2790703003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:216:
return;
On 2017/03/31 17:48:47, dgn wrote:
> not the old code path as that would be skipping the 2 calls after the catch
> block.
> 
> Extract the fetchSuggestionFavicon call like we have done for the old code
path
> and just use a `else`?

Oh, thanks for catching this!

[commit-bot: I haz the power, 2017-04-03 12:41:58]

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

[commit-bot: I haz the power, 2017-04-03 12:41:59]

Dry run: This issue passed the CQ dry run.

[dgn, 2017-04-03 12:55:47]

lgtm

[jkrcal, 2017-04-03 12:56:01]

The CQ bit was checked by jkrcal@chromium.org

[jkrcal, 2017-04-03 12:56:01]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2790703003/#ps40001
(title: "Bernhard's and Nicolas' comments")

[commit-bot: I haz the power, 2017-04-03 12:56:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 13:00:15]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491224161360160,
"parent_rev": "ca7e8fffb3a38d5fdb83f987cb2bc3f8c9909a8a", "commit_rev":
"af528b15fa5c278f063d7fbc5450ff187b8afdfb"}

[commit-bot: I haz the power, 2017-04-03 13:01:05]

Description was changed from

==========
[Remote suggestions] Fetch favicon via ContentSuggestionsService

This CL follows up on 2781583002 and implements java code (behind a
feature) that fetches publisher's icons via content suggestions service.

BUG=692975
==========

to

==========
[Remote suggestions] Fetch favicon via ContentSuggestionsService

This CL follows up on 2781583002 and implements java code (behind a
feature) that fetches publisher's icons via content suggestions service.

BUG=692975

Review-Url: https://codereview.chromium.org/2790703003
Cr-Commit-Position: refs/heads/master@{#461412}
Committed:
https://chromium.googlesource.com/chromium/src/+/af528b15fa5c278f063d7fbc5450...
==========

[commit-bot: I haz the power, 2017-04-03 13:01:06]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/af528b15fa5c278f063d7fbc5450...