[Mfill Android] Ensure accessory reports keyboard replacements timely

Before this CL, components like the infobar would query the state of
the keyboard as soon as the activity is resized. That means, the
accessory might not have updated the state of filling components (and
immediate updates might cause relayouts) and an old keyboard state is
reported.

Now, the accessory reports the state of keyboard replacements before the
actual component is updated. This way, replacements can be considered
for keyboard visibility, even if the component updates are pending.

Bug: 903256
Change-Id: Ie95297b6dbdfc1929aaf0643a805fbb4fd93378b
Reviewed-on: https://chromium-review.googlesource.com/c/1329246
Commit-Queue: Friedrich Horschig [CET] <fhorschig@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607671}

chrome/android/java/src/org/chromium/chrome/browser/ChromeKeyboardVisibilityDelegate.java

@@ -59,7 +59,7 @@ public class ChromeKeyboardVisibilityDelegate extends SingleWindowKeyboardVisibi
         boolean wasManualFillingViewShowing = false;
         if (activity != null) {
             wasManualFillingViewShowing =
-                    activity.getManualFillingController().isFillingViewShown();
+                    activity.getManualFillingController().isFillingViewShown(view);
             activity.getManualFillingController().hide();
         }
         return super.hideKeyboard(view) || wasManualFillingViewShowing;
@@ -69,6 +69,7 @@ public class ChromeKeyboardVisibilityDelegate extends SingleWindowKeyboardVisibi
     public boolean isKeyboardShowing(Context context, View view) {
         ChromeActivity activity = getActivity();
         return super.isKeyboardShowing(context, view)
-                || (activity != null && activity.getManualFillingController().isFillingViewShown());
+                || (activity != null
+                           && activity.getManualFillingController().isFillingViewShown(view));
     }
 }
\ No newline at end of file

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

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
 import android.support.annotation.Nullable;
 import android.support.v4.view.ViewPager;
+import android.view.View;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.Provider;
@@ -141,10 +142,11 @@ public class ManualFillingCoordinator {
     }
 
     /**
-     * Returns whether - at this very moment - the Keyboard is replaced by an accessory sheet.
-     * @return True if an accessory sheet is open and replacing the keyboard.
+     * Returns whether the Keyboard is replaced by an accessory sheet or is about to do so.
+     * @return True if an accessory sheet is (being) opened and replacing the keyboard.
+     * @param view A {@link View} that is used to find the window root.
      */
-    public boolean isFillingViewShown() {
-        return mMediator.isFillingViewShown();
+    public boolean isFillingViewShown(View view) {
+        return mMediator.isFillingViewShown(view);
     }
 }
\ No newline at end of file

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

@@ -32,7 +32,6 @@ import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.chrome.browser.tabmodel.TabModelObserver;
 import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver;
 import org.chromium.ui.DropdownPopupWindow;
-import org.chromium.ui.KeyboardVisibilityDelegate;
 import org.chromium.ui.base.WindowAndroid;
 
 import java.util.HashMap;
@@ -185,16 +184,17 @@ class ManualFillingMediator extends EmptyTabObserver
         return mWindowAndroid != null;
     }
 
-    boolean isFillingViewShown() {
-        return mAccessorySheet != null && mAccessorySheet.isShown();
+    boolean isFillingViewShown(View view) {
+        if (!isInitialized()) return false;
+        boolean isSoftInputShowing = getKeyboard().isSoftKeyboardShowing(mActivity, view);
+        return !isSoftInputShowing && mKeyboardAccessory.hasActiveTab();
     }
 
     @Override
     public void onLayoutChange(View view, int left, int top, int right, int bottom, int oldLeft,
             int oldTop, int oldRight, int oldBottom) {
         if (mActivity == null) return; // Activity has been cleaned up already.
-        onKeyboardVisibilityPossiblyChanged(
-                getKeyboardDelegate().isSoftKeyboardShowing(mActivity, view));
+        onKeyboardVisibilityPossiblyChanged(getKeyboard().isSoftKeyboardShowing(mActivity, view));
     }
 
     private void onKeyboardVisibilityPossiblyChanged(boolean isShowing) {
@@ -387,8 +387,9 @@ class ManualFillingMediator extends EmptyTabObserver
     }
 
     private ChromeKeyboardVisibilityDelegate getKeyboard() {
-        KeyboardVisibilityDelegate delegate = mWindowAndroid.getKeyboardDelegate();
-        return (ChromeKeyboardVisibilityDelegate) delegate;
+        assert mWindowAndroid instanceof ChromeWindow;
+        assert mWindowAndroid.getKeyboardDelegate() instanceof ChromeKeyboardVisibilityDelegate;
+        return (ChromeKeyboardVisibilityDelegate) mWindowAndroid.getKeyboardDelegate();
     }
 
     private AccessoryState getOrCreateAccessoryState(Tab tab) {
@@ -432,12 +433,6 @@ class ManualFillingMediator extends EmptyTabObserver
                 org.chromium.chrome.R.dimen.keyboard_accessory_suggestion_height);
     }
 
