[Android] Fix search action view for J devices

This CL corrects the unified search box on J devices as described in the
linked bugs.

To be more precise, the failure on J devices was coincidental: on those
devices, export was not available and therefore no overflow menu and
instead of hiding the overflow menu, the search box would be hidden.

The main changes are ensuring, that we only ever affect the overflow
menu, in case it exists and unifying the way action bar buttons change
appearance.

Bug: 868279, 868316
Change-Id: I969f5085262f4452327e445b2bb2e9286e20a956
Reviewed-on: https://chromium-review.googlesource.com/1154972Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579357}

chrome/android/java/src/org/chromium/chrome/browser/preferences/PreferenceUtils.java

@@ -18,6 +18,7 @@ import android.support.v7.widget.ActionMenuView;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewTreeObserver.OnScrollChangedListener;
+import android.widget.ImageView;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
@@ -95,8 +96,25 @@ public class PreferenceUtils {
         }
         if (menuView == null) return false;
         View overflowButton = menuView.getChildAt(menuView.getChildCount() - 1);
-        if (overflowButton == null) return false;
+        if (!isOverflowMenuButton(overflowButton, menuView)) return false;
         overflowButton.setVisibility(visibility);
         return true;
     }
+
+    /**
+     * There is no regular way to access the overflow button of an {@link ActionMenuView}.
+     * Checking whether a given view is an {@link ImageView} with the correct icon is an
+     * approximation to this issue as the exact icon that the parent menu will set is always known.
+     *
+     * @param button A view in the |parentMenu| that might be the overflow menu.
+     * @param parentMenu The menu that created the overflow button.
+     * @return True, if the given button can belong to the overflow menu. False otherwise.
+     */
+    private static boolean isOverflowMenuButton(View button, ActionMenuView parentMenu) {
+        if (button == null) return false;
+        if (!(button instanceof ImageView))
+            return false; // Normal items are usually TextView or LinearLayouts.
+        ImageView imageButton = (ImageView) button;
+        return imageButton.getDrawable() == parentMenu.getOverflowIcon();
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchUtils.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.preferences;
 
+import android.app.Activity;
 import android.support.annotation.NonNull;
 import android.support.v7.widget.SearchView;
 import android.view.MenuItem;
@@ -19,7 +20,7 @@ import javax.annotation.Nullable;
 public class SearchUtils {
     /**
      * This interface allows to react to changed search queries when initialized with
-     * {@link SearchUtils#initializeSearchView(MenuItem, String, QueryChangeListener)}.
+     * {@link SearchUtils#initializeSearchView(MenuItem, String, Activity, QueryChangeListener)}.
      */
     public interface QueryChangeListener {
         /**
@@ -35,11 +36,13 @@ public class SearchUtils {
      * search icon, box and close icon.
      * @param searchItem The existing item that can trigger the search action view.
      * @param initialQuery The query that the search field should be opened with.
+     * @param activity Optional. If set, overflow icons in the activity's action bar will be hidden.
      * @param changeListener The listener to be notified when the user changes the query.
      */
     public static void initializeSearchView(@NonNull MenuItem searchItem,
-            @Nullable String initialQuery, @NonNull QueryChangeListener changeListener) {
-        SearchView searchView = getSearchView(searchItem);
+            @Nullable String initialQuery, @Nullable Activity activity,
+            @NonNull QueryChangeListener changeListener) {
+        SearchView searchView = (SearchView) searchItem.getActionView();
         searchView.setImeOptions(EditorInfo.IME_FLAG_NO_FULLSCREEN);
 
         // Restore the search view if a query was recovered.
@@ -47,11 +50,12 @@ public class SearchUtils {
             searchItem.expandActionView();
             searchView.setIconified(false);
             searchView.setQuery(initialQuery, false);
+            updateActionBarButtons(searchItem, initialQuery, activity);
         }
 
         // Clicking the menu item hides the clear button and triggers search for an empty query.
         searchItem.setOnMenuItemClickListener((MenuItem m) -> {
-            updateSearchClearButtonVisibility(searchItem, "");
+            updateActionBarButtons(searchItem, "", activity);
             changeListener.onQueryTextChange("");
             return false; // Continue with the default action.
         });
@@ -60,13 +64,20 @@ public class SearchUtils {
         searchView.findViewById(org.chromium.chrome.R.id.search_close_btn)
                 .setOnClickListener((View v) -> {
                     searchView.setQuery("", false);
-                    updateSearchClearButtonVisibility(searchItem, "");
+                    updateActionBarButtons(searchItem, "", activity);
                     changeListener.onQueryTextChange("");
                 });
 
-        // Ensure that a changed search view triggers the search - independent from use code path.
+        // Ensure the clear button doesn't reappear with layout changes (e.g. keyboard visibility).
+        findSearchClearButton(searchView)
+                .addOnLayoutChangeListener(
+                        (view, i, i1, i2, i3, i4, i5, i6, i7)
+                                -> updateActionBarButtons(
+                                        searchItem, searchView.getQuery().toString(), activity));
+
+        // Ensure that a changed search view triggers the search - independent from used code path.
         searchView.setOnSearchClickListener(view -> {
-            updateSearchClearButtonVisibility(searchItem, "");
+            updateActionBarButtons(searchItem, "", activity);
             changeListener.onQueryTextChange("");
         });
         searchView.setOnQueryTextListener(new SearchView.OnQueryTextListener() {
@@ -78,7 +89,7 @@ public class SearchUtils {
             @Override
             public boolean onQueryTextChange(String query) {
                 // TODO(fhorschig) Exit early if a tracked query indicates no changes.
-                updateSearchClearButtonVisibility(searchItem, query);
+                updateActionBarButtons(searchItem, query, activity);
                 changeListener.onQueryTextChange(query);
                 return true; // Consume event.
             }
@@ -91,30 +102,38 @@ public class SearchUtils {
      * @param selectedItem The user-selected menu item.
      * @param searchItem The menu item known to contain the search view.
      * @param query The current search query.
+     * @param activity Optional. If set, overflow icons in the activity's action bar will be hidden.
      * @return Returns true if the item is a search item and could be handled. False otherwise.
      */
-    public static boolean handleSearchNavigation(
-            @NonNull MenuItem selectedItem, @NonNull MenuItem searchItem, @Nullable String query) {
+    public static boolean handleSearchNavigation(@NonNull MenuItem selectedItem,
+            @NonNull MenuItem searchItem, @Nullable String query, @Nullable Activity activity) {
         if (selectedItem.getItemId() != android.R.id.home || query == null) return false;
-        SearchView searchView = (SearchView) searchItem.getActionView();
-        searchView.setQuery(null, false);
-        searchView.setIconified(true);
-        searchItem.collapseActionView();
+        clearSearch(searchItem, activity);
         return true;
     }
 
     /**
-     * Shorthand to easily access a search item's action view.
-     * @param searchItem The menu item containing search item.
-     * @return The search view associated with the menu item.
+     * Reset a search item by clearing and collapsing it.
+     * @param searchItem The menu item that contains the search item.
+     * @param activity Optional. If set, overflow icons in the activity's action bar will be hidden.
      */
-    public static SearchView getSearchView(MenuItem searchItem) {
-        return (SearchView) searchItem.getActionView();
+    public static void clearSearch(@NonNull MenuItem searchItem, @Nullable Activity activity) {
+        SearchView searchView = (SearchView) searchItem.getActionView();
+        searchView.setQuery(null, false);
+        searchView.setIconified(true);
+        searchItem.collapseActionView();
+        updateActionBarButtons(searchItem, null, activity);
     }
 
-    private static void updateSearchClearButtonVisibility(MenuItem searchItem, String query) {
-        ImageView clearButton = findSearchClearButton(getSearchView(searchItem));
+    private static void updateActionBarButtons(
+            MenuItem searchItem, String query, @Nullable Activity activity) {
+        SearchView searchView = (SearchView) searchItem.getActionView();
+        ImageView clearButton = findSearchClearButton(searchView);
         clearButton.setVisibility(query == null || query.equals("") ? View.GONE : View.VISIBLE);
+        if (activity != null) {
+            PreferenceUtils.setOverflowMenuVisibility(
+                    activity, query != null ? View.GONE : View.VISIBLE);
+        }
     }
 
     private static ImageView findSearchClearButton(SearchView searchView) {

chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java

@@ -21,7 +21,6 @@ import android.text.style.ForegroundColorSpan;
 import android.view.Menu;
 import android.view.MenuInflater;
 import android.view.MenuItem;
-import android.view.View;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.VisibleForTesting;
@@ -32,7 +31,6 @@ import org.chromium.chrome.browser.preferences.ChromeBaseCheckBoxPreference;
 import org.chromium.chrome.browser.preferences.ChromeBasePreference;
 import org.chromium.chrome.browser.preferences.ChromeSwitchPreference;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
-import org.chromium.chrome.browser.preferences.PreferenceUtils;
 import org.chromium.chrome.browser.preferences.Preferences;
 import org.chromium.chrome.browser.preferences.PreferencesLauncher;
 import org.chromium.chrome.browser.preferences.SearchUtils;
@@ -145,7 +143,7 @@ public class SavePasswordsPreferences
         mSearchItem.setVisible(providesPasswordSearch());
         if (providesPasswordSearch()) {
             mHelpItem = menu.findItem(R.id.menu_id_general_help);
-            SearchUtils.initializeSearchView(mSearchItem, mSearchQuery, (query) -> {
+            SearchUtils.initializeSearchView(mSearchItem, mSearchQuery, getActivity(), (query) -> {
                 maybeRecordTriggeredPasswordSearch(true);
                 filterPasswords(query);
             });
@@ -178,7 +176,7 @@ public class SavePasswordsPreferences
             mExportFlow.startExporting();
             return true;
         }
-        if (SearchUtils.handleSearchNavigation(item, mSearchItem, mSearchQuery)) {
+        if (SearchUtils.handleSearchNavigation(item, mSearchItem, mSearchQuery, getActivity())) {
             filterPasswords(null);
             return true;
         }
@@ -187,13 +185,8 @@ public class SavePasswordsPreferences
 
     private void filterPasswords(String query) {
         mSearchQuery = query;
-        if (mSearchQuery == null) {
-            mHelpItem.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM);
-            PreferenceUtils.setOverflowMenuVisibility(getActivity(), View.VISIBLE);
-        } else {
-            mHelpItem.setShowAsAction(MenuItem.SHOW_AS_ACTION_NEVER);
-            PreferenceUtils.setOverflowMenuVisibility(getActivity(), View.GONE);
-        }
+        mHelpItem.setShowAsAction(mSearchQuery == null ? MenuItem.SHOW_AS_ACTION_IF_ROOM
+                                                       : MenuItem.SHOW_AS_ACTION_NEVER);
         rebuildPasswordLists();
     }
 

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleCategoryPreferences.java

@@ -328,7 +328,7 @@ public class SingleCategoryPreferences extends PreferenceFragment
         inflater.inflate(R.menu.website_preferences_menu, menu);
 
         mSearchItem = menu.findItem(R.id.search);
-        SearchUtils.initializeSearchView(mSearchItem, mSearch, (query) -> {
+        SearchUtils.initializeSearchView(mSearchItem, mSearch, getActivity(), (query) -> {
             mSearch = query;
             getInfoForOrigins();
         });
@@ -365,7 +365,7 @@ public class SingleCategoryPreferences extends PreferenceFragment
                     getActivity(), getString(helpContextResId), Profile.getLastUsedProfile(), null);
             return true;
         }
-        if (handleSearchNavigation(item, mSearchItem, mSearch)) {
+        if (handleSearchNavigation(item, mSearchItem, mSearch, getActivity())) {
             mSearch = null;
             getInfoForOrigins();
             return true;
@@ -382,13 +382,6 @@ public class SingleCategoryPreferences extends PreferenceFragment
             return false;
         }
 
-        if (mSearch != null) {
-            // Clear out any lingering searches, so that the full list is shown
-            // when coming back to this page.
-            mSearch = null;
-            SearchUtils.getSearchView(mSearchItem).setQuery("", false);
-        }
-
         if (preference instanceof WebsitePreference) {
             WebsitePreference website = (WebsitePreference) preference;
             website.setFragment(SingleWebsitePreferences.class.getName());
@@ -524,6 +517,10 @@ public class SingleCategoryPreferences extends PreferenceFragment
     public void onResume() {
         super.onResume();
 
+        if (mSearch == null && mSearchItem != null) {
+            SearchUtils.clearSearch(mSearchItem, getActivity());
+        }
+
         getInfoForOrigins();
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferencesTest.java

@@ -1787,8 +1787,8 @@ public class SavePasswordsPreferencesTest {
         Espresso.onView(isRoot()).check(
                 (root, e)
                         -> waitForView((ViewGroup) root,
-                                withContentDescription(
-                                        R.string.abc_action_menu_overflow_description),
+                                withParent(withContentDescription(
+                                        R.string.abc_action_menu_overflow_description)),
                                 VIEW_INVISIBLE | VIEW_GONE | VIEW_NULL));
 
         Espresso.onView(withContentDescription(R.string.abc_action_bar_up_description))