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

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

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
index 988e128e8d55e5796099b99217e2d8960f9fdba0..124891addcbcc7edc0b0279a8886e755c7276649 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
@@ -17,7 +17,6 @@ import android.net.Uri;
 import android.os.Build;
 import android.support.v4.graphics.drawable.RoundedBitmapDrawable;
 import android.support.v4.graphics.drawable.RoundedBitmapDrawableFactory;
-import android.support.v7.widget.LinearLayoutManager;
 import android.support.v7.widget.RecyclerView;
 import android.text.Editable;
 import android.text.TextUtils;
@@ -73,8 +72,12 @@ public class NewTabPageView extends FrameLayout
     private static final long SNAP_SCROLL_DELAY_MS = 30;
     private static final String TAG = "NewTabPageView";
 
-    private ViewGroup mContentView;
+    // Note: Only one of these will be valid at a time, depending on if we are using the old NTP
+    // (NewTabScrollView) or the new NTP with cards (NewTabRecyclerView).
     private NewTabScrollView mScrollView;

[dgn, 2016/03/25 16:29:20]

Would it make sense to use composition or inheritance to abstract away the fact
that we have two different modes here? one of the members is always invalid, and
another member is being used to differentiate in which case we are. We even have
methods in here to hide that (getVerticalScroll)

+    private NewTabRecyclerView mRecyclerView;
+
+    private NewTabPageLayout mContentView;
     private LogoView mSearchProviderLogoView;
     private View mSearchBoxView;
     private TextView mSearchBoxTextView;
@@ -83,7 +86,7 @@ public class NewTabPageView extends FrameLayout
     private View mMostVisitedPlaceholder;
     private View mOptOutView;
     private View mNoSearchLogoSpacer;
-    private RecyclerView mSnippetsView;
+    private NewTabPageCardsManager mCardsManager;
 
     private OnSearchBoxScrollListener mSearchBoxScrollListener;
 
@@ -94,6 +97,7 @@ public class NewTabPageView extends FrameLayout
     private boolean mSearchProviderHasLogo = true;
     private boolean mHasReceivedMostVisitedSites;
     private boolean mPendingSnapScroll;
