Make possible to remove the last most visited tile

Make possible to remove the last most visited tile

The last Most Visited tile could not be removed in the NTP. This CL fixes it.

BUG=730033
Review-Url: https://codereview.chromium.org/2923803002
Cr-Commit-Position: refs/heads/master@{#477569}
Committed: https://chromium.googlesource.com/chromium/src/+/b90a205103775b54fac946c7617222ba9700b544

[gambard, 2017-06-06 15:40:51]

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

[gambard, 2017-06-06 15:40:52]

justincohen@: PTAL
fhorschig@: FYI

[justincohen, 2017-06-06 15:47:34]

https://codereview.chromium.org/2923803002/diff/1/ios/chrome/browser/ui/ntp/g...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2923803002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:251: if
(_mostVisitedData.size() > 0) {
When you remove the last time isn't this 0, so it falls thru and tells the
consumer to update?

https://codereview.chromium.org/2923803002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:385: - (void)useFreshData {
Did we also want a DCHECK to confirm that |freshMostVisitedData| is updated
after updating the blacklist?

[gambard, 2017-06-06 16:19:23]

https://codereview.chromium.org/2923803002/diff/1/ios/chrome/browser/ui/ntp/g...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2923803002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:251: if
(_mostVisitedData.size() > 0) {
On 2017/06/06 15:47:34, justincohen (OOTO til Jun 5) wrote:
> When you remove the last time isn't this 0, so it falls thru and tells the
> consumer to update?

No, _mostVisitedData is the previous data (i.e. before removal).
When it is 0, the update should go through and update the view with the new ones
even if the user does not remove a tile.

https://codereview.chromium.org/2923803002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:385: - (void)useFreshData {
On 2017/06/06 15:47:34, justincohen (OOTO til Jun 5) wrote:
> Did we also want a DCHECK to confirm that |freshMostVisitedData| is updated
> after updating the blacklist?

It seems complicated for a little gain in my opinion. Do you think it is very
important?

[justincohen, 2017-06-06 16:25:32]

LGTM

[gambard, 2017-06-07 06:49:53]

The CQ bit was checked by gambard@chromium.org

[commit-bot: I haz the power, 2017-06-07 06:50:07]

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-06-07 07:02:02]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496818193549040, "parent_rev":
"d8b13e21c0c676d7f8e075236948983d09b42bbe", "commit_rev":
"b90a205103775b54fac946c7617222ba9700b544"}

[commit-bot: I haz the power, 2017-06-07 07:02:16]

Description was changed from

==========
Make possible to remove the last most visited tile

The last Most Visited tile could not be removed in the NTP. This CL fixes it.

BUG=730033
==========

to

==========
Make possible to remove the last most visited tile

The last Most Visited tile could not be removed in the NTP. This CL fixes it.

BUG=730033

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

[commit-bot: I haz the power, 2017-06-07 07:02:17]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/b90a205103775b54fac946c76172...