Revert "Query Tiles: Submit query on last level tile click"

This reverts commit 011b4cb3.

Reason for revert: Broke internal builder (renaming of a method:
LocationBarLayout::performSearchQueryForTest)

Original change's description:
> Query Tiles: Submit query on last level tile click
> 
> This CL adds :
> 1 - Submit query and show SRP on the last level tile click
> 2 - Shorten most visited tile section to 1 row for smaller screens
> 
> Bug: 1068308
> Change-Id: Icae29ad8bcc563ca6cfa3bfd35d18fa90ff65b7f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2147041
> Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#758724}

TBR=dtrainor@chromium.org,shaktisahu@chromium.org

Change-Id: I606eaefdff636668d0c8e30759d4bb8cbf38def8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1068308
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2148958Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758832}

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

@@ -26,14 +26,6 @@ public interface FakeboxDelegate {
     void setUrlBarFocus(
             boolean shouldBeFocused, @Nullable String pastedText, @OmniboxFocusReason int reason);
 
-    /**
-     * Performs a search query on the current {@link Tab}.  This calls {@link
-     * TemplateUrlService#getUrlForSearchQuery(String)} to get a url based on {@code query} and
-     * loads that url in the current {@link Tab}.
-     * @param query The {@link String} that represents the text query that should be searched for.
-     */
-    void performSearchQuery(String query);
-
     /**
      * @return Whether the URL bar is currently focused.
      */

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

@@ -219,12 +219,6 @@ public class NewTabPage implements NativePage, InvalidationAwareThumbnailProvide
             }
         }
 
-        @Override
-        public void performSearchQuery(String query) {
-            if (mFakeboxDelegate == null) return;
-            mFakeboxDelegate.performSearchQuery(query);
-        }
-
         @Override
         public boolean isCurrentPage() {
             if (mIsDestroyed) return false;

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

@@ -267,11 +267,10 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
         setSearchProviderInfo(searchProviderHasLogo, searchProviderIsGoogle);
         mSearchProviderLogoView.showSearchProviderInitialView();
 
-        mQueryTileSection = new QueryTileSection(findViewById(R.id.query_tiles),
-                mSearchBoxCoordinator, profile, mManager::performSearchQuery);
+        mQueryTileSection = new QueryTileSection(
+                findViewById(R.id.query_tiles), mSearchBoxCoordinator, profile);
 
-        mTileGroup.startObserving(
-                getMaxRowsForMostVisitedTiles() * getMaxColumnsForMostVisitedTiles());
+        mTileGroup.startObserving(getMaxTileRows() * getMaxTileColumns());
 
         VrModuleProvider.registerVrModeObserver(this);
         if (VrModuleProvider.getDelegate().isInVr()) onEnterVr();
@@ -786,15 +785,15 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
         }
     }
 