+    private boolean mAreSnippetsEnabled;
 
     /**
      * The number of asynchronous tasks that need to complete before the page is done loading.
@@ -257,18 +261,30 @@ public class NewTabPageView extends FrameLayout
     public void initialize(NewTabPageManager manager, boolean isSingleUrlBarMode,
             boolean searchProviderHasLogo, SnippetsManager snippetsManager) {
         mManager = manager;
+        ViewStub stub = (ViewStub) findViewById(R.id.new_tab_page_layout_stub);
+
+        mAreSnippetsEnabled = ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS);
 
-        mScrollView = (NewTabScrollView) findViewById(R.id.ntp_scrollview);
-        mScrollView.enableBottomShadow(SHADOW_COLOR);
-        mContentView = (ViewGroup) findViewById(R.id.ntp_content);
+        if (mAreSnippetsEnabled) {
+            stub.setLayoutResource(R.layout.new_tab_page_recyclerview);
+            mRecyclerView = (NewTabRecyclerView) stub.inflate();
+            mContentView = (NewTabPageLayout) LayoutInflater.from(getContext())
+                                   .inflate(R.layout.new_tab_page_layout, null);
+        } else {
+            stub.setLayoutResource(R.layout.new_tab_page_scrollview);
+            mScrollView = (NewTabScrollView) stub.inflate();
+            mScrollView.enableBottomShadow(SHADOW_COLOR);
+            mContentView = (NewTabPageLayout) findViewById(R.id.ntp_content);
+        }
 
         mMostVisitedDesign = new MostVisitedDesign(getContext());
-        mMostVisitedLayout = (MostVisitedLayout) findViewById(R.id.most_visited_layout);
+        mMostVisitedLayout =
+                (MostVisitedLayout) mContentView.findViewById(R.id.most_visited_layout);
         mMostVisitedDesign.initMostVisitedLayout(mMostVisitedLayout, searchProviderHasLogo);
 
-        mSearchProviderLogoView = (LogoView) findViewById(R.id.search_provider_logo);
-        mSearchBoxView = findViewById(R.id.search_box);
-        mNoSearchLogoSpacer = findViewById(R.id.no_search_logo_spacer);
+        mSearchProviderLogoView = (LogoView) mContentView.findViewById(R.id.search_provider_logo);
+        mSearchBoxView = mContentView.findViewById(R.id.search_box);
+        mNoSearchLogoSpacer = mContentView.findViewById(R.id.no_search_logo_spacer);
 
         mSearchBoxTextView = (TextView) mSearchBoxView.findViewById(R.id.search_box_text);
         String hintText = getResources().getString(R.string.search_or_type_url);
@@ -300,7 +316,7 @@ public class NewTabPageView extends FrameLayout
             }
         });
 
-        mVoiceSearchButton = (ImageView) findViewById(R.id.voice_search_button);
+        mVoiceSearchButton = (ImageView) mContentView.findViewById(R.id.voice_search_button);
         mVoiceSearchButton.setOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
@@ -336,13 +352,16 @@ public class NewTabPageView extends FrameLayout
             }
         } else {
             ((ViewGroup) toolbar.getParent()).removeView(toolbar);
-            FrameLayout.LayoutParams params =
-                    (FrameLayout.LayoutParams) mScrollView.getLayoutParams();
-            params.bottomMargin = 0;
-            mScrollView.setLayoutParams(params);
+            if (!ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS)) {

[newt (away), 2016/03/24 19:47:39]

better: if (mScrollView != null)

[dgn, 2016/03/28 21:33:02]

We have mAreSnippetsEnabled for that, right? Isn't it clearer to use it? We
probably want it to crash is somehow mScrollView turns out to be created with
enabledSnippets

+                // Only remove if we're using the old NTP view, the new one does not use a
+                // ScrollView
+                FrameLayout.LayoutParams params =
+                        (FrameLayout.LayoutParams) mScrollView.getLayoutParams();
+                params.bottomMargin = 0;
+                mScrollView.setLayoutParams(params);
+            }
         }
 
-        initializeSearchBoxScrollHandling();
         addOnLayoutChangeListener(this);
         setSearchProviderHasLogo(searchProviderHasLogo);
 
@@ -353,13 +372,13 @@ public class NewTabPageView extends FrameLayout
         if (mManager.shouldShowOptOutPromo()) showOptOutPromo();
 
         // Set up snippets
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS)) {
-            mSnippetsView = (RecyclerView) findViewById(R.id.snippets_card_list);
-            mSnippetsView.setVisibility(View.VISIBLE);
+        if (mAreSnippetsEnabled) {
+            mCardsManager = new NewTabPageCardsManager(snippetsManager, mContentView);
+            mRecyclerView.setAdapter(mCardsManager);
+
             RecordHistogram.recordEnumeratedHistogram(SnippetsManager.SNIPPETS_STATE_HISTOGRAM,
                     SnippetsManager.SNIPPETS_SHOWN, SnippetsManager.NUM_SNIPPETS_ACTIONS);
-            mSnippetsView.setLayoutManager(new LinearLayoutManager(getContext()));
-            mSnippetsView.addOnScrollListener(new RecyclerView.OnScrollListener() {
+            mRecyclerView.addOnScrollListener(new RecyclerView.OnScrollListener() {
                 private boolean mScrolledOnce = false;
                 @Override
                 public void onScrollStateChanged(RecyclerView recyclerView, int newState) {
@@ -373,7 +392,9 @@ public class NewTabPageView extends FrameLayout
                             SnippetsManager.NUM_SNIPPETS_ACTIONS);
                 }
             });
-            snippetsManager.setSnippetsView(mSnippetsView);
+            initializeSearchBoxRecyclerViewScrollHandling();
+        } else {
+            initializeSearchBoxScrollHandling();
         }
     }
 
@@ -444,10 +465,16 @@ public class NewTabPageView extends FrameLayout
         float percentage = 0;
         // During startup the view may not be fully initialized, so we only calculate the current
         // percentage if some basic view properties are sane.
-        if (mScrollView.getHeight() != 0 && mSearchBoxView.getTop() != 0) {
-            int scrollY = mScrollView.getScrollY();
-            percentage = Math.max(
-                    0f, Math.min(1f, scrollY / (float) mSearchBoxView.getTop()));
+        if (mAreSnippetsEnabled) {

[newt (away), 2016/03/24 19:47:39]

lines 468-478 need more de-duplication. The only difference between the two part
is "mRecyclerView.getHeight()" and "mScrollView.getHeight()"; extract that
difference and remove the duplication.

[dgn, 2016/03/28 21:33:02]

Done.

+            if (mRecyclerView.getHeight() != 0 && mSearchBoxView.getTop() != 0) {
+                int scrollY = getVerticalScroll();
+                percentage = Math.max(0f, Math.min(1f, scrollY / (float) mSearchBoxView.getTop()));
+            }
+        } else {
+            if (mScrollView.getHeight() != 0 && mSearchBoxView.getTop() != 0) {
+                int scrollY = getVerticalScroll();
+                percentage = Math.max(0f, Math.min(1f, scrollY / (float) mSearchBoxView.getTop()));
+            }
         }
 
         updateVisualsForToolbarTransition(percentage);
@@ -457,6 +484,15 @@ public class NewTabPageView extends FrameLayout
         }
     }
 
+    private void initializeSearchBoxRecyclerViewScrollHandling() {
+        mRecyclerView.addOnScrollListener(new RecyclerView.OnScrollListener() {
+            @Override
+            public void onScrolled(RecyclerView recyclerView, int dx, int dy) {
+                updateSearchBoxOnScroll();
+            }
+        });
+    }
+
     private void initializeSearchBoxScrollHandling() {
         final Runnable mSnapScrollRunnable = new Runnable() {
             @Override
@@ -629,8 +665,10 @@ public class NewTabPageView extends FrameLayout
      * @see #setUrlFocusChangeAnimationPercent
      */
     private void setUrlFocusChangeAnimationPercentInternal(float percent) {
-        mContentView.setTranslationY(percent * (-mMostVisitedLayout.getTop()
-                + mScrollView.getScrollY() + mContentView.getPaddingTop()));
+        int scrollOffset = mAreSnippetsEnabled ? mRecyclerView.computeVerticalScrollOffset()
+                                               : mScrollView.getScrollY();
+        mContentView.setTranslationY(percent
+                * (-mMostVisitedLayout.getTop() + scrollOffset + mContentView.getPaddingTop()));
         updateVisualsForToolbarTransition(percent);
     }
 
