Add no-image image in ContentSuggestions

Add no-image image in ContentSuggestions

Add a background image displayed while the application is fetching the images
or if there is no image for a suggestion.

BUG=698171
Review-Url: https://codereview.chromium.org/2770113006
Cr-Commit-Position: refs/heads/master@{#460139}
Committed: https://chromium.googlesource.com/chromium/src/+/3a1acac93195ccb06ca7d19fc40c1d81be32987b

[gambard, 2017-03-24 15:07:21]

gambard@chromium.org changed reviewers:
+ jif@chromium.org

[gambard, 2017-03-24 15:07:21]

PTAL.

[jif, 2017-03-28 09:04:58]

Consider using "thumbnail" almost everywhere you use "image", e.g.
s/imageContainer/thumbnailContainer/.

https://codereview.chromium.org/2770113006/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm
(right):

https://codereview.chromium.org/2770113006/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm:26:
NSString* const kImageName = @"content_suggestions_no_image";
kImageName can be made more precise. Possible replacements:
kNoThumbnailImageName.

https://codereview.chromium.org/2770113006/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm:83:
if (self.image)
we are already checking in |setContentImage| if its parameter is nil.

Either define |setContentImage| as never taking in a nil parameter (and put a
DCHECK inside), or have |setContentImage| support taking in nil parameters.

[gambard, 2017-03-28 09:40:30]

Thanks, PTAL.

https://codereview.chromium.org/2770113006/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm
(right):

https://codereview.chromium.org/2770113006/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm:26:
NSString* const kImageName = @"content_suggestions_no_image";
On 2017/03/28 09:04:58, jif wrote:
> kImageName can be made more precise. Possible replacements:
> kNoThumbnailImageName.

The backend service use "fetchSuggestionImage". I have added comments to make it
clearer.

https://codereview.chromium.org/2770113006/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_article_item.mm:83:
if (self.image)
On 2017/03/28 09:04:58, jif wrote:
> we are already checking in |setContentImage| if its parameter is nil.
> 
> Either define |setContentImage| as never taking in a nil parameter (and put a
> DCHECK inside), or have |setContentImage| support taking in nil parameters.

Done.

[jif, 2017-03-28 11:15:06]

lgtm

[gambard, 2017-03-28 11:58:42]

The CQ bit was checked by gambard@chromium.org

[commit-bot: I haz the power, 2017-03-28 11:58:59]

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-28 12:00:34]

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

[commit-bot: I haz the power, 2017-03-28 12:00:35]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[gambard, 2017-03-28 15:32:55]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-03-28 15:32:56]

The patchset sent to the CQ was uploaded after l-g-t-m from jif@chromium.org
Link to the patchset: https://codereview.chromium.org/2770113006/#ps60001
(title: "Fix tests")

[commit-bot: I haz the power, 2017-03-28 15:33:21]

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-28 17:01:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490715175524830,
"parent_rev": "86ade12dc989297c292d985e7bb9519fa4e2fce2", "commit_rev":
"3a1acac93195ccb06ca7d19fc40c1d81be32987b"}

[commit-bot: I haz the power, 2017-03-28 17:02:09]

Description was changed from

==========
Add no-image image in ContentSuggestions

Add a background image displayed while the application is fetching the images
or if there is no image for a suggestion.

BUG=698171
==========

to

==========
Add no-image image in ContentSuggestions

Add a background image displayed while the application is fetching the images
or if there is no image for a suggestion.

BUG=698171

Review-Url: https://codereview.chromium.org/2770113006
Cr-Commit-Position: refs/heads/master@{#460139}
Committed:
https://chromium.googlesource.com/chromium/src/+/3a1acac93195ccb06ca7d19fc40c...
==========

[commit-bot: I haz the power, 2017-03-28 17:02:10]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3a1acac93195ccb06ca7d19fc40c...