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 e2b074a2d2f9a38bbbe412a1bdbaf4973062b053..b6d826b0aad4a70d204cfed89729c1125f8f682c 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
@@ -16,7 +16,6 @@
 import android.net.Uri;
 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;
@@ -34,10 +33,8 @@
 
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
-import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.favicon.FaviconHelper.FaviconImageCallback;
 import org.chromium.chrome.browser.favicon.FaviconHelper.IconAvailabilityCallback;
 import org.chromium.chrome.browser.favicon.LargeIconBridge.LargeIconCallback;
@@ -45,7 +42,9 @@
 import org.chromium.chrome.browser.ntp.LogoBridge.LogoObserver;
 import org.chromium.chrome.browser.ntp.MostVisitedItem.MostVisitedItemManager;
 import org.chromium.chrome.browser.ntp.NewTabPage.OnSearchBoxScrollListener;
-import org.chromium.chrome.browser.ntp.snippets.SnippetsManager;
+import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
+import org.chromium.chrome.browser.ntp.v2.NewTabPageAdapter;
+import org.chromium.chrome.browser.ntp.v2.NewTabPageRecyclerView;
 import org.chromium.chrome.browser.profiles.MostVisitedSites.MostVisitedURLsObserver;
 import org.chromium.chrome.browser.profiles.MostVisitedSites.ThumbnailCallback;
 import org.chromium.chrome.browser.util.ViewUtils;
@@ -64,8 +63,12 @@
     private static final long SNAP_SCROLL_DELAY_MS = 30;
     private static final String TAG = "NewTabPageView";
 
-    private ViewGroup mContentView;
-    private NewTabScrollView mScrollView;
+    // Note: Only one of these will be valid at a time, depending on if we are using the old NTP
+    // (NewTabPageScrollView) or the new NTP with cards (NewTabPageRecyclerView).
+    private NewTabPageScrollView mScrollView;
+    private NewTabPageRecyclerView mRecyclerView;
+
+    private NewTabPageLayout mContentView;
     private LogoView mSearchProviderLogoView;
     private View mSearchBoxView;
     private TextView mSearchBoxTextView;
@@ -73,7 +76,7 @@
     private MostVisitedLayout mMostVisitedLayout;
     private View mMostVisitedPlaceholder;
     private View mNoSearchLogoSpacer;
-    private RecyclerView mSnippetsView;
+    private NewTabPageAdapter mCardsManager;

[newt (away), 2016/03/30 19:03:50]

update variable name

[dgn, 2016/03/31 00:46:45]

Done.

 
     private OnSearchBoxScrollListener mSearchBoxScrollListener;
 
@@ -84,6 +87,7 @@
     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.
@@ -236,20 +240,28 @@ public NewTabPageView(Context context, AttributeSet attrs) {
      * @param searchProviderHasLogo Whether the search provider has a logo.
      */
     public void initialize(NewTabPageManager manager, boolean isSingleUrlBarMode,
-            boolean searchProviderHasLogo, SnippetsManager snippetsManager) {
+            boolean searchProviderHasLogo, SnippetsBridge snippetsBridge) {

[newt (away), 2016/03/30 19:03:50]

add javadoc for this param, and mention that passing in null will revert to the
old NTP layout

[dgn, 2016/03/31 00:46:44]

Done.

         mManager = manager;
 
-        mScrollView = (NewTabScrollView) findViewById(R.id.ntp_scrollview);
-        mScrollView.enableBottomShadow(SHADOW_COLOR);
-        mContentView = (ViewGroup) findViewById(R.id.ntp_content);
+        mAreSnippetsEnabled = snippetsBridge != null;
+        if (mAreSnippetsEnabled) {
+            mRecyclerView = (NewTabPageRecyclerView) findViewById(R.id.ntp_recycler_view);

[dgn, 2016/03/30 11:51:30]

Possible change: NewTabPageRecyclerView and NewTabPageScrollView could implement
an interface that would be what NewTabPageView cares about, which would avoid
having two members here and having to choose all the time which one to use. 

[newt (away), 2016/03/30 19:03:50]

That would help in certain places, but I'd weigh the decision against the added
indirection and complexity when reading the code. In this case, NewTabPageView
still needs to be aware of the two different modes in a number of places (e.g.
when inflating the layouts, removing the toolbar, initializing the search box
scroll handling). The only benefit would be removing the if-else inside of
getVerticalScroll() and updateSearchBoxOnScroll(), but that's a pretty small
win.

[dgn, 2016/03/31 00:46:45]

I think it's very possible to make it aware of the difference only in the
constructor, and have the rest handled by the child view. But yeah, it wouldn't
be a very big win anyway. Can be reconsidered when we add new NTP experiments.

+            mContentView = (NewTabPageLayout) LayoutInflater.from(getContext())
+                                   .inflate(R.layout.new_tab_page_layout, null);

[newt (away), 2016/03/30 19:03:50]

fix indentation. Also, it's better to pass in the parent view when inflating a
layout, as it allows the correct subclass of ViewGroup.LayoutParams to be used.
E.g.

    LayoutInflater.from(getContext()).inflate(R.layout.new_tab_page_layout,
mRecyclerView, false);

If the parent is, say, a LinearLayout and the inflated layout has
layout_weight="1", then the layout_weight will be lost unless you pass in the
parent view.

[dgn, 2016/03/31 00:46:45]

I think May did that on purpose. When setting the parent, the contentview does
like with ScrollView, and matches the parent. But with the new UI, instead of
being the only child, it's just the abovethefold item, and should not fill the
view. 
If I set the parent, I then have to adjust the height to match content. Is it
better to do that?

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

I see. The current code is OK in that case, though it would increase clarity to
set the layout params explicitly, or at least comment that the default layout
params are what we want.

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

Did both.

+        } else {
+            mScrollView = (NewTabPageScrollView) findViewById(R.id.ntp_scroll_view);
+            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);
@@ -281,7 +293,7 @@ public void afterTextChanged(Editable s) {
             }
         });
 
-        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) {
@@ -315,7 +327,9 @@ public void onClick(View v) {
             if (manager.isInterestsEnabled()) {
                 toolbar.getInterestsButton().setVisibility(View.VISIBLE);
             }
-        } else {
+        } else if (!mAreSnippetsEnabled) {
+            // Only remove if we're using the old NTP view, the new one does not use a
+            // ScrollView
             ((ViewGroup) toolbar.getParent()).removeView(toolbar);
             FrameLayout.LayoutParams params =
                     (FrameLayout.LayoutParams) mScrollView.getLayoutParams();
@@ -323,7 +337,6 @@ public void onClick(View v) {
             mScrollView.setLayoutParams(params);
         }
 
-        initializeSearchBoxScrollHandling();
         addOnLayoutChangeListener(this);
         setSearchProviderHasLogo(searchProviderHasLogo);
 
@@ -332,13 +345,12 @@ public void onClick(View v) {
                 mMostVisitedDesign.getNumberOfTiles(searchProviderHasLogo));
 
         // Set up snippets
-        if (ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS)) {
-            mSnippetsView = (RecyclerView) findViewById(R.id.snippets_card_list);
-            mSnippetsView.setVisibility(View.VISIBLE);
-            RecordHistogram.recordEnumeratedHistogram(SnippetsManager.SNIPPETS_STATE_HISTOGRAM,
-                    SnippetsManager.SNIPPETS_SHOWN, SnippetsManager.NUM_SNIPPETS_ACTIONS);
-            mSnippetsView.setLayoutManager(new LinearLayoutManager(getContext()));
-            mSnippetsView.addOnScrollListener(new RecyclerView.OnScrollListener() {
+        if (mAreSnippetsEnabled) {
+            mCardsManager = new NewTabPageAdapter(snippetsBridge, mManager, mContentView);
+            mRecyclerView.setAdapter(mCardsManager);
+
+            NewTabPageUma.recordSnippetAction(NewTabPageUma.SNIPPETS_ACTION_SHOWN);
+            mRecyclerView.addOnScrollListener(new RecyclerView.OnScrollListener() {
                 private boolean mScrolledOnce = false;
                 @Override
                 public void onScrollStateChanged(RecyclerView recyclerView, int newState) {
@@ -346,13 +358,12 @@ public void onScrollStateChanged(RecyclerView recyclerView, int newState) {
                     RecordUserAction.record("MobileNTP.Snippets.Scrolled");
                     if (mScrolledOnce) return;
                     mScrolledOnce = true;
-                    RecordHistogram.recordEnumeratedHistogram(
-                            SnippetsManager.SNIPPETS_STATE_HISTOGRAM,
-                            SnippetsManager.SNIPPETS_SCROLLED,
-                            SnippetsManager.NUM_SNIPPETS_ACTIONS);
+                    NewTabPageUma.recordSnippetAction(NewTabPageUma.SNIPPETS_ACTION_SCROLLED);
                 }
             });
-            snippetsManager.setSnippetsView(mSnippetsView);
+            initializeSearchBoxRecyclerViewScrollHandling();
+        } else {
+            initializeSearchBoxScrollHandling();
         }
     }
 
@@ -362,10 +373,10 @@ private void updateSearchBoxOnScroll() {
         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()));
+        View wrapperView = mAreSnippetsEnabled ? mRecyclerView : mScrollView;
+        if (wrapperView.getHeight() != 0 && mSearchBoxView.getTop() != 0) {
+            int scrollY = getVerticalScroll();
+            percentage = Math.max(0f, Math.min(1f, scrollY / (float) mSearchBoxView.getTop()));
         }
 
         updateVisualsForToolbarTransition(percentage);
@@ -375,6 +386,15 @@ private void updateSearchBoxOnScroll() {
         }
     }
 
+    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
@@ -388,7 +408,7 @@ public void run() {
                 mPendingSnapScroll = false;
             }
         };
-        mScrollView.setOnScrollListener(new NewTabScrollView.OnScrollListener() {
+        mScrollView.setOnScrollListener(new NewTabPageScrollView.OnScrollListener() {
             @Override
             public void onScrollChanged(int l, int t, int oldl, int oldt) {
                 if (mPendingSnapScroll) {
@@ -547,8 +567,8 @@ float getUrlFocusChangeAnimationPercent() {
      * @see #setUrlFocusChangeAnimationPercent
      */
     private void setUrlFocusChangeAnimationPercentInternal(float percent) {
-        mContentView.setTranslationY(percent * (-mMostVisitedLayout.getTop()
-                + mScrollView.getScrollY() + mContentView.getPaddingTop()));
+        mContentView.setTranslationY(percent * (-mMostVisitedLayout.getTop() + getVerticalScroll()
+                                                       + mContentView.getPaddingTop()));

[newt (away), 2016/03/30 19:03:50]

nit: fix indentation. Should just be 8 spaces beyond the line above. (You could
wrap at a higher level, e.g. after *, to make this clearer too)

[dgn, 2016/03/31 00:46:44]

git cl format being weird again. Fixed.

         updateVisualsForToolbarTransition(percent);
     }
 
@@ -586,7 +606,12 @@ void getSearchBoxBounds(Rect originalBounds, Rect transformedBounds) {
         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();
@@ -640,23 +665,6 @@ protected void onWindowVisibilityChanged(int visibility) {
         }
     }
 
-    @Override
-    protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
-        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
-
-        // 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);
-        }
-    }
-
     /**
      * @see org.chromium.chrome.browser.compositor.layouts.content.
      *         InvalidationAwareThumbnailProvider#shouldCaptureThumbnail()
@@ -664,10 +672,8 @@ protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
     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;
     }
 
     /**
@@ -679,7 +685,7 @@ void captureThumbnail(Canvas canvas) {
         ViewUtils.captureBitmap(this, canvas);
         mSnapshotWidth = getWidth();
         mSnapshotHeight = getHeight();
-        mSnapshotScrollY = mScrollView.getScrollY();
+        mSnapshotScrollY = getVerticalScroll();
         mSnapshotMostVisitedChanged = false;
     }
 
@@ -825,15 +831,12 @@ private void updateMostVisitedPlaceholderVisibility() {
         private static final int ICON_BACKGROUND_COLOR = 0xff787878;
         private static final int ICON_MIN_SIZE_PX = 48;
 
-        private int mMostVisitedLayoutBleed;
         private int mMinIconSize;
         private int mDesiredIconSize;
         private RoundedIconGenerator mIconGenerator;
 
         MostVisitedDesign(Context context) {
             Resources res = context.getResources();
-            mMostVisitedLayoutBleed = res.getDimensionPixelSize(
-                    R.dimen.most_visited_layout_bleed);
             mDesiredIconSize = res.getDimensionPixelSize(R.dimen.most_visited_icon_size);
             // On ldpi devices, mDesiredIconSize could be even smaller than ICON_MIN_SIZE_PX.
             mMinIconSize = Math.min(mDesiredIconSize, ICON_MIN_SIZE_PX);
@@ -848,10 +851,6 @@ public int getNumberOfTiles(boolean searchProviderHasLogo) {
             return searchProviderHasLogo ? NUM_TILES : NUM_TILES_NO_LOGO;
         }
 
-        public int getMostVisitedLayoutBleed() {
-            return mMostVisitedLayoutBleed;
-        }
-
         public void initMostVisitedLayout(MostVisitedLayout mostVisitedLayout,
                 boolean searchProviderHasLogo) {
             mostVisitedLayout.setMaxRows(searchProviderHasLogo ? MAX_ROWS : MAX_ROWS_NO_LOGO);
@@ -942,4 +941,12 @@ public void onIconUpdated(final String url) {
             }
         }
     }
+
+    private int getVerticalScroll() {
+        if (mAreSnippetsEnabled) {
+            return mRecyclerView.computeVerticalScrollOffset();
+        } else {
+            return mScrollView.getScrollY();
+        }
+    }
 }