🍝🏠 Refactor MostVisited UI management, extract it for reuse in Home.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroupDelegateImpl.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroupDelegateImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroupDelegateImpl.java
new file mode 100644
index 0000000000000000000000000000000000000000..556eae2b8fa7e6d073e8fd4a31b6769dc81c70d5
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroupDelegateImpl.java
@@ -0,0 +1,181 @@
+// 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.annotation.TargetApi;
+import android.content.Context;
+import android.os.Build;
+
+import org.chromium.base.CommandLine;
+import org.chromium.base.VisibleForTesting;
+import org.chromium.base.metrics.RecordUserAction;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.ntp.NewTabPageUma;
+import org.chromium.chrome.browser.profiles.MostVisitedSites;
+import org.chromium.chrome.browser.profiles.MostVisitedSites.MostVisitedURLsObserver;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.snackbar.Snackbar;
+import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarController;
+import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tabmodel.TabModel;
+import org.chromium.chrome.browser.tabmodel.TabModelSelector;
+import org.chromium.chrome.browser.tabmodel.TabModelUtils;
+import org.chromium.chrome.browser.util.UrlUtilities;
+import org.chromium.content_public.browser.LoadUrlParams;
+import org.chromium.ui.base.PageTransition;
+import org.chromium.ui.mojom.WindowOpenDisposition;
+
+/**
+ * Implements {@link TileGroup.Delegate}.
+ */
+public class TileGroupDelegateImpl implements TileGroup.Delegate {
+    private static MostVisitedSites sMostVisitedSitesForTests;
+
+    private final Context mContext;
+    private final Tab mTab;
+    private final TabModelSelector mTabModelSelector;
+    private final SuggestionsNavigationDelegate mNavigationDelegate;
+    private final MostVisitedSites mMostVisitedSites;
+
+    private boolean mIsDestroyed;
+    private SnackbarController mTileRemovedSnackbarController;
+
+    public TileGroupDelegateImpl(Context context, Tab tab, TabModelSelector tabModelSelector,
+            SuggestionsNavigationDelegate navigationDelegate) {
+        mContext = context;
+        mTab = tab;
+        mTabModelSelector = tabModelSelector;
+        mNavigationDelegate = navigationDelegate;
+        mMostVisitedSites = buildMostVisitedSites(tab.getProfile());
+    }
+
+    @Override
+    public void removeMostVisitedItem(Tile item) {

[dgn, 2017/01/31 18:21:35]

assert isdestroyed?

[Michael van Ouwerkerk, 2017/02/03 12:37:48]

Done.

+        mMostVisitedSites.addBlacklistedUrl(item.getUrl());
+        showTileRemovedSnackbar(item.getUrl());
+    }
+
+    @Override
+    public void openMostVisitedItem(int windowDisposition, Tile item) {
+        if (mIsDestroyed) return;
+
+        String url = item.getUrl();
+
+        // TODO(treib): Should we call recordOpenedMostVisitedItem here?
+        if (windowDisposition != WindowOpenDisposition.NEW_WINDOW) {
+            recordOpenedTile(item);
+        }
+
+        if (windowDisposition == WindowOpenDisposition.CURRENT_TAB && switchToExistingTab(url)) {
+            return;
+        }
+
+        mNavigationDelegate.openUrl(
+                windowDisposition, new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK));
+    }
+
+    @Override
+    public void setMostVisitedURLsObserver(MostVisitedURLsObserver observer, int maxResults) {
+        assert !mIsDestroyed;
+        mMostVisitedSites.setMostVisitedURLsObserver(observer, maxResults);
+    }
+
+    @Override
+    public void onLoadingComplete(Tile[] items) {

[dgn, 2017/01/31 18:21:35]

assert isDestroyed?

[Michael van Ouwerkerk, 2017/02/03 12:37:48]

Done.

+        int tileTypes[] = new int[items.length];
+        int sources[] = new int[items.length];
+        String tileUrls[] = new String[items.length];
+
+        for (int i = 0; i < items.length; i++) {
+            tileTypes[i] = items[i].getTileType();
+            sources[i] = items[i].getSource();
+            tileUrls[i] = items[i].getUrl();
+        }
+
+        mMostVisitedSites.recordPageImpression(tileTypes, sources, tileUrls);
+    }
+
+    @Override
+    public void destroy() {
+        assert !mIsDestroyed;
+        mIsDestroyed = true;
+
+        mMostVisitedSites.destroy();
+    }
+
+    private static MostVisitedSites buildMostVisitedSites(Profile profile) {
+        if (sMostVisitedSitesForTests != null) {
+            return sMostVisitedSitesForTests;
+        } else {
+            return new MostVisitedSites(profile);
+        }
+    }
+
+    @VisibleForTesting
+    public static void setMostVisitedSitesForTests(MostVisitedSites mostVisitedSitesForTests) {
+        sMostVisitedSitesForTests = mostVisitedSitesForTests;
+    }
+
+    private void showTileRemovedSnackbar(String url) {
+        if (mTileRemovedSnackbarController == null) {
+            mTileRemovedSnackbarController = new SnackbarController() {
+                @Override
+                public void onDismissNoAction(Object actionData) {}
+
+                /** Undoes the tile removal. */
+                @Override
+                public void onAction(Object actionData) {
+                    if (mIsDestroyed) return;
+                    String url = (String) actionData;
+                    mMostVisitedSites.removeBlacklistedUrl(url);
+                }
+            };
+        }
+        Snackbar snackbar = Snackbar.make(mContext.getString(R.string.most_visited_item_removed),
+                                            mTileRemovedSnackbarController, Snackbar.TYPE_ACTION,
+                                            Snackbar.UMA_NTP_MOST_VISITED_DELETE_UNDO)
+                                    .setAction(mContext.getString(R.string.undo), url);
+        mTab.getSnackbarManager().showSnackbar(snackbar);
+    }
+
+    private void recordOpenedTile(Tile tile) {
+        // TODO(mvanouwerkerk): Fix metrics to distinguish NTP from Home.
+        NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_MOST_VISITED_TILE);
+        RecordUserAction.record("MobileNTPMostVisited");
+        NewTabPageUma.recordExplicitUserNavigation(
+                tile.getUrl(), NewTabPageUma.RAPPOR_ACTION_VISITED_SUGGESTED_TILE);
+        mMostVisitedSites.recordOpenedMostVisitedItem(
+                tile.getIndex(), tile.getTileType(), tile.getSource());
+    }
+
+    @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+    private boolean switchToExistingTab(String url) {
+        String matchPattern =
+                CommandLine.getInstance().getSwitchValue(ChromeSwitches.NTP_SWITCH_TO_EXISTING_TAB);

[dgn, 2017/01/31 18:21:35]

That flag was added for an M49 experiment. Can we remove it now?

https://bugs.chromium.org/p/chromium/issues/detail?id=565953

File a new bug or update the old one to prioritise this?

[Michael van Ouwerkerk, 2017/02/03 12:37:48]

I've updated the bug.

+        boolean matchByHost;
+        if ("url".equals(matchPattern)) {
+            matchByHost = false;
+        } else if ("host".equals(matchPattern)) {
+            matchByHost = true;
+        } else {
+            return false;
+        }
+
+        TabModel tabModel = mTabModelSelector.getModel(false);
+        for (int i = tabModel.getCount() - 1; i >= 0; --i) {
+            if (matchURLs(tabModel.getTabAt(i).getUrl(), url, matchByHost)) {
+                TabModelUtils.setIndex(tabModel, i);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static boolean matchURLs(String url1, String url2, boolean matchByHost) {
+        if (url1 == null || url2 == null) return false;
+        return matchByHost ? UrlUtilities.sameHost(url1, url2) : url1.equals(url2);
+    }
+}