📰 Refresh the tiles when a removal is undone

chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.suggestions;
 
 import android.content.Context;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.graphics.Color;
 import android.graphics.drawable.BitmapDrawable;
 import android.support.annotation.Nullable;
 import android.support.v4.graphics.drawable.RoundedBitmapDrawable;
 import android.support.v4.graphics.drawable.RoundedBitmapDrawableFactory;
 import android.view.ContextMenu;
 import android.view.ContextMenu.ContextMenuInfo;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.View.OnCreateContextMenuListener;
 import android.view.ViewGroup;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.Callback;
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.favicon.LargeIconBridge.LargeIconCallback;
 import org.chromium.chrome.browser.ntp.ContextMenuManager;
 import org.chromium.chrome.browser.ntp.ContextMenuManager.ContextMenuItemId;
 import org.chromium.chrome.browser.ntp.MostVisitedTileType;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.widget.RoundedIconGenerator;
 import org.chromium.ui.mojom.WindowOpenDisposition;
 
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
 /**
  * The model and controller for a group of site suggestion tiles.
  */
 public class TileGroup implements MostVisitedSites.Observer {
     /**
      * Performs work in other parts of the system that the {@link TileGroup} should not know about.
      */
     public interface Delegate {
-        void removeMostVisitedItem(Tile tile);
+        /**
+         * @param tile The tile corresponding to the most visited item to remove.
+         * @param removalUndoneCallback The callback to invoke if the removal is reverted.
+         */
+        void removeMostVisitedItem(Tile tile, @Nullable Callback<String> removalUndoneCallback);

[Michael van Ouwerkerk, 2017/03/17 15:53:07]

Why is the callback nullable? Please document what the callback string argument
represents.

[dgn, 2017/03/17 16:13:45]

I intended the callback to be optional but there is only one user anyway so
there is no point. Removed Nullable. Added explanation about the argument.

 
         void openMostVisitedItem(int windowDisposition, Tile tile);
 
         /**
          * Gets the list of most visited sites.
          * @param observer The observer to be notified with the list of sites.
          * @param maxResults The maximum number of sites to retrieve.
          */
         void setMostVisitedSitesObserver(MostVisitedSites.Observer observer, int maxResults);
 
@@ skipping 84 matching lines @@
     @Nullable
     private Tile[] mPendingTileData;
 
     /**
      * URL of the most recently removed tile. Used to identify when a tile removal is confirmed by
      * the tile backend.
      */
     @Nullable
     private String mPendingRemovalUrl;
 
+    /**
+     * URL of the most recently added tile. Used to identify when a given tile's insertion is
+     * confirmed by the tile backend. This is relevant when a previously existing tile is removed,
+     * then the user undoes the action and wants that tile back.
+     */
+    @Nullable
+    private String mPendingInsertionUrl;
+
     private boolean mHasReceivedData;
 
     /**
      * @param context Used for initialisation and resolving resources.
      * @param uiDelegate Delegate used to interact with the rest of the system.
      * @param contextMenuManager Used to handle context menu invocations on the tiles.
      * @param tileGroupDelegate Used for interactions with the Most Visited backend.
      * @param observer Will be notified of changes to the tile data.
      * @param titleLines The number of text lines to use for each tile title.
      */
@@ skipping 23 matching lines @@
             mUiDelegate.addDestructionObserver(mOfflineModelObserver);
         } else {
             mOfflineModelObserver = null;
         }
     }
 
     @Override
     public void onMostVisitedURLsAvailable(final String[] titles, final String[] urls,
             final String[] whitelistIconPaths, final int[] sources) {
         boolean removalCompleted = mPendingRemovalUrl != null;
+        boolean insertionCompleted = mPendingInsertionUrl == null;
+
         Set<String> addedUrls = new HashSet<>();
         mPendingTileData = new Tile[titles.length];
         for (int i = 0; i < titles.length; i++) {
             assert urls[i] != null; // We assume everywhere that the url is not null.
 
             // TODO(dgn): Add UMA to track the cause of https://crbug.com/690926. Checking this
             // should not even be necessary as the backend is supposed to send non dupes URLs.
             if (addedUrls.contains(urls[i])) {
                 assert false : "Incoming NTP Tiles are not unique. Dupe on " + urls[i];
                 continue;
             }
 
             mPendingTileData[i] =
                     new Tile(titles[i], urls[i], whitelistIconPaths[i], i, sources[i]);
             addedUrls.add(urls[i]);
 
             if (urls[i].equals(mPendingRemovalUrl)) removalCompleted = false;
+            if (urls[i].equals(mPendingInsertionUrl)) insertionCompleted = true;
         }
 
-        if (removalCompleted) mPendingRemovalUrl = null;
-        if (!mHasReceivedData || !mUiDelegate.isVisible() || removalCompleted) loadTiles();
+        boolean expectedChangeCompleted = false;
+        if (mPendingRemovalUrl != null && removalCompleted) {
+            mPendingRemovalUrl = null;
+            expectedChangeCompleted = true;
+        }
+        if (mPendingInsertionUrl != null && insertionCompleted) {
+            mPendingInsertionUrl = null;
+            expectedChangeCompleted = true;
+        }
+
+        if (!mHasReceivedData || !mUiDelegate.isVisible() || expectedChangeCompleted) loadTiles();
     }
 
     @Override
     public void onIconMadeAvailable(String siteUrl) {
         Tile tile = getTile(siteUrl);
         if (tile == null) return; // The tile might have been removed.
 
         LargeIconCallback iconCallback =
                 new LargeIconCallbackImpl(siteUrl, /* trackLoadTask = */ false);
         mUiDelegate.getLargeIconForUrl(siteUrl, mMinIconSize, iconCallback);
@@ skipping 199 matching lines @@
         }
 
         @Override
         public void removeItem() {
             Tile tile = getTile(mUrl);
             if (tile == null) return;
 
             // Note: This does not track all the removals, but will track the most recent one. If
             // that removal is committed, it's good enough for change detection.
             mPendingRemovalUrl = mUrl;
-            mTileGroupDelegate.removeMostVisitedItem(tile);
+            mTileGroupDelegate.removeMostVisitedItem(tile, new RemovalUndoneCallback());
         }
 
         @Override
         public String getUrl() {
             return mUrl;
         }
 
         @Override
         public boolean isItemSupported(@ContextMenuItemId int menuItemId) {
             return true;
         }
 
         @Override
         public void onContextMenuCreated() {}
 
         @Override
         public void onCreateContextMenu(
                 ContextMenu contextMenu, View view, ContextMenuInfo contextMenuInfo) {
             mContextMenuManager.createContextMenu(contextMenu, view, this);
         }
     }
 
+    private class RemovalUndoneCallback extends Callback<String> {
+        @Override
+        public void onResult(String result) {
+            mPendingInsertionUrl = result;
+        }
+    }
+
     private class OfflineModelObserver extends SuggestionsOfflineModelObserver<Tile> {
         public OfflineModelObserver(OfflinePageBridge bridge) {
             super(bridge);
         }
 
         @Override
         public void onSuggestionOfflineIdChanged(Tile suggestion, @Nullable Long id) {
             // Retrieve a tile from the internal data, to make sure we don't update a stale object.
             Tile tile = getTile(suggestion.getUrl());
             if (tile == null) return;
 
             boolean oldOfflineAvailable = tile.isOfflineAvailable();
             tile.setOfflinePageOfflineId(id);
 
             // Only notify to update the view if there will be a visible change.
             if (oldOfflineAvailable == tile.isOfflineAvailable()) return;
             mObserver.onTileOfflineBadgeVisibilityChanged(tile);
         }
 
         @Override
         public Iterable<Tile> getOfflinableSuggestions() {
             return Arrays.asList(mTiles);
         }
     }
 }