Attach NTP to Chrome Home bottom sheet

chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java

Index: chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java b/chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
index ba8338109b648bf842d94a319596c003504d9ed4..3d97fab4b51dcc344f45e18aaac0f2e904c9040b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java
@@ -10,15 +10,22 @@ import android.animation.ObjectAnimator;
 import android.content.Context;
 import android.graphics.Region;
 import android.support.annotation.IntDef;
+import android.support.v4.view.ScrollingView;
 import android.util.AttributeSet;
 import android.view.GestureDetector;
 import android.view.MotionEvent;
 import android.view.VelocityTracker;
 import android.view.View;
+import android.view.ViewGroup;
 import android.view.animation.DecelerateInterpolator;
 import android.view.animation.Interpolator;
+import android.widget.FrameLayout;
 import android.widget.LinearLayout;
 
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.NativePage;
+import org.chromium.chrome.browser.ntp.NewTabPage;
+import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.util.MathUtils;
 
 import java.lang.annotation.Retention;
@@ -58,6 +65,9 @@ public class BottomSheet extends LinearLayout {
     /** The minimum y/x ratio that a scroll must have to be considered vertical. */
     private static final float MIN_VERTICAL_SCROLL_SLOPE = 2.0f;
 
+    /** The minimum difference that two floats must have to be considered different. */
+    private static final float EPSILON = 0.0001f;

[gone, 2017/01/23 19:48:39]

Can we just get a common version of this?  We've got a bunch of different
EPSILON definitions for different float equality checks that really shouldn't
all be different.

[mdjones, 2017/01/24 01:37:24]

Added constant to MathUtils, I'll update other references later.

+
     /**
      * Information about the different scroll states of the sheet. Order is important for these,
      * they go from smallest to largest.
@@ -84,12 +94,31 @@ public class BottomSheet extends LinearLayout {
     /** The height of the toolbar. */
     private float mToolbarHeight;
 
-    /** The height of the view that contains the bottom sheet. */
+    /** The width and height of the view that contains the bottom sheet. */

[gone, 2017/01/23 19:48:39]

Separate the javadocs.  They're meant to apply to the statement immediately
after them, which confuses IDEs.

[mdjones, 2017/01/24 01:37:24]

Done.

+    private float mContainerWidth;
     private float mContainerHeight;
 
     /** The current sheet state. If the sheet is moving, this will be the target state. */
     private int mCurrentState;
 
+    /** Used for getting the current tab. */
+    private TabModelSelector mTabModelSelector;
+
+    /** A handle to the native page being shown by the sheet. */
+    private NativePage mNativePage;
+
+    /** A handle to the toolbar control container. */
+    private View mControlContainer;
+
+    /** A handle to the FrameLayout that holds the content of the bottom sheet. */
+    private FrameLayout mBottomSheetContent;
+
+    /** A handle to the main scrolling view in the bottom sheet's content. */
+    private ScrollingView mScrollingContentView;
+
+    /** This is a cached array for getting the window location of different views. */
+    private final int[] mLocationArr;

[gone, 2017/01/23 19:48:39]

1) Don't abbreviate Array.
2) Private finals up top, unless you have a good reason to keep this here.

[mdjones, 2017/01/24 01:37:24]

Done.