@@ -666,9 +704,15 @@ public class NewTabPageView extends FrameLayout
                 searchBoxY + mSearchBoxView.getHeight() - mSearchBoxView.getPaddingBottom());
 
         transformedBounds.set(originalBounds);
+
         View view = (View) mSearchBoxView.getParent();
         while (view != null) {
-            transformedBounds.offset(-view.getScrollX(), -view.getScrollY());
+            if (view instanceof RecyclerView) {
+                transformedBounds.offset(-((RecyclerView) view).computeHorizontalScrollOffset(),
+                        -((RecyclerView) view).computeVerticalScrollOffset());
+            } else {
+                transformedBounds.offset(-view.getScrollX(), -view.getScrollY());
+            }
             if (view == this) break;
             transformedBounds.offset((int) view.getX(), (int) view.getY());
             view = (View) view.getParent();
@@ -728,14 +772,8 @@ public class NewTabPageView extends FrameLayout
 
         // Make the search box and logo the same width as the most visited tiles.
         if (mMostVisitedLayout.getVisibility() != GONE) {
-            int mostVisitedWidth = MeasureSpec.makeMeasureSpec(mMostVisitedLayout.getMeasuredWidth()
-                    - mMostVisitedDesign.getMostVisitedLayoutBleed(), MeasureSpec.EXACTLY);
-            int searchBoxHeight = MeasureSpec.makeMeasureSpec(
-                    mSearchBoxView.getMeasuredHeight(), MeasureSpec.EXACTLY);
-            int logoHeight = MeasureSpec.makeMeasureSpec(
-                    mSearchProviderLogoView.getMeasuredHeight(), MeasureSpec.EXACTLY);
-            mSearchBoxView.measure(mostVisitedWidth, searchBoxHeight);
-            mSearchProviderLogoView.measure(mostVisitedWidth, logoHeight);
+            mContentView.setMostVisitedWidth(mMostVisitedLayout.getMeasuredWidth()
+                    - mMostVisitedDesign.getMostVisitedLayoutBleed());
         }

[May, 2016/03/24 07:21:20]

To whoever takes over this CL: This needs an else {
mContentView.setMostVisitedWidth(0); }

[dgn, 2016/03/25 16:29:20]

Done.

     }
 
@@ -746,10 +784,8 @@ public class NewTabPageView extends FrameLayout
     boolean shouldCaptureThumbnail() {
         if (getWidth() == 0 || getHeight() == 0) return false;
 
-        return mSnapshotMostVisitedChanged
-                || getWidth() != mSnapshotWidth
-                || getHeight() != mSnapshotHeight
-                || mScrollView.getScrollY() != mSnapshotScrollY;
+        return mSnapshotMostVisitedChanged || getWidth() != mSnapshotWidth
+                || getHeight() != mSnapshotHeight || getVerticalScroll() != mSnapshotScrollY;
     }
 
     /**
@@ -761,7 +797,7 @@ public class NewTabPageView extends FrameLayout
         ViewUtils.captureBitmap(this, canvas);
         mSnapshotWidth = getWidth();
         mSnapshotHeight = getHeight();
-        mSnapshotScrollY = mScrollView.getScrollY();
+        mSnapshotScrollY = getVerticalScroll();
         mSnapshotMostVisitedChanged = false;
     }
 
@@ -1025,4 +1061,12 @@ public class NewTabPageView extends FrameLayout
             }
         }
     }
+
+    private int getVerticalScroll() {
+        if (mAreSnippetsEnabled) {
+            return mRecyclerView.computeVerticalScrollOffset();
+        } else {
+            return mScrollView.getScrollY();
+        }
+    }
 }