Remove actions from keyboard accessory when hiding it

This aims to fix the case in which when moving focus to a different
field (for which the action doesn't apply), the accessory is first
shown and then the action is removed. When this happens, there is
a perceivable delay before the action button disappears.

Bug: 856971
Change-Id: I1dfb63c513d109b74592870d3d63277b8237df25
Reviewed-on: https://chromium-review.googlesource.com/1142150
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579060}

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java

@@ -147,6 +147,9 @@ public class KeyboardAccessoryCoordinator {
     /**
      * Allows any {@link KeyboardAccessoryData.Provider} to communicate with the
      * {@link KeyboardAccessoryMediator} of this component.
+     *
+     * Note that the provided actions are removed when the accessory is hidden.
+     *
      * @param provider The object providing action lists to observers in this component.
      */
     public void registerActionListProvider(

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryMediator.java

@@ -10,6 +10,7 @@ import android.support.design.widget.TabLayout;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.autofill.AutofillKeyboardSuggestions;
 import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryCoordinator.VisibilityDelegate;
+import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.Action;
 import org.chromium.chrome.browser.modelutil.ListObservable;
 import org.chromium.chrome.browser.modelutil.PropertyObservable;
 import org.chromium.ui.base.WindowAndroid;
@@ -126,6 +127,9 @@ class KeyboardAccessoryMediator
         if (propertyKey == KeyboardAccessoryModel.PropertyKey.VISIBLE) {
             // When the accessory just (dis)appeared, there should be no active tab.
             mModel.setActiveTab(null);
+            if (!mModel.isVisible()) {
+                mModel.setActions(new Action[0]);
+            }
             return;
         }
         if (propertyKey == KeyboardAccessoryModel.PropertyKey.ACTIVE_TAB) {

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
 import android.graphics.Bitmap;
-import android.support.annotation.Nullable;
 
 import org.chromium.base.Callback;
 import org.chromium.base.annotations.CalledByNative;
@@ -22,7 +21,6 @@ class PasswordAccessoryBridge {
             new KeyboardAccessoryData.PropertyProvider<>();
     private final ManualFillingCoordinator mManualFillingCoordinator;
     private final ChromeActivity mActivity;
-    private @Nullable Action mGenerationAction;
     private long mNativeView;
 
     private PasswordAccessoryBridge(long nativeView, WindowAndroid windowAndroid) {
@@ -48,9 +46,6 @@ class PasswordAccessoryBridge {
     private void onAutomaticGenerationStatusChanged(boolean available) {
         final Action[] generationAction;
         if (available) {
-            if (mGenerationAction != null) {
-                return;
-            }
             // This is meant to suppress the warning that the short string is not used.
             // TODO(crbug.com/855581): Switch between strings based on whether they fit on the
             // screen or not.
@@ -58,16 +53,12 @@ class PasswordAccessoryBridge {
             String caption = useLongString
                     ? mActivity.getString(R.string.password_generation_accessory_button)
                     : mActivity.getString(R.string.password_generation_accessory_button_short);
-
-            mGenerationAction = new Action(caption, (action) -> {
+            generationAction = new Action[] {new Action(caption, (action) -> {
                 assert mNativeView
                         != 0 : "Controller has been destroyed but the bridge wasn't cleaned up!";
                 nativeOnGenerationRequested(mNativeView);
-            });
-            generationAction = new Action[] {mGenerationAction};
+            })};
         } else {
-            if (mGenerationAction == null) return;
-            mGenerationAction = null;
             generationAction = new Action[0];
         }
         mActionProvider.notifyObservers(generationAction);

chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java

@@ -203,6 +203,17 @@ public class KeyboardAccessoryControllerTest {
         assertThat(mModel.isVisible(), is(true));
     }
 
+    @Test
+    public void testActionsRemovedWhenNotVisible() {
+        // Make the accessory visible and add an action to it.
+        mMediator.keyboardVisibilityChanged(true);
+        mModel.getActionList().add(new Action(null, null));
+
+        // Hiding the accessory should also remove actions.
+        mMediator.keyboardVisibilityChanged(false);
+        assertThat(mModel.getActionList().size(), is(0));
+    }
+
     @Test
     public void testIsVisibleWithTabs() {
         // Without any actions, the accessory should remain invisible.