[Home] Add lifecycle events and some tests

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 274e9f1627fd457b5a9932a9264a36713e876abe..1c6243dc82da12806b28f748f5e18c9d825e1536 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
@@ -393,6 +393,8 @@ public class BottomSheet
 
     @Override
     public int loadUrl(LoadUrlParams params) {
+        for (BottomSheetObserver o : mObservers) o.onLoadUrl(params.getUrl());

[Theresa, 2017/02/17 18:54:48]

Should this happen after the early return for native page URLs?

[mdjones, 2017/02/17 21:33:19]

That depends on how we consider navigations inside of native pages. The reason
the early return is there is so we don't use the tab's native handling logic. My
thinking was that it is still *technically* loading a URL so it would be here.
If you feel strongly the other way, I can move it.

[Theresa, 2017/02/17 21:48:07]

The early return is to handle bookmarks/downloads changing the URL?
I don't feel strongly about it. 

[mdjones, 2017/02/17 21:52:11]

That's the plan right now. There needs to be some more plumbing before that
works though. Things will be more concrete when we start adding the other chrome
home content.

+
         // Native page URLs in this context do not need to communicate with the tab.
         if (NativePageFactory.isNativePageUrl(params.getUrl(), isIncognito())) {
             return TabLoadStatus.PAGE_LOAD_FAILED;
@@ -447,13 +449,22 @@ public class BottomSheet
     /**
      * A notification that the sheet is exiting the peek state into one that shows content.
      */
-    private void onExitPeekState() {
+    private void onSheetOpened() {
         if (mSuggestionsContent == null) {
             mSuggestionsContent = new SuggestionsBottomSheetContent(
                     mTabModelSelector.getCurrentTab().getActivity(), this, mTabModelSelector);
         }
 
         showContent(mSuggestionsContent);
+
+        for (BottomSheetObserver o : mObservers) o.onSheetOpened();
+    }
+
+    /**
+     * A notification that the sheet has returned to the peeking state.
+     */
+    private void onSheetClosed() {
+        for (BottomSheetObserver o : mObservers) o.onSheetClosed();
     }
 
     /**
@@ -593,12 +604,19 @@ public class BottomSheet
 
     /**
      * Sets the sheet's offset relative to the bottom of the screen.
+     * THIS SHOULD NOT BE CALLED PUBLICLY OUTSIDE OF TESTS.
      * @param offset The offset that the sheet should be.
      */
-    private void setSheetOffsetFromBottom(float offset) {
+    @VisibleForTesting
+    public void setSheetOffsetFromBottom(float offset) {

[gone, 2017/02/17 20:02:32]

If you feel strongly enough to yell, maybe keep this method private and add a
method like setSheetOffsetFromBottomForTests()?

[mdjones, 2017/02/17 21:33:18]

Good plan; done.

         if (MathUtils.areFloatsEqual(getSheetOffsetFromBottom(), getMinOffset())
                 && offset > getMinOffset()) {
-            onExitPeekState();
+            onSheetOpened();
+        }
+
+        if (MathUtils.areFloatsEqual(offset, getMinOffset())
+                && getSheetOffsetFromBottom() > getMinOffset()) {

[Theresa, 2017/02/17 18:54:48]

Would this second line fail if the sheet were offscreen (e.g. due to a scroll)?

[mdjones, 2017/02/17 21:33:19]

Yes but the sheet's android view never moves below the peeking state, it is just
invisible. If that line fails, it also indicates that the sheet is moving up
instead of down, so we wouldn't want the event to trigger anyway.

+            onSheetClosed();
         }
 
         setTranslationY(mContainerHeight - offset);
@@ -608,25 +626,36 @@ public class BottomSheet
     /**
      * @return The ratio of the height of the screen that the peeking state is.
      */
-    private float getPeekRatio() {
+    @VisibleForTesting
+    public float getPeekRatio() {
         return mStateRatios[0];
     }
 
     /**
      * @return The ratio of the height of the screen that the half expanded state is.
      */
-    private float getHalfRatio() {
+    @VisibleForTesting
+    public float getHalfRatio() {
         return mStateRatios[1];
     }
 
     /**
      * @return The ratio of the height of the screen that the fully expanded state is.
      */
-    private float getFullRatio() {
+    @VisibleForTesting
+    public float getFullRatio() {
         return mStateRatios[2];
     }
 
     /**
+     * @return The height of the container that the bottom sheet exists in.
+     */
+    @VisibleForTesting
+    public float getSheetContainerHeight() {
+        return mContainerHeight;
+    }
+
+    /**
      * Sends a notification if the sheet is transitioning from the peeking to half expanded state.
      * This method only sends events when the sheet is between the peeking and half states.
      */
@@ -642,9 +671,9 @@ public class BottomSheet
         // If the ratio is close enough to zero, just set it to zero.
         if (MathUtils.areFloatsEqual(peekHalfRatio, 0f)) peekHalfRatio = 0f;
 
-        for (BottomSheetObserver o : mObservers) {
-            if (mLastPeekToHalfRatioSent < 1f || peekHalfRatio < 1f) {
-                mLastPeekToHalfRatioSent = peekHalfRatio;
+        if (mLastPeekToHalfRatioSent < 1f || peekHalfRatio < 1f) {
+            mLastPeekToHalfRatioSent = peekHalfRatio;
+            for (BottomSheetObserver o : mObservers) {
                 o.onTransitionPeekToHalf(peekHalfRatio);
             }
         }