📰 Do not refresh Tiles while visible

[NTP Client] Do not refresh Tiles while visible

Tiles are now refreshed when:
- The TileGroup first receives data
- The user explicitly modifies the tiles (removes one)
- The user switches to an existing NTP or opens Home

Known issues:
- New NTPs might have stale data because on registration
it pulls the currently cached tile data, then refreshes it
expecting the NTP to display the changes. But this patches
explicitly blocks that. 

BUG=694262
Review-Url: https://codereview.chromium.org/2742433002
Cr-Commit-Position: refs/heads/master@{#455806}
Committed: https://chromium.googlesource.com/chromium/src/+/2d368dbfe6323fe630398869d3e82c7d728a1035

[dgn, 2017-03-08 17:27:24]

Description was changed from

==========
[NTP Client] Do not refresh Tiles while visible

BUG=
==========

to

==========
[NTP Client] Do not refresh Tiles while visible

Tiles are now refreshed when:
- The TileGroup first receives data
- The user explicitly modifies the tiles (removes one)
- The user switches to an existing NTP or opens Home

Known issues:
- New NTPs might have stale data because on registration
it pulls the currently cached tile data, then refreshes it
expecting the NTP to display the changes. But this patches
explicitly blocks that. 

BUG=694262
==========

[dgn, 2017-03-08 17:27:25]

dgn@chromium.org changed reviewers:
+ bauerb@chromium.org, fhorschig@chromium.org

[dgn, 2017-03-08 17:43:24]

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

[dgn, 2017-03-08 17:43:36]

PTAL

[commit-bot: I haz the power, 2017-03-08 17:43:54]

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

[Bernhard Bauer, 2017-03-08 17:56:25]

lgtm

[commit-bot: I haz the power, 2017-03-08 19:27:20]

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

[commit-bot: I haz the power, 2017-03-08 19:27:21]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dgn, 2017-03-09 12:11:51]

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

[commit-bot: I haz the power, 2017-03-09 12: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-03-09 12:12:36]

PTAL. Meaningful change: I now always replace mTiles with the pending data, even
if I don't send a notification that data changed. This is so that I can clear
the pending data to acknowledge that we loaded it. 
If the data did not change, the tiles themselves should not have significant
changes and since we don't keep references, it should not have any impact to the
current behaviour.

[dgn, 2017-03-09 12:14:34]

note: patchset 4 should be easier that 5 to review vs master

[dgn, 2017-03-09 12:14:46]

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

[commit-bot: I haz the power, 2017-03-09 12:14:57]

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-09 12:47:46]

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

[commit-bot: I haz the power, 2017-03-09 12:47:47]

Dry run: This issue passed the CQ dry run.

[dgn, 2017-03-09 18:34:41]

The CQ bit was checked by dgn@chromium.org

[dgn, 2017-03-09 18:34:41]

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/2742433002/#ps80001
(title: "move loadTiles with other private methods")

[commit-bot: I haz the power, 2017-03-09 18:35:35]

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

[Bernhard Bauer, 2017-03-09 18:38:12]

Sry, still LGTM. One nit, but you could do that in a followup.

https://codereview.chromium.org/2742433002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java
(right):

https://codereview.chromium.org/2742433002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java:340:
if (newTile.importData(getTile(newTile.getUrl()))) dataChanged = true;
Nit: it technically fits onto one line, but this is getting to the limit of
readability. I would consider breaking and using braces.

[commit-bot: I haz the power, 2017-03-09 18:40:55]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489084481029590,
"parent_rev": "870bce2d5e6ed3ba5d7b513b69d55fdb26efc2d6", "commit_rev":
"2d368dbfe6323fe630398869d3e82c7d728a1035"}

[commit-bot: I haz the power, 2017-03-09 18:41:33]

Description was changed from

==========
[NTP Client] Do not refresh Tiles while visible

Tiles are now refreshed when:
- The TileGroup first receives data
- The user explicitly modifies the tiles (removes one)
- The user switches to an existing NTP or opens Home

Known issues:
- New NTPs might have stale data because on registration
it pulls the currently cached tile data, then refreshes it
expecting the NTP to display the changes. But this patches
explicitly blocks that. 

BUG=694262
==========

to

==========
[NTP Client] Do not refresh Tiles while visible

Tiles are now refreshed when:
- The TileGroup first receives data
- The user explicitly modifies the tiles (removes one)
- The user switches to an existing NTP or opens Home

Known issues:
- New NTPs might have stale data because on registration
it pulls the currently cached tile data, then refreshes it
expecting the NTP to display the changes. But this patches
explicitly blocks that. 

BUG=694262

Review-Url: https://codereview.chromium.org/2742433002
Cr-Commit-Position: refs/heads/master@{#455806}
Committed:
https://chromium.googlesource.com/chromium/src/+/2d368dbfe6323fe630398869d3e8...
==========

[commit-bot: I haz the power, 2017-03-09 18:41:34]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/2d368dbfe6323fe630398869d3e8...