-    private ChromeKeyboardVisibilityDelegate getKeyboardDelegate() {
-        assert mWindowAndroid instanceof ChromeWindow;
-        assert mWindowAndroid.getKeyboardDelegate() instanceof ChromeKeyboardVisibilityDelegate;
-        return (ChromeKeyboardVisibilityDelegate) mWindowAndroid.getKeyboardDelegate();
-    }
-
     @VisibleForTesting
     void addTab(KeyboardAccessoryData.Tab tab) {
         if (!isInitialized()) return;

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

@@ -10,12 +10,15 @@ import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import static org.chromium.chrome.browser.autofill.keyboard_accessory.AccessoryAction.GENERATE_PASSWORD_AUTOMATIC;
+import static org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryProperties.ACTIVE_TAB;
 import static org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryProperties.TABS;
 import static org.chromium.chrome.browser.tab.Tab.INVALID_TAB_ID;
 import static org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType.FROM_BROWSER_ACTIONS;
@@ -41,6 +44,8 @@ import org.chromium.base.metrics.test.ShadowRecordHistogram;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.ChromeKeyboardVisibilityDelegate;
+import org.chromium.chrome.browser.ChromeWindow;
 import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.Action;
 import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.Item;
 import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.PropertyProvider;
@@ -56,7 +61,7 @@ import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
 import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
 import org.chromium.chrome.test.util.browser.modelutil.FakeViewProvider;
-import org.chromium.ui.base.ActivityWindowAndroid;
+import org.chromium.ui.KeyboardVisibilityDelegate;
 
 import java.lang.ref.WeakReference;
 import java.util.Map;
@@ -70,7 +75,7 @@ import java.util.Map;
 @DisableFeatures(ChromeFeatureList.EXPERIMENTAL_UI)
 public class ManualFillingControllerTest {
     @Mock
-    private ActivityWindowAndroid mMockWindow;
+    private ChromeWindow mMockWindow;
     @Mock
     private ChromeActivity mMockActivity;
     @Mock
@@ -89,6 +94,8 @@ public class ManualFillingControllerTest {
     private Drawable mMockIcon;
     @Mock
     private android.content.res.Resources mMockResources;
+    @Mock
+    private ChromeKeyboardVisibilityDelegate mMockKeyboard;
 
     @Rule
     public Features.JUnitProcessor mFeaturesProcessor = new Features.JUnitProcessor();
@@ -101,6 +108,8 @@ public class ManualFillingControllerTest {
     public void setUp() {
         ShadowRecordHistogram.reset();
         MockitoAnnotations.initMocks(this);
+        KeyboardVisibilityDelegate.setInstance(mMockKeyboard);
+        when(mMockWindow.getKeyboardDelegate()).thenReturn(mMockKeyboard);
         when(mMockWindow.getActivity()).thenReturn(new WeakReference<>(mMockActivity));
         when(mMockActivity.getTabModelSelector()).thenReturn(mMockTabModelSelector);
         ChromeFullscreenManager fullscreenManager = new ChromeFullscreenManager(mMockActivity, 0);
@@ -478,6 +487,27 @@ public class ManualFillingControllerTest {
         assertThat(mediator.getPasswordAccessorySheet(), is(nullValue()));
     }
 
+    @Test
+    public void testIsFillingViewShownReturnsTargetValueAheadOfComponentUpdate() {
+        // After initialization with one tab, the accessory sheet is closed.
+        KeyboardAccessoryCoordinator accessory =
+                mController.getMediatorForTesting().getKeyboardAccessory();
+        mController.getMediatorForTesting().addTab(
+                new KeyboardAccessoryData.Tab(null, null, 0, 0, null));
+        accessory.requestShowing();
+        assertThat(mController.isFillingViewShown(null), is(false));
+
+        // As soon as active tab and keyboard change, |isFillingViewShown| returns the expected
+        // state - even if the sheet component wasn't updated yet.
+        accessory.getMediatorForTesting().getModelForTesting().set(ACTIVE_TAB, 0);
+        when(mMockKeyboard.isSoftKeyboardShowing(eq(mMockActivity), any())).thenReturn(false);
+        assertThat(mController.isFillingViewShown(null), is(true));
+
+        // The layout change impacts the component, but not the coordinator method.
+        mController.getMediatorForTesting().onLayoutChange(null, 0, 0, 0, 0, 0, 0, 0, 0);
+        assertThat(mController.isFillingViewShown(null), is(true));
+    }
+
     /**
      * Creates a tab and calls the observer events as if it was just created and switched to.
      * @param mediator The {@link ManualFillingMediator} whose observers should be triggered.