Add new NTP layout with snippet cards and hide it behind a flag

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

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 b2c873350c55da5307802d9b9de669108249bd72..8d68a00e104b2bdcdeb1f6d358814ecddba627fe 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
@@ -46,7 +46,7 @@
 import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
 import org.chromium.chrome.browser.ntp.interests.InterestsPage;
 import org.chromium.chrome.browser.ntp.interests.InterestsPage.InterestsClickListener;
-import org.chromium.chrome.browser.ntp.snippets.SnippetsManager;
+import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.profiles.MostVisitedSites;
@@ -113,7 +113,7 @@
     private String mAnimatedLogoUrl;
     private FakeboxDelegate mFakeboxDelegate;
     private OfflinePageBridge mOfflinePageBridge;
-    private SnippetsManager mSnippetsManager;
+    private SnippetsBridge mSnippetsBridge;
 
     // The timestamp at which the constructor was called.
     private final long mConstructedTimeNs;
@@ -238,7 +238,8 @@ public boolean isInterestsEnabled() {
 
         @Override
         public boolean isToolbarEnabled() {
-            return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_TOOLBAR);
+            return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_TOOLBAR)
+                    && !ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS);
         }
 
         private void recordOpenedMostVisitedItem(MostVisitedItem item) {
@@ -525,7 +526,9 @@ public NewTabPage(Activity activity, Tab tab, TabModelSelector tabModelSelector)
         mProfile = tab.getProfile();
 
         mTitle = activity.getResources().getString(R.string.button_new_tab);
-        mBackgroundColor = ApiCompatibilityUtils.getColor(activity.getResources(), R.color.ntp_bg);
+        mBackgroundColor = ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS)
+                ? ApiCompatibilityUtils.getColor(activity.getResources(), R.color.ntp_with_cards_bg)
+                : ApiCompatibilityUtils.getColor(activity.getResources(), R.color.ntp_bg);
         mThemeColor = ApiCompatibilityUtils.getColor(
                 activity.getResources(), R.color.default_primary_color);
         TemplateUrlService.getInstance().addObserver(this);
@@ -549,12 +552,14 @@ public void onHidden(Tab tab) {
         mLogoBridge = new LogoBridge(mProfile);
         updateSearchProviderHasLogo();
 
-        mSnippetsManager = new SnippetsManager(mNewTabPageManager, mProfile);
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS)) {
+            mSnippetsBridge = new SnippetsBridge(mProfile);
+        }
 
         LayoutInflater inflater = LayoutInflater.from(activity);
         mNewTabPageView = (NewTabPageView) inflater.inflate(R.layout.new_tab_page, null);
         mNewTabPageView.initialize(mNewTabPageManager, isInSingleUrlBarMode(activity),
-                mSearchProviderHasLogo, mSnippetsManager);
+                mSearchProviderHasLogo, mSnippetsBridge);
 
         RecordHistogram.recordBooleanHistogram(
                 "NewTabPage.MobileIsUserOnline", NetworkChangeNotifier.isOnline());
@@ -611,12 +616,7 @@ private boolean isInSingleUrlBarMode(Context context) {
     }
 
     private void updateSearchProviderHasLogo() {
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS)) {
-            mSearchProviderHasLogo = false;
-            if (mNewTabPageView != null) mNewTabPageView.setSearchProviderHasLogo(false);
-        } else {
-            mSearchProviderHasLogo = TemplateUrlService.getInstance().isDefaultSearchEngineGoogle();
-        }
+        mSearchProviderHasLogo = TemplateUrlService.getInstance().isDefaultSearchEngineGoogle();
     }
 
     private void onSearchEngineUpdated() {
@@ -712,6 +712,9 @@ public void destroy() {
         assert !mIsDestroyed;
         assert getView().getParent() == null : "Destroy called before removed from window";
         if (mIsVisible) recordNTPInteractionTime();
+
+        mNewTabPageView.onPageDestroyed();

[newt (away), 2016/03/31 04:16:22]

Hmmm. In retrospect, I don't actually like the idea of involving NewTabPageView
in lifecycle events. So far there's been a clean separation between the page's
creation-destruction lifecycle and the all the view logic, and I think it makes
sense to keep it that way.

I'd suggest this:
 - NewTabPage owns the SnippetsBridge and doesn't expose it to any other class
 - Add a method "setSnippetsObserver()" to NewTabPageManager (similar to
setMostVisitedURLsObserver())
 - Instead of passing SnippetsBridge to NewTabPageAdapter, NewTabPageAdapter
will call NewTabPageManager.setSnippetsObserver() to register itself as an
observer.

This makes lifetime management simple, and parallel to the lifetime management
of all the bridge/helper classes.

[dgn, 2016/03/31 13:38:31]

Thanks, Done. The manager is always a valid reference so I removed the adapter's
unregistration. The bridge clears its reference in destroy() anyway.

+
         if (mOfflinePageBridge != null) {
             mOfflinePageBridge = null;
         }
@@ -731,9 +734,9 @@ public void destroy() {
             mLogoBridge.destroy();
             mLogoBridge = null;
         }
-        if (mSnippetsManager != null) {
-            mSnippetsManager.destroy();
-            mSnippetsManager = null;
+        if (mSnippetsBridge != null) {
+            mSnippetsBridge.destroy();
+            mSnippetsBridge = null;
         }
         if (mMostVisitedItemRemovedController != null) {
             mTab.getSnackbarManager().dismissSnackbars(mMostVisitedItemRemovedController);