[EoC] Don't auto-peek when toolbar button is enabled

Also disables the peek state when the toolbar button is enabled and
makes other functional updates to ContextualSuggestionsMediator so that
the button works properly.

Since the peek state is completely disabled, swipe-to-dismiss is enabled
when the toolbar button is enabled. This enforces that swiping down on
the open sheet will completely dismiss the UI.

Finally, this CL updates FetchHelper to clear state in #onPageLoadStarted()
instead of #onUpdateUrl() since the latter event may happen considerably
later than the first. We need to hide the toolbar button as soon as a
new page load begins.

BUG=856403

Change-Id: Ic2f0c084d521a07acf985b8001480b7605dbd190
Reviewed-on: https://chromium-review.googlesource.com/1119250Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571689}

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsBottomSheetContent.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.contextual_suggestions;
 
 import android.view.View;
 
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.ContentPriority;
 
@@ -55,7 +56,12 @@ public class ContextualSuggestionsBottomSheetContent implements BottomSheetConte
 
     @Override
     public boolean swipeToDismissEnabled() {
-        return false;
+        return ChromeFeatureList.isEnabled(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BUTTON);
+    }
+
+    @Override
+    public boolean isPeekStateEnabled() {
+        return !ChromeFeatureList.isEnabled(ChromeFeatureList.CONTEXTUAL_SUGGESTIONS_BUTTON);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/FetchHelper.java

@@ -179,7 +179,7 @@ class FetchHelper {
 
         mTabObserver = new EmptyTabObserver() {
             @Override
-            public void onUpdateUrl(Tab tab, String url) {
+            public void onPageLoadStarted(Tab tab, String url) {
                 assert !tab.isIncognito();
                 if (tab == mCurrentTab) {
                     clearState();

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

@@ -304,6 +304,11 @@ public class BottomSheet extends FrameLayout
          */
         boolean swipeToDismissEnabled();
 
+        /**
+         * @return Whether the peek state is enabled.
+         */
+        boolean isPeekStateEnabled();
+
         /**
          * @return Whether a slimmer peek UI should be used for this content.
          */
@@ -749,7 +754,8 @@ public class BottomSheet extends FrameLayout
      *         close the sheet or peek it.
      */
     private @SheetState int getMinSwipableSheetState() {
-        return swipeToDismissEnabled() ? SHEET_STATE_HIDDEN : SHEET_STATE_PEEK;
+        return swipeToDismissEnabled() || !mSheetContent.isPeekStateEnabled() ? SHEET_STATE_HIDDEN
+                : SHEET_STATE_PEEK;
     }
 
     @Override
@@ -1412,6 +1418,7 @@ public class BottomSheet extends FrameLayout
         int prevState = nextState;
         for (int i = getMinSwipableSheetState(); i < sStates.length; i++) {
             if (sStates[i] == SHEET_STATE_HALF && shouldSkipHalfState) continue;
+            if (sStates[i] == SHEET_STATE_PEEK && !mSheetContent.isPeekStateEnabled()) continue;
             prevState = nextState;
             nextState = sStates[i];
             // The values in PanelState are ascending, they should be kept that way in order for

chrome/android/javatests/src/org/chromium/chrome/browser/widget/bottomsheet/TestBottomSheetContent.java

@@ -76,6 +76,11 @@ class TestBottomSheetContent implements BottomSheetContent {
         return false;
     }
 
+    @Override
+    public boolean isPeekStateEnabled() {
+        return true;
+    }
+
     @Override
     public boolean useSlimPeek() {
         return false;

chrome/android/junit/src/org/chromium/chrome/browser/contextual_suggestions/FetchHelperTest.java

@@ -155,12 +155,12 @@ public final class FetchHelperTest {
     }
 
     @Test
-    public void tabObserver_updateUrl() {
+    public void tabObserver_pageLoadStarted() {
         FetchHelper helper = createFetchHelper();
-        getTabObserver().onUpdateUrl(mTab, STARTING_URL);
+        getTabObserver().onPageLoadStarted(mTab, STARTING_URL);
         verify(mDelegate, times(1)).clearState();
         // Normally we would change the suffix here, but ShadowUrlUtilities don't support that now.
-        getTabObserver().onUpdateUrl(mTab, DIFFERENT_URL);
+        getTabObserver().onPageLoadStarted(mTab, DIFFERENT_URL);
         verify(mDelegate, times(2)).clearState();
     }
 
@@ -170,14 +170,14 @@ public final class FetchHelperTest {
     }
 
     @Test
-    public void tabObserver_didFirstVisuallyNonEmptyPaint_updateUrl_toSame() {
-        delayFetchExecutionTest_updateUrl_toSame(
+    public void tabObserver_didFirstVisuallyNonEmptyPaint_pageLoadStarted_toSame() {
+        delayFetchExecutionTest_pageLoadStarted_toSame(
                 (tabObserver) -> tabObserver.didFirstVisuallyNonEmptyPaint(mTab));
     }
 
     @Test
-    public void tabObserver_didFirstVisuallyNonEmptyPaint_updateUrl_toDifferent() {
-        delayFetchExecutionTest_updateUrl_toDifferent(
+    public void tabObserver_didFirstVisuallyNonEmptyPaint_pageLoadStarted_toDifferent() {
+        delayFetchExecutionTest_pageLoadStarted_toDifferent(
                 (tabObserver) -> tabObserver.didFirstVisuallyNonEmptyPaint(mTab));
     }
 
@@ -187,14 +187,14 @@ public final class FetchHelperTest {
     }
 
     @Test
-    public void tabObserver_onPageLoadFinished_updateUrl_toSame() {
-        delayFetchExecutionTest_updateUrl_toSame(
+    public void tabObserver_onPageLoadFinished_pageLoadStarted_toSame() {
+        delayFetchExecutionTest_pageLoadStarted_toSame(
                 (tabObserver) -> tabObserver.onPageLoadFinished(mTab));
     }
 
     @Test
-    public void tabObserver_onPageLoadFinished_updateUrl_toDifferent() {
-        delayFetchExecutionTest_updateUrl_toDifferent(
+    public void tabObserver_onPageLoadFinished_pageLoadStarted_toDifferent() {
+        delayFetchExecutionTest_pageLoadStarted_toDifferent(
                 (tabObserver) -> tabObserver.onPageLoadFinished(mTab));
     }
 
@@ -204,14 +204,14 @@ public final class FetchHelperTest {
     }
 
     @Test
-    public void tabObserver_onLoadStopped_updateUrl_toSame() {
-        delayFetchExecutionTest_updateUrl_toSame(
+    public void tabObserver_onLoadStopped_pageLoadStarted_toSame() {
+        delayFetchExecutionTest_pageLoadStarted_toSame(
                 (tabObserver) -> tabObserver.onLoadStopped(mTab, false));
     }
 
     @Test
-    public void tabObserver_onLoadStopped_updateUrl_toDifferent() {
-        delayFetchExecutionTest_updateUrl_toDifferent(
+    public void tabObserver_onLoadStopped_pageLoadStarted_toDifferent() {
+        delayFetchExecutionTest_pageLoadStarted_toDifferent(
                 (tabObserver) -> tabObserver.onLoadStopped(mTab, false));
     }
 
@@ -504,24 +504,25 @@ public final class FetchHelperTest {
         verify(mDelegate, times(1)).requestSuggestions(eq(STARTING_URL));
     }
 
-    private void delayFetchExecutionTest_updateUrl_toSame(Consumer<TabObserver> consumer) {
+    private void delayFetchExecutionTest_pageLoadStarted_toSame(Consumer<TabObserver> consumer) {
         FetchHelper helper = createFetchHelper();
         verify(mTab, times(1)).addObserver(mTabObserverCaptor.capture());
         consumer.accept(getTabObserver());
 
-        mTabObserverCaptor.getValue().onUpdateUrl(mTab, STARTING_URL);
+        mTabObserverCaptor.getValue().onPageLoadStarted(mTab, STARTING_URL);
         verify(mDelegate, times(1)).clearState();
 
         runUntilFetchPossible();
         verify(mDelegate, times(1)).requestSuggestions(eq(STARTING_URL));
     }
 
-    private void delayFetchExecutionTest_updateUrl_toDifferent(Consumer<TabObserver> consumer) {
+    private void delayFetchExecutionTest_pageLoadStarted_toDifferent(
+            Consumer<TabObserver> consumer) {
         FetchHelper helper = createFetchHelper();
         verify(mTab, times(1)).addObserver(mTabObserverCaptor.capture());
         consumer.accept(getTabObserver());
 
-        mTabObserverCaptor.getValue().onUpdateUrl(mTab, DIFFERENT_URL);
+        mTabObserverCaptor.getValue().onPageLoadStarted(mTab, DIFFERENT_URL);
         verify(mDelegate, times(1)).clearState();
 
         // Request suggestions should not be called.