chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java - Issue 2651673010: 🍝🏠 Refactor MostVisited UI management, extract it for reuse in Home.

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Issue 2651673010: 🍝🏠 Refactor MostVisited UI management, extract it for reuse in Home. (Closed)

Patch Set: Rebase. Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedLayout.java ('k') | chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

index 3a840a17d7ffd1bb17ddf178a9ef972b2cdb2afe..0e5e08b5dba7c4a0ca902a9b33575853c09a27ec 100644

--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

@@ -38,17 +38,17 @@ import org.chromium.chrome.browser.ntp.LogoBridge.LogoObserver;

import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;

import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;

import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;

-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.search_engines.TemplateUrlService;

import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrlServiceObserver;

-import org.chromium.chrome.browser.snackbar.Snackbar;

import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarController;

import org.chromium.chrome.browser.suggestions.SuggestionsMetricsReporter;

import org.chromium.chrome.browser.suggestions.SuggestionsNavigationDelegate;

import org.chromium.chrome.browser.suggestions.SuggestionsNavigationDelegateImpl;

import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegateImpl;

+import org.chromium.chrome.browser.suggestions.Tile;

+import org.chromium.chrome.browser.suggestions.TileGroup;

+import org.chromium.chrome.browser.suggestions.TileGroupDelegateImpl;

import org.chromium.chrome.browser.sync.SyncSessionsMetrics;

import org.chromium.chrome.browser.tab.EmptyTabObserver;

import org.chromium.chrome.browser.tab.Tab;

@@ -89,7 +89,6 @@ public class NewTabPage

// Key for the scroll position data that may be stored in a navigation entry.

private static final String NAVIGATION_ENTRY_SCROLL_POSITION_KEY = "NewTabPageScrollPosition";

- private static MostVisitedSites sMostVisitedSitesForTests;

private static SuggestionsSource sSuggestionsSourceForTests;

private final Tab mTab;

@@ -100,9 +99,9 @@ public class NewTabPage

private final int mThemeColor;

private final NewTabPageView mNewTabPageView;

private final NewTabPageManagerImpl mNewTabPageManager;

+ private final TileGroup.Delegate mTileGroupDelegate;

private TabObserver mTabObserver;

- private MostVisitedSites mMostVisitedSites;

private SnackbarController mMostVisitedItemRemovedController;

private LogoBridge mLogoBridge;

private boolean mSearchProviderHasLogo;

@@ -190,9 +189,8 @@ public class NewTabPage

&& (url.startsWith(UrlConstants.NTP_URL) || url.startsWith("chrome://newtab"));

}

- @VisibleForTesting

- public static void setMostVisitedSitesForTests(MostVisitedSites mostVisitedSitesForTests) {

- sMostVisitedSitesForTests = mostVisitedSitesForTests;

+ private boolean isNtpOfflinePagesEnabled() {

+ return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_OFFLINE_PAGES_FEATURE_NAME);

}

@VisibleForTesting

@@ -208,10 +206,6 @@ public class NewTabPage

super(suggestionsSource, metricsReporter, navigationDelegate, profile, currentTab);

}

- private boolean isNtpOfflinePagesEnabled() {

- return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_OFFLINE_PAGES_FEATURE_NAME);

- }

@Override

public boolean isLocationBarShownInNTP() {

if (mIsDestroyed) return false;

@@ -230,44 +224,6 @@ public class NewTabPage

return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_FAKE_OMNIBOX_TEXT);

}

- private void recordOpenedMostVisitedItem(MostVisitedItem item) {

- if (mIsDestroyed) return;

- NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_MOST_VISITED_TILE);

- RecordUserAction.record("MobileNTPMostVisited");

- NewTabPageUma.recordExplicitUserNavigation(

- item.getUrl(), NewTabPageUma.RAPPOR_ACTION_VISITED_SUGGESTED_TILE);

- RecordHistogram.recordMediumTimesHistogram("NewTabPage.MostVisitedTime",

- System.nanoTime() - mLastShownTimeNs, TimeUnit.NANOSECONDS);

- mMostVisitedSites.recordOpenedMostVisitedItem(

- item.getIndex(), item.getTileType(), item.getSource());

- }

- @Override

- public void openMostVisitedItem(int windowDisposition, MostVisitedItem item) {

- if (mIsDestroyed) return;

- String url = item.getUrl();

- // TODO(treib): Should we call recordOpenedMostVisitedItem here?

- if (windowDisposition != WindowOpenDisposition.NEW_WINDOW) {

- recordOpenedMostVisitedItem(item);

- }

- if (windowDisposition == WindowOpenDisposition.CURRENT_TAB

- && switchToExistingTab(url)) {

- return;

- }

- getNavigationDelegate().openUrl(

- windowDisposition, new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK));

- }

- @Override

- public void removeMostVisitedItem(MostVisitedItem item) {

- mMostVisitedSites.addBlacklistedUrl(item.getUrl());

- showMostVisitedItemRemovedSnackbar(item.getUrl());

- }

@TargetApi(Build.VERSION_CODES.LOLLIPOP)

private boolean switchToExistingTab(String url) {

String matchPattern = CommandLine.getInstance().getSwitchValue(

@@ -309,12 +265,6 @@ public class NewTabPage

}

@Override

- public void setMostVisitedURLsObserver(MostVisitedURLsObserver observer, int numResults) {

- if (mIsDestroyed) return;

- mMostVisitedSites.setMostVisitedURLsObserver(observer, numResults);

- }

- @Override

public void onLogoClicked(boolean isAnimatedLogoShowing) {

if (mIsDestroyed) return;

@@ -355,9 +305,42 @@ public class NewTabPage

}

@Override

- public void onLoadingComplete(MostVisitedItem[] items) {

+ public SuggestionsSource getSuggestionsSource() {

+ if (sSuggestionsSourceForTests != null) return sSuggestionsSourceForTests;

+ return mSnippetsBridge;

+ }

+ @Override

+ public boolean isCurrentPage() {

+ if (mIsDestroyed) return false;

+ if (mFakeboxDelegate == null) return false;

+ return mFakeboxDelegate.isCurrentPage(NewTabPage.this);

+ }

+ @Override

+ public ContextMenuManager getContextMenuManager() {

+ assert !mIsDestroyed;

+ return mNewTabPageView.getContextMenuManager();

+ }

+ /**

+ * Extends {@link TileGroupDelegateImpl} to add metrics logging that is specific to

+ * {@link NewTabPage}.

+ */

+ private class NewTabPageTileGroupDelegate extends TileGroupDelegateImpl {

+ private NewTabPageTileGroupDelegate(Context context, Tab tab,

+ TabModelSelector tabModelSelector,

+ SuggestionsNavigationDelegate navigationDelegate) {

+ super(context, tab, tabModelSelector, navigationDelegate);

+ }

+ @Override

+ public void onLoadingComplete(Tile[] items) {

if (mIsDestroyed) return;

+ super.onLoadingComplete(items);

long loadTimeMs = (System.nanoTime() - mConstructedTimeNs) / 1000000;

RecordHistogram.recordTimesHistogram(

"Tab.NewTabOnload", loadTimeMs, TimeUnit.MILLISECONDS);

@@ -367,18 +350,6 @@ public class NewTabPage

// If not visible when loading completes, wait until onShown is received.

if (!mTab.isHidden()) recordNTPShown();

- 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);

if (isNtpOfflinePagesEnabled()) {

final int maxNumTiles = 12;

for (int i = 0; i < items.length; i++) {

@@ -392,22 +363,15 @@ public class NewTabPage

}

@Override

- public SuggestionsSource getSuggestionsSource() {

- if (sSuggestionsSourceForTests != null) return sSuggestionsSourceForTests;

- return mSnippetsBridge;

- }

+ public void openMostVisitedItem(int windowDisposition, Tile tile) {

+ if (mIsDestroyed) return;

- @Override

- public boolean isCurrentPage() {

- if (mIsDestroyed) return false;

- if (mFakeboxDelegate == null) return false;

- return mFakeboxDelegate.isCurrentPage(NewTabPage.this);

- }

+ super.openMostVisitedItem(windowDisposition, tile);

- @Override

- public ContextMenuManager getContextMenuManager() {

- assert !mIsDestroyed;

- return mNewTabPageView.getContextMenuManager();

+ if (windowDisposition != WindowOpenDisposition.NEW_WINDOW) {

+ RecordHistogram.recordMediumTimesHistogram("NewTabPage.MostVisitedTime",

+ System.nanoTime() - mLastShownTimeNs, TimeUnit.NANOSECONDS);

+ }

}

@@ -431,6 +395,8 @@ public class NewTabPage

new SuggestionsNavigationDelegateImpl(activity, profile, tab, tabModelSelector);

mNewTabPageManager = new NewTabPageManagerImpl(

mSnippetsBridge, mSnippetsBridge, navigationDelegate, profile, tab);

+ mTileGroupDelegate = new NewTabPageTileGroupDelegate(

+ activity, tab, tabModelSelector, navigationDelegate);

mTitle = activity.getResources().getString(R.string.button_new_tab);

mBackgroundColor = ApiCompatibilityUtils.getColor(activity.getResources(), R.color.ntp_bg);

@@ -474,14 +440,13 @@ public class NewTabPage

}

};

mTab.addObserver(mTabObserver);

- mMostVisitedSites = buildMostVisitedSites(profile);

mLogoBridge = new LogoBridge(profile);

updateSearchProviderHasLogo();

LayoutInflater inflater = LayoutInflater.from(activity);

mNewTabPageView = (NewTabPageView) inflater.inflate(R.layout.new_tab_page_view, null);

- mNewTabPageView.initialize(mNewTabPageManager, mTab, mSearchProviderHasLogo,

- getScrollPositionFromNavigationEntry());

+ mNewTabPageView.initialize(mNewTabPageManager, mTab, mTileGroupDelegate,

+ mSearchProviderHasLogo, getScrollPositionFromNavigationEntry());

if (mSnippetsBridge != null) {

mSnippetsBridge.onNtpInitialized();

@@ -496,38 +461,6 @@ public class NewTabPage

TraceEvent.end(TAG);

}

- private static MostVisitedSites buildMostVisitedSites(Profile profile) {

- if (sMostVisitedSitesForTests != null) {

- return sMostVisitedSitesForTests;

- } else {

- return new MostVisitedSites(profile);

- }

- private void showMostVisitedItemRemovedSnackbar(String url) {

- if (mMostVisitedItemRemovedController == null) {

- mMostVisitedItemRemovedController = new SnackbarController() {

- @Override

- public void onDismissNoAction(Object actionData) {}

- /** Undoes the most visited item removal. */

- @Override

- public void onAction(Object actionData) {

- if (mIsDestroyed) return;

- String url = (String) actionData;

- mMostVisitedSites.removeBlacklistedUrl(url);

- }

- };

- }

- Context context = mNewTabPageView.getContext();

- Snackbar snackbar = Snackbar

- .make(context.getString(R.string.most_visited_item_removed),

- mMostVisitedItemRemovedController, Snackbar.TYPE_ACTION,

- Snackbar.UMA_NTP_MOST_VISITED_DELETE_UNDO)

- .setAction(context.getString(R.string.undo), url);

- mTab.getSnackbarManager().showSnackbar(snackbar);

- }

/** @return The view container for the new tab page. */

@VisibleForTesting

public NewTabPageView getNewTabPageView() {

@@ -690,10 +623,6 @@ public class NewTabPage

.isAttachedToWindow(getView()) : "Destroy called before removed from window";

if (mIsLoaded && !mTab.isHidden()) recordNTPInteractionTime();

- if (mMostVisitedSites != null) {

- mMostVisitedSites.destroy();

- mMostVisitedSites = null;

- }

if (mLogoBridge != null) {

mLogoBridge.destroy();

mLogoBridge = null;

@@ -706,6 +635,7 @@ public class NewTabPage

mTab.getSnackbarManager().dismissSnackbars(mMostVisitedItemRemovedController);

}

mNewTabPageManager.onDestroy();

+ mTileGroupDelegate.destroy();

TemplateUrlService.getInstance().removeObserver(this);

mTab.removeObserver(mTabObserver);

mTabObserver = null;