📰 Fix tile icon rendering issue in Chrome Home

[Suggestions] Fix tile icon rendering issue in Chrome Home

Notifying the RecyclerView of item changes without providing a payload
makes it create a new ViewHolder to replace the existing one. The code
assumed the ViewHolder and the associated view are unique and ended up
updating 2 different view alternatively, triggering the visual glitch.

BUG=704130
Review-Url: https://codereview.chromium.org/2827363002
Cr-Commit-Position: refs/heads/master@{#466629}
Committed: https://chromium.googlesource.com/chromium/src/+/de49e186e1af90c584ba9d1b263a8b7afd215a04

[dgn, 2017-04-20 18:11:41]

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

[commit-bot: I haz the power, 2017-04-20 18:12:13]

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

[dgn, 2017-04-20 18:13:00]

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

[dgn, 2017-04-20 18:13:01]

PTAL. This does not fix the issue completely, as when scrolling there seems to
be some times when the RV still wants to recreate an item (observed only on the
spacing item  so far), but the bug does not seem reproducible at least.

Not sure what a definitive fix would be, it seems that the RV is just not meant
to hold "unique" items. Will keep looking and might propose something else
later.

[commit-bot: I haz the power, 2017-04-20 19:34:02]

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

[commit-bot: I haz the power, 2017-04-20 19:34:03]

Dry run: This issue passed the CQ dry run.

[dgn, 2017-04-24 09:43:27]

Description was changed from

==========
[Suggestions] Fix tile icon rendering issue in Chrome Home

Notifying the RecyclerView of item changes without providing a payload
makes it create a new ViewHolder to replace the existing one. The code
assumed the ViewHolder and the associated view are unique and ended up
updating 2 different view alternatively, triggering the vieual glitch.

BUG=704130
==========

to

==========
[Suggestions] Fix tile icon rendering issue in Chrome Home

Notifying the RecyclerView of item changes without providing a payload
makes it create a new ViewHolder to replace the existing one. The code
assumed the ViewHolder and the associated view are unique and ended up
updating 2 different view alternatively, triggering the visual glitch.

BUG=704130
==========

[Michael van Ouwerkerk, 2017-04-24 12:31:40]

lgtm

I wonder whether we should avoid reusing the views altogether. It has a speed
benefit, but if it turns out we have correctness problems, we shouldn't.

https://codereview.chromium.org/2827363002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
(right):

https://codereview.chromium.org/2827363002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java:70:
* @deprecated Change notification without payload recreate the view holder. Is
that on purpose?
s/notification/notifications/

[dgn, 2017-04-24 13:51:23]

Thanks!

I think our current usage is within the contract of the RV, so it should work
fine, even through occasional recreations. Only change notifications were
introducing a data/view mismatch and they are now fixed. The others should not
lead to visible bugs, just duplicate the view.

I'll try to get more info on whether it's possible to let the RV explicitly
treat some items as static, like a dumb scroll view would do. From what I've
seen marking things non recyclable throws them out and always recreates.

Landing this for now.

https://codereview.chromium.org/2827363002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
(right):

https://codereview.chromium.org/2827363002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java:70:
* @deprecated Change notification without payload recreate the view holder. Is
that on purpose?
On 2017/04/24 12:31:40, Michael van Ouwerkerk wrote:
> s/notification/notifications/

Done.

[dgn, 2017-04-24 13:51:34]

The CQ bit was checked by dgn@chromium.org

[dgn, 2017-04-24 13:51:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2827363002/#ps20001
(title: "rebase and address comment")

[commit-bot: I haz the power, 2017-04-24 13:51: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-04-24 14:24:22]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493041894595270,
"parent_rev": "a85fa6ea57acf1e6d8b3a13f1dc9b9754c2ec89b", "commit_rev":
"de49e186e1af90c584ba9d1b263a8b7afd215a04"}

[commit-bot: I haz the power, 2017-04-24 14:24:33]

Description was changed from

==========
[Suggestions] Fix tile icon rendering issue in Chrome Home

Notifying the RecyclerView of item changes without providing a payload
makes it create a new ViewHolder to replace the existing one. The code
assumed the ViewHolder and the associated view are unique and ended up
updating 2 different view alternatively, triggering the visual glitch.

BUG=704130
==========

to

==========
[Suggestions] Fix tile icon rendering issue in Chrome Home

Notifying the RecyclerView of item changes without providing a payload
makes it create a new ViewHolder to replace the existing one. The code
assumed the ViewHolder and the associated view are unique and ended up
updating 2 different view alternatively, triggering the visual glitch.

BUG=704130

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

[commit-bot: I haz the power, 2017-04-24 14:24:33]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/de49e186e1af90c584ba9d1b263a...