Refresh the Most Visited tiles only on user interaction

ios/chrome/browser/ui/ntp/google_landing_mediator.mm

Index: ios/chrome/browser/ui/ntp/google_landing_mediator.mm
diff --git a/ios/chrome/browser/ui/ntp/google_landing_mediator.mm b/ios/chrome/browser/ui/ntp/google_landing_mediator.mm
index d93c60f241ac4b4d0f5bc1e1c2959463121e907a..47ae5c43e87f33a41bef14c82233d5e9dc833fc3 100644
--- a/ios/chrome/browser/ui/ntp/google_landing_mediator.mm
+++ b/ios/chrome/browser/ui/ntp/google_landing_mediator.mm
@@ -106,8 +106,7 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 
   std::unique_ptr<ntp_tiles::MostVisitedSites> _mostVisitedSites;
 
-  // Most visited data from the MostVisitedSites service (copied upon receiving
-  // the callback).
+  // Most visited data from the MostVisitedSites service currently in use.
   ntp_tiles::NTPTilesVector _mostVisitedData;
 
   // Observes the WebStateList so that this mediator can update the UI when the
@@ -127,9 +126,17 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 // The dispatcher for this mediator.
 @property(nonatomic, assign) id<ChromeExecuteCommand, UrlLoader> dispatcher;
 
+// Most visited data from the MostVisitedSites service (copied upon receiving
+// the callback), not yet used.
+@property(nonatomic, assign) ntp_tiles::NTPTilesVector freshMostVisitedData;
+
 // Perform initial setup.
 - (void)setUp;
 
+// If there is some fresh most visited tiles, they become the current tiles and
+// the consumer gets notified.
+- (void)useFreshData;
+
 @end
 
 @implementation GoogleLandingMediator
@@ -137,6 +144,7 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 @synthesize consumer = _consumer;
 @synthesize dispatcher = _dispatcher;
 @synthesize webStateList = _webStateList;
+@synthesize freshMostVisitedData = _freshMostVisitedData;
 
 - (instancetype)initWithConsumer:(id<GoogleLandingConsumer>)consumer
                     browserState:(ios::ChromeBrowserState*)browserState
@@ -240,6 +248,13 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 #pragma mark - MostVisitedSitesObserving
 
 - (void)onMostVisitedURLsAvailable:(const ntp_tiles::NTPTilesVector&)data {
+  if (_mostVisitedData.size() > 0) {
+    // If some content is already displayed to the user, do not update it to
+    // prevent updating the all the tiles without any action from the user.
+    self.freshMostVisitedData = data;
+    return;
+  }
+
   _mostVisitedData = data;
   [self.consumer mostVisitedDataUpdated];
 
@@ -299,10 +314,12 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 
 - (void)addBlacklistedURL:(const GURL&)url {
   _mostVisitedSites->AddOrRemoveBlacklistedUrl(url, true);
+  [self useFreshData];
 }
 
 - (void)removeBlacklistedURL:(const GURL&)url {
   _mostVisitedSites->AddOrRemoveBlacklistedUrl(url, false);
+  [self useFreshData];
 }
 
 - (ntp_tiles::NTPTile)mostVisitedAtIndex:(NSUInteger)index {
@@ -363,4 +380,15 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
   NOTREACHED();
 }
 
+#pragma mark - Private
+
+- (void)useFreshData {
+  if (self.freshMostVisitedData.size() == 0) {
+    return;

[justincohen, 2017/05/24 15:58:21]

should this NOTREACHED?

[gambard, 2017/06/02 12:47:04]

No. If you remove a tile, all the tiles are not necessarily updated. Why do you
think not REACHED would be good?

[justincohen, 2017/06/05 15:18:59]

When would removing a tile not call -onMostVisitedURLsAvailable?

It seems -useFreshData is only called after changing the blacklist, which should
always fire 
-onMostVisitedURLsAvailable: and set freshMostVisitedData.  I was suggesting
that if useFreshData is called without freshMostVisitedData being set, something
is wrong.

If you only have 1 tile, and you add it to the blacklist, freshMostVisitedData
won't get updated (if (_mostVisitedData.size() > 0))... so maybe it's not worth
it.

[gambard, 2017/06/06 15:34:54]

Actually this code do not remove the last tile. I will modify it, adding the
NOTREACHED and giving the possibility to remove the last tile.

+  }
+  _mostVisitedData = self.freshMostVisitedData;
+  self.freshMostVisitedData.clear();
+  [self.consumer mostVisitedDataUpdated];
+}
+
 @end