-    private int getMaxRowsForMostVisitedTiles() {
-        return mQueryTileSection != null && mQueryTileSection.shouldConsiderAsSmallScreen() ? 1 : 2;
+    private static int getMaxTileRows() {
+        return 2;
     }
 
     /**
      * Determines The maximum number of tiles to try and fit in a row. On smaller screens, there
      * may not be enough space to fit all of them.
      */
-    private int getMaxColumnsForMostVisitedTiles() {
+    private int getMaxTileColumns() {
         return 4;
     }
 

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

@@ -77,12 +77,6 @@ public class NewTabPageView extends FrameLayout {
          */
         void focusSearchBox(boolean beginVoiceSearch, String pastedText);
 
-        /**
-         * Performs a search query on the current {@link Tab}.
-         * @param query The {@link String} representing the query text.
-         */
-        void performSearchQuery(String query);
-
         /**
          * @return whether the {@link NewTabPage} associated with this manager is the current page
          * displayed to the user.

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java

@@ -66,6 +66,7 @@ import org.chromium.chrome.browser.ui.native_page.NativePage;
 import org.chromium.chrome.browser.util.AccessibilityUtil;
 import org.chromium.components.browser_ui.styles.ChromeColors;
 import org.chromium.components.browser_ui.widget.CompositeTouchDelegate;
+import org.chromium.components.search_engines.TemplateUrlService;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.common.ResourceRequestBody;
 import org.chromium.ui.base.DeviceFormFactor;
@@ -729,8 +730,14 @@ public class LocationBarLayout extends FrameLayout
         // When we restore tabs, we focus the selected tab so the URL of the page shows.
     }
 
-    @Override
-    public void performSearchQuery(String query) {
+    /**
+     * Performs a search query on the current {@link Tab}.  This calls
+     * {@link TemplateUrlService#getUrlForSearchQuery(String)} to get a url based on {@code query}
+     * and loads that url in the current {@link Tab}.
+     * @param query The {@link String} that represents the text query that should be searched for.
+     */
+    @VisibleForTesting
+    public void performSearchQueryForTest(String query) {
         if (TextUtils.isEmpty(query)) return;
 
         String queryUrl = TemplateUrlServiceFactory.get().getUrlForSearchQuery(query);

chrome/android/java/src/org/chromium/chrome/browser/query_tiles/QueryTileSection.java

@@ -13,8 +13,6 @@ import org.chromium.base.Callback;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.ntp.search.SearchBoxCoordinator;
 import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.ui.display.DisplayAndroid;
-import org.chromium.ui.display.DisplayUtil;
 
 import java.util.List;
 
@@ -24,24 +22,17 @@ import java.util.List;
  * section.
  */
 public class QueryTileSection {
-    private static final String QUERY_TILES_SHORTEN_MOST_VISITED_TILES_FOR_SMALL_SCREEN =
-            "shorten_most_visited_tiles_for_small_screen";
-    private static final int SMALL_SCREEN_HEIGHT_THRESHOLD_DP = 600;
-
     private final ViewGroup mQueryTileSectionView;
     private final SearchBoxCoordinator mSearchBoxCoordinator;
-    private final Callback<String> mSubmitQueryCallback;
     private QueryTileCoordinator mQueryTileCoordinator;
     private TileProvider mTileProvider;
 
     /** Constructor. */
     public QueryTileSection(ViewGroup queryTileSectionView,
-            SearchBoxCoordinator searchBoxCoordinator, Profile profile,
-            Callback<String> performSearchQueryCallback) {
+            SearchBoxCoordinator searchBoxCoordinator, Profile profile) {
         mQueryTileSectionView = queryTileSectionView;
         mSearchBoxCoordinator = searchBoxCoordinator;
-        mSubmitQueryCallback = performSearchQueryCallback;
-        if (!isFeatureEnabled()) return;
+        if (!ChromeFeatureList.isEnabled(ChromeFeatureList.QUERY_TILES)) return;
 
         mTileProvider = TileProviderFactory.getForProfile(profile);
         mQueryTileCoordinator = QueryTileCoordinatorFactory.create(
@@ -52,46 +43,15 @@ public class QueryTileSection {
     }
 
     private void onTileClicked(Tile tile) {
-        if (tile == null) {
-            mTileProvider.getQueryTiles(this::setTiles);
-        } else {
-            boolean isLastLevelTile = tile.children.isEmpty();
-            setTiles(tile.children);
-            if (isLastLevelTile) {
-                mSubmitQueryCallback.onResult(tile.queryText);
-            } else {
-                // TODO(shaktisahu): Show chip on fakebox;
-            }
-        }
-    }
+        mTileProvider.getQueryTiles(tiles -> {
+            mQueryTileCoordinator.setTiles(tiles);
+            mQueryTileSectionView.setVisibility(tiles.isEmpty() ? View.GONE : View.VISIBLE);
+        });
 
-    private void setTiles(List<Tile> tiles) {
-        mQueryTileCoordinator.setTiles(tiles);
-        mQueryTileSectionView.setVisibility(tiles.isEmpty() ? View.GONE : View.VISIBLE);
+        if (tile != null) mSearchBoxCoordinator.setSearchText(tile.queryText);
     }
 
     private void getVisuals(Tile tile, Callback<List<Bitmap>> callback) {
         mTileProvider.getVisuals(tile.id, callback);
     }
-
-    /**
+}
\ No newline at end of file
-     * @return Whether the screen height is small. Used for shortening the most visited tiles
-     *         section on NTP so that feed is still visible above the fold.
-     */
-    public boolean shouldConsiderAsSmallScreen() {
-        if (!isFeatureEnabled()) return false;
-        boolean shortenMostVisitedTiles = ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
-                ChromeFeatureList.QUERY_TILES,
-                QUERY_TILES_SHORTEN_MOST_VISITED_TILES_FOR_SMALL_SCREEN, false);
-        if (!shortenMostVisitedTiles) return false;
-
-        DisplayAndroid display =
-                DisplayAndroid.getNonMultiDisplay(mQueryTileSectionView.getContext());
-        int screenHeightDp = DisplayUtil.pxToDp(display, display.getDisplayHeight());
-        return screenHeightDp < SMALL_SCREEN_HEIGHT_THRESHOLD_DP;
-    }
-
-    private static boolean isFeatureEnabled() {
-        return ChromeFeatureList.isEnabled(ChromeFeatureList.QUERY_TILES);
-    }
-}

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/CustomTabToolbar.java

@@ -847,9 +847,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
             return false;
         }
 
-        @Override
-        public void performSearchQuery(String query) {}
-
         @Override
         public void setUrlBarFocus(boolean shouldBeFocused, @Nullable String pastedText,
                 @LocationBar.OmniboxFocusReason int reason) {}

chrome/browser/upboarding/query_tiles/android/javatests/src/org/chromium/chrome/browser/query_tiles/QueryTileSectionTest.java

@@ -8,11 +8,8 @@ import static android.support.test.espresso.Espresso.onView;
 import static android.support.test.espresso.assertion.ViewAssertions.matches;
 import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
 import static android.support.test.espresso.matcher.ViewMatchers.withId;
-import static android.support.test.espresso.matcher.ViewMatchers.withText;
 
 import android.graphics.Bitmap;
-import android.graphics.BitmapFactory;
-import android.support.test.espresso.action.ViewActions;
 import android.support.test.filters.SmallTest;
 
 import org.junit.After;
@@ -22,7 +19,6 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.Callback;
-import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -32,8 +28,6 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.NewTabPageTestUtils;
 import org.chromium.chrome.test.util.browser.Features;
-import org.chromium.content_public.browser.test.util.Criteria;
-import org.chromium.content_public.browser.test.util.CriteriaHelper;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -44,8 +38,6 @@ import java.util.List;
 @RunWith(ChromeJUnit4ClassRunner.class)
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
 public class QueryTileSectionTest {
-    private static final String SEARCH_URL_PATTERN = "https://www.google.com/search?q=";
-
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
 
@@ -66,35 +58,13 @@ public class QueryTileSectionTest {
     @Features.EnableFeatures(ChromeFeatureList.QUERY_TILES)
     public void testSimpleTest() throws Exception {
         onView(withId(R.id.query_tiles)).check(matches(isDisplayed()));
-        onView(withText("Tile 1")).check(matches(isDisplayed()));
-    }
-
-    @Test
-    @SmallTest
-    @Features.EnableFeatures(ChromeFeatureList.QUERY_TILES)
-    public void testLeafLevelTileLoadsSearchResultsPage() throws Exception {
-        onView(withId(R.id.query_tiles)).check(matches(isDisplayed()));
-        onView(withText("Tile 1")).perform(ViewActions.click());
-        onView(withText("Tile 1_1")).check(matches(isDisplayed())).perform(ViewActions.click());
-        waitForSearchResultsPage(mActivityTestRule.getActivity().getActivityTab());
-    }
-
-    private static void waitForSearchResultsPage(final Tab tab) {
-        CriteriaHelper.pollUiThread(new Criteria("The SRP was never loaded.") {
-            @Override
-            public boolean isSatisfied() {
-                return tab.getUrl().getValidSpecOrEmpty().contains(SEARCH_URL_PATTERN);
-            }
-        });
     }
 
     private static class TestTileProvider implements TileProvider {
         private List<Tile> mTiles = new ArrayList<>();
 
         private TestTileProvider() {
-            List<Tile> children = new ArrayList<>();
-            children.add(new Tile("tile1_1", "Tile 1_1", "Tile 1_1", "Tile 1_1 Query", null));
-            Tile tile = new Tile("1", "Tile 1", "Tile 1", "Tile 1 Query", children);
+            Tile tile = new Tile("1", "Tile 1", "Tile 1", "Tile 1", null);
             mTiles.add(tile);
         }
 
@@ -105,12 +75,7 @@ public class QueryTileSectionTest {
 
         @Override
         public void getVisuals(String id, Callback<List<Bitmap>> callback) {
-            List<Bitmap> bitmapList = new ArrayList<>();
-            Bitmap bitmap = BitmapFactory.decodeResource(
-                    ContextUtils.getApplicationContext().getResources(),
-                    R.drawable.chrome_sync_logo);
-            bitmapList.add(bitmap);
-            callback.onResult(bitmapList);
+            callback.onResult(null);
         }
     }
-}
+}
\ No newline at end of file