+
     /**
      * This class is responsible for detecting swipe and scroll events on the bottom sheet or
      * ignoring them when appropriate.
@@ -113,16 +142,25 @@ public class BottomSheet extends LinearLayout {
 
             // Cancel the settling animation if it is running so it doesn't conflict with where the
             // user wants to move the sheet.
+            boolean wasSettleAnimatorRunning = mSettleAnimator != null;
             cancelAnimation();
 
             mVelocityTracker.addMovement(e2);
 
             float currentShownRatio =
                     mContainerHeight > 0 ? getSheetOffsetFromBottom() / mContainerHeight : 0;
+            boolean isSheetInMaxPosition =
+                    areFloatsEqual(currentShownRatio, mStateRatios[mStateRatios.length - 1]);
+
+            // Allow the bottom sheet's content to be scrolled up without dragging the sheet down.
+            if (!isTouchEventInToolbar(e2) && isSheetInMaxPosition && mScrollingContentView != null
+                    && mScrollingContentView.computeVerticalScrollOffset() > 0) {
+                mIsScrolling = false;
+                return false;
+            }
 
             // If the sheet is in the max position, don't move if the scroll is upward.

[gone, 2017/01/23 19:48:39]

Comment's not terribly helpful; fix comment s'il vous plait.

[mdjones, 2017/01/24 01:37:24]

Done.

-            if (currentShownRatio >= mStateRatios[mStateRatios.length - 1]
-                    && distanceY > 0) {
+            if (isSheetInMaxPosition && distanceY > 0) {
                 mIsScrolling = false;
                 return false;
             }
@@ -133,6 +171,12 @@ public class BottomSheet extends LinearLayout {
                 return false;
             }
 
+            // Send a notification that the sheet is exiting the peeking state into something that
+            // will show content.
+            if (!mIsScrolling && mCurrentState == SHEET_STATE_PEEK && !wasSettleAnimatorRunning) {
+                onExitPeekState();
+            }
+
             float newOffset = getSheetOffsetFromBottom() + distanceY;
             setSheetOffsetFromBottom(MathUtils.clamp(newOffset, getMinOffset(), getMaxOffset()));
 
@@ -166,7 +210,7 @@ public class BottomSheet extends LinearLayout {
         super(context, atts);
 
         setOrientation(LinearLayout.VERTICAL);
-
+        mLocationArr = new int[2];

[gone, 2017/01/23 19:48:39]

Might as well

private final int mLocationArray = new int[2];

[mdjones, 2017/01/24 01:37:24]

Done.

         mVelocityTracker = VelocityTracker.obtain();
 
         mGestureDetector = new GestureDetector(context, new BottomSheetSwipeDetector());
@@ -220,6 +264,13 @@ public class BottomSheet extends LinearLayout {
     }
 
     /**
+     * @param tabModelSelector A TabModelSelector for getting the current tab and activity.
+     */
+    public void setTabModelSelector(TabModelSelector tabModelSelector) {
+        mTabModelSelector = tabModelSelector;
+    }
+
+    /**
      * Adds layout change listeners to the views that the bottom sheet depends on. Namely the
      * heights of the root view and control container are important as they are used in many of the
      * calculations in this class.
@@ -227,7 +278,9 @@ public class BottomSheet extends LinearLayout {
      * @param controlContainer The container for the toolbar.
      */
     public void init(View root, View controlContainer) {
-        mToolbarHeight = controlContainer.getHeight();
+        mControlContainer = controlContainer;
+        mToolbarHeight = mControlContainer.getHeight();
+        mBottomSheetContent = (FrameLayout) findViewById(R.id.bottom_sheet_content);
         mCurrentState = SHEET_STATE_PEEK;
 
         // Listen to height changes on the root.
@@ -239,8 +292,9 @@ public class BottomSheet extends LinearLayout {
                     return;
                 }
 
+                mContainerWidth = right - left;
                 mContainerHeight = bottom - top;
-                updateSheetPeekHeight();
+                updateSheetDimensions();
 
                 cancelAnimation();
                 setSheetState(mCurrentState, false);
@@ -257,7 +311,7 @@ public class BottomSheet extends LinearLayout {
                 }
 
                 mToolbarHeight = bottom - top;
-                updateSheetPeekHeight();
+                updateSheetDimensions();
 
                 cancelAnimation();
                 setSheetState(mCurrentState, false);
@@ -266,6 +320,57 @@ public class BottomSheet extends LinearLayout {
     }
 
     /**
+     * Determines if a touch event is inside the toolbar. This assumes the toolbar is the full
+     * width of the screen and that the toolbar is at the top of the bottom sheet.
+     * @param e The motion event to test.
+     * @return True if the event occured in the toolbar region.
+     */
+    private boolean isTouchEventInToolbar(MotionEvent e) {
+        // Is the touch in the toolbar area?
+        if (mControlContainer != null) {
+            mControlContainer.getLocationInWindow(mLocationArr);
+        } else {
+            mLocationArr[0] = mLocationArr[1] = 0;

[gone, 2017/01/23 19:48:39]

Shouldn't this thing straight up return false here because it's an invalid
state?  Granted, mToolbarHeight is set only when the sheet is initialized, so
it'll be zero in that case and it'll fall through, but it'd return false in that
case anyway.

[mdjones, 2017/01/24 01:37:24]

Done.

+        }
+
+        return e.getRawY() < mLocationArr[1] + mToolbarHeight;
+    }
+
+    /**
+     * Determine if two floats are equal.
+     * @param f1 The first float to compare.
+     * @param f2 The second float to compare.
+     * @return True if the floats are equal.
+     */
+    private static boolean areFloatsEqual(float f1, float f2) {
+        return Math.abs(f1 - f2) < EPSILON;
+    }
+
+    /**
+     * A notification that the sheet is exiting the peek state into one that shows content.
+     */
+    private void onExitPeekState() {
+        if (mNativePage == null) {
+            showNativePage(new NewTabPage(mTabModelSelector.getCurrentTab().getActivity(),
+                    mTabModelSelector.getCurrentTab(), mTabModelSelector));
+        }
+    }
+
+    /**
+     * Show a native page in the bottom sheet's content area.
+     * @param page The NativePage to show.
+     */
+    private void showNativePage(NativePage page) {
+        if (mNativePage != null) mBottomSheetContent.removeView(mNativePage.getView());
+
+        mNativePage = page;
+        mBottomSheetContent.addView(mNativePage.getView());
+        mScrollingContentView = findScrollingChild(mNativePage.getView());
+
+        mNativePage.updateForUrl("");
+    }
+
+    /**
      * Creates an unadjusted version of a MotionEvent.
      * @param e The original event.
      * @return The unadjusted version of the event.
@@ -277,12 +382,41 @@ public class BottomSheet extends LinearLayout {
     }
 
     /**
-     * Updates the bottom sheet's peeking height.
+     * Updates the bottom sheet's peeking and content height.
      */
-    private void updateSheetPeekHeight() {
+    private void updateSheetDimensions() {
         if (mContainerHeight <= 0) return;
 
         mStateRatios[0] = mToolbarHeight / mContainerHeight;

[gone, 2017/01/23 19:48:39]

Is this thing actually updating the ratio array?  I'd thought that mStateRatios
was supposed to be unchanging and final.

[mdjones, 2017/01/24 01:37:24]

Yeah, unfortunately we don't know the height of the toolbar or compositor view
until it is inflated (both are needed to compute this value).

+
+        // Compute the height that the content section of the bottom sheet should be.

[gone, 2017/01/23 19:48:39]

// Compute the height of the content section of the bottom sheet.

[mdjones, 2017/01/24 01:37:24]

Done.

+        float contentHeight =
+                (mContainerHeight * mStateRatios[mStateRatios.length - 1]) - mToolbarHeight;
+        mBottomSheetContent.setLayoutParams(
+                new LinearLayout.LayoutParams((int) mContainerWidth, (int) contentHeight));
+    }
+
+    /**
+     * Find the first ScrollingView in a view hierarchy.
+     * TODO(mdjones): The root of native pages should be a ScrollingView so this logic is not
+     * necessary.

[gone, 2017/01/23 19:48:39]

Yeah... this logic makes me sad.  Is this only until we have all the sheets
implemented as ScrollingViews?  It'd switch to an assert at that point, I
imagine?

[mdjones, 2017/01/24 01:37:24]

Yeah, I'll remove after I switch away from using native pages for content. Each
content view will implement a limited interface and assert that the view
obtained from each is something that scrolls.

+     * @param view The root of the tree or subtree.
+     * @return The first scrolling view or null.
+     */
+    private ScrollingView findScrollingChild(View view) {

[gone, 2017/01/23 19:48:39]

@Nullable

[mdjones, 2017/01/24 01:37:24]

Done.

+        if (view instanceof ScrollingView) {
+            return (ScrollingView) view;
+        }
+        if (view instanceof ViewGroup) {
+            ViewGroup group = (ViewGroup) view;
+            for (int i = 0, count = group.getChildCount(); i < count; i++) {
+                ScrollingView scrollingChild = findScrollingChild(group.getChildAt(i));
+                if (scrollingChild != null) {
+                    return scrollingChild;
+                }
+            }
+        }
+        return null;
     }
 
     /**