[NTP::Downloads] Store file type icon padding as a constant.

[NTP::Downloads] Store file type icon padding as a constant.

Add file type icon padding into dimens.xml, read it once and store as a
member of SnippetArticleViewHolder.

BUG=693552
Review-Url: https://codereview.chromium.org/2789133002
Cr-Commit-Position: refs/heads/master@{#461408}
Committed: https://chromium.googlesource.com/chromium/src/+/51e2977044591f818e84c297e5dec72c639f465f

[vitaliii, 2017-04-03 07:11:51]

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

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

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

[vitaliii, 2017-04-03 07:12:10]

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

[vitaliii, 2017-04-03 07:12:10]

Hi dgn@,

PTAL.

[vitaliii, 2017-04-03 07:38:51]

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

[commit-bot: I haz the power, 2017-04-03 07:39:00]

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-04-03 08:14:05]

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

[commit-bot: I haz the power, 2017-04-03 08:14:05]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2017-04-03 09:08:01]

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

[Michael van Ouwerkerk, 2017-04-03 09:08:02]

lgtm with nits

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

https://codereview.chromium.org/2789133002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:88:
private final ColorStateList mIconForegroundColorList;
nit: move this one below mIconBackgroundColor as it is assigned in that order.

https://codereview.chromium.org/2789133002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:128:
mFileTypeIconPaddingPx = mThumbnailView.getResources().getDimensionPixelSize(
nit: please keep assignment of similar members together and in declaration order
where it makes sense. In this case, maybe move this after assigning
mIconForegroundColorList.

https://codereview.chromium.org/2789133002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:128:
mFileTypeIconPaddingPx = mThumbnailView.getResources().getDimensionPixelSize(
nit: use itemView.getResources() like in the rest of this constructor.

[dgn, 2017-04-03 10:22:06]

Thanks! avoiding unnecessary calculations is always good. lgtm

[dgn, 2017-04-03 10:22:23]

(% michael's comments)

[vitaliii, 2017-04-03 10:59:36]

The CQ bit was checked by vitaliii@chromium.org

[vitaliii, 2017-04-03 10:59:36]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2789133002/#ps40001
(title: "mvanouwerkerk@ nits.")

[commit-bot: I haz the power, 2017-04-03 10:59:43]

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 11:36:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491217176165480,
"parent_rev": "c0b79d75dc0b349a3ae2d9faf74e4f5d8584e844", "commit_rev":
"51e2977044591f818e84c297e5dec72c639f465f"}

[commit-bot: I haz the power, 2017-04-03 11:37:43]

Description was changed from

==========
[NTP::Downloads] Store file type icon padding as a constant.

Add file type icon padding into dimens.xml, read it once and store as a
member of SnippetArticleViewHolder.

BUG=693552
==========

to

==========
[NTP::Downloads] Store file type icon padding as a constant.

Add file type icon padding into dimens.xml, read it once and store as a
member of SnippetArticleViewHolder.

BUG=693552

Review-Url: https://codereview.chromium.org/2789133002
Cr-Commit-Position: refs/heads/master@{#461408}
Committed:
https://chromium.googlesource.com/chromium/src/+/51e2977044591f818e84c297e5de...
==========

[commit-bot: I haz the power, 2017-04-03 11:37:44]

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

[vitaliii, 2017-04-03 13:08:50]

I addressed your nits.
No need to look.

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

https://codereview.chromium.org/2789133002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:88:
private final ColorStateList mIconForegroundColorList;
On 2017/04/03 09:08:01, Michael van Ouwerkerk wrote:
> nit: move this one below mIconBackgroundColor as it is assigned in that order.

Done.

https://codereview.chromium.org/2789133002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java:128:
mFileTypeIconPaddingPx = mThumbnailView.getResources().getDimensionPixelSize(
On 2017/04/03 09:08:01, Michael van Ouwerkerk wrote:
> nit: use itemView.getResources() like in the rest of this constructor.

Both done.