Wire view component to KeyboardAccessory

This CL completes the MVC structure of the keyboard accessory component.
The accessory view is changed only as reaction to model changes.
It now lives in the ChromeActivity and is properly destroyed like other
high-level UI components.

Bug: 828832
Change-Id: I8dac0e79a6032abc6df36014bf9925df9a53bc92
Reviewed-on: https://chromium-review.googlesource.com/1013702Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554330}

chrome/android/java/res/layout/keyboard_accessory.xml

+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2015 The Chromium Authors. All rights reserved.
+     Use of this source code is governed by a BSD-style license that can be
+     found in the LICENSE file. -->
+
+<org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryView
+    xmlns:android="http://schemas.android.com/apk/res/android"
+    android:id="@+id/keyboard_accessory"
+    android:layout_gravity="start|bottom"
+    android:background="@drawable/keyboard_accessory_background"
+    android:contentDescription="@string/autofill_keyboard_accessory_content_description"
+    android:fillViewport="true"
+    android:scrollbars="none"
+    android:visibility="gone"
+    android:orientation="horizontal"
+    android:layout_height="@dimen/keyboard_accessory_height"
+    android:layout_width="match_parent"
+    android:paddingEnd="@dimen/keyboard_accessory_padding"
+    android:paddingStart="@dimen/keyboard_accessory_padding">
+
+    <android.support.v7.widget.RecyclerView
+        android:id="@+id/tabs"
+        android:layout_width="wrap_content"
+        android:layout_height="match_parent" />
+
+    <android.support.v7.widget.RecyclerView
+        android:id="@+id/actions_view"
+        android:layout_width="wrap_content"
+        android:layout_height="match_parent" />
+
+    <HorizontalScrollView
+        android:id="@+id/suggestions_view"
+        android:layout_width="wrap_content"
+        android:layout_height="match_parent" />
+
+</org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryView>

chrome/android/java/res/layout/main.xml

@@ -27,17 +27,13 @@
             android:layout_height="wrap_content"
             android:layout_gravity="start|bottom">
 
-            <LinearLayout
-                android:id="@+id/keyboard_accessory"
+            <ViewStub
+                android:id="@+id/keyboard_accessory_stub"
+                android:inflatedId="@+id/keyboard_accessory"
+                android:layout="@layout/keyboard_accessory"
                 android:layout_height="@dimen/keyboard_accessory_height"
                 android:layout_width="match_parent"
-                android:layout_gravity="start|bottom"
-                android:background="@drawable/keyboard_accessory_background"
-                android:contentDescription="@string/autofill_keyboard_accessory_content_description"
-                android:fillViewport="true"
-                android:scrollbars="none"
-                android:visibility="gone"
-                android:orientation="horizontal" />
+                android:layout_gravity="start|bottom"/>
 
         </org.chromium.chrome.browser.snackbar.BottomContainer>
 

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

@@ -57,6 +57,7 @@ import org.chromium.chrome.browser.appmenu.AppMenu;
 import org.chromium.chrome.browser.appmenu.AppMenuHandler;
 import org.chromium.chrome.browser.appmenu.AppMenuObserver;
 import org.chromium.chrome.browser.appmenu.AppMenuPropertiesDelegate;
+import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryCoordinator;
 import org.chromium.chrome.browser.bookmarks.BookmarkModel;
 import org.chromium.chrome.browser.bookmarks.BookmarkUtils;
 import org.chromium.chrome.browser.compositor.CompositorViewHolder;
@@ -262,6 +263,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
     private BottomSheet mBottomSheet;
     private ContextualSuggestionsCoordinator mContextualSuggestionsCoordinator;
     private FadingBackgroundView mFadingBackgroundView;
+    private KeyboardAccessoryCoordinator mKeyboardAccessoryCoordinator;
 
     // Time in ms that it took took us to inflate the initial layout
     private long mInflateInitialLayoutDurationMs;
@@ -371,9 +373,9 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
         // SurfaceView's 'hole' clipping during animations that are notified to the window.
         getWindowAndroid().setAnimationPlaceholderView(mCompositorViewHolder.getCompositorView());
 
-        // Inform the WindowAndroid of the keyboard accessory view.
-        getWindowAndroid().setKeyboardAccessoryView(
-                (ViewGroup) findViewById(R.id.keyboard_accessory));
+        mKeyboardAccessoryCoordinator = new KeyboardAccessoryCoordinator(
+                getWindowAndroid(), findViewById(R.id.keyboard_accessory_stub));
+
         initializeToolbar();
         initializeTabModels();
         if (!isFinishing() && getFullscreenManager() != null) {
@@ -651,6 +653,13 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
         return mFindToolbarManager;
     }
 
+    /**
+     * @return The {@link KeyboardAccessoryCoordinator} that belongs to this activity.
+     */
+    public KeyboardAccessoryCoordinator getKeyboardAccessory() {
+        return mKeyboardAccessoryCoordinator;
+    }
+
     /**
      * @return The resource id for the menu to use in {@link AppMenu}. Default is R.menu.main_menu.
      */
@@ -1185,6 +1194,11 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
             mTabContentManager = null;
         }
 
+        if (mKeyboardAccessoryCoordinator != null) {
+            mKeyboardAccessoryCoordinator.destroy();
+            mKeyboardAccessoryCoordinator = null;
+        }
+
         AccessibilityManager manager = (AccessibilityManager)
                 getBaseContext().getSystemService(Context.ACCESSIBILITY_SERVICE);
         manager.removeAccessibilityStateChangeListener(this);

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

@@ -152,6 +152,7 @@ public abstract class ChromeFeatureList {
     public static final String ANDROID_PAY_INTEGRATION_V1 = "AndroidPayIntegrationV1";
     public static final String ANDROID_PAY_INTEGRATION_V2 = "AndroidPayIntegrationV2";
     public static final String ANDROID_PAYMENT_APPS = "AndroidPaymentApps";
+    public static final String AUTOFILL_KEYBOARD_ACCESSORY = "AutofillKeyboardAccessory";
     public static final String AUTOFILL_SCAN_CARDHOLDER_NAME = "AutofillScanCardholderName";
     public static final String CAF_MEDIA_ROUTER_IMPL = "CafMediaRouterImpl";
     public static final String CAPTIVE_PORTAL_CERTIFICATE_LIST = "CaptivePortalCertificateList";

chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryBridge.java

@@ -11,8 +11,9 @@ import android.support.v7.app.AlertDialog;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ResourceId;
-import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryView;
+import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryCoordinator;
 import org.chromium.components.autofill.AutofillDelegate;
 import org.chromium.components.autofill.AutofillSuggestion;
 import org.chromium.ui.DropdownItem;
@@ -27,7 +28,7 @@ import org.chromium.ui.base.WindowAndroid;
 public class AutofillKeyboardAccessoryBridge
         implements AutofillDelegate, DialogInterface.OnClickListener {
     private long mNativeAutofillKeyboardAccessory;
-    private KeyboardAccessoryView mAccessoryView;
+    private KeyboardAccessoryCoordinator mKeyboardAccessory;
     private Context mContext;
     private AutofillKeyboardSuggestions mAutofillSuggestions;
 
@@ -81,18 +82,15 @@ public class AutofillKeyboardAccessoryBridge
     @CalledByNative
     private void init(long nativeAutofillKeyboardAccessory, WindowAndroid windowAndroid,
             int animationDurationMillis, boolean shouldLimitLabelWidth) {
-        if (windowAndroid == null || windowAndroid.getActivity().get() == null) {
-            nativeViewDismissed(nativeAutofillKeyboardAccessory);
-            dismissed();
-            return;
+        mContext = windowAndroid.getActivity().get();
+        assert mContext != null;
+        if (mContext instanceof ChromeActivity) {
+            mKeyboardAccessory = ((ChromeActivity) mContext).getKeyboardAccessory();
         }
 
         mNativeAutofillKeyboardAccessory = nativeAutofillKeyboardAccessory;
-        mAccessoryView = new KeyboardAccessoryView(windowAndroid);
         mAutofillSuggestions =
                 new AutofillKeyboardSuggestions(windowAndroid, this, shouldLimitLabelWidth);
-        mAccessoryView.setSuggestions(mAutofillSuggestions);
-        mContext = windowAndroid.getActivity().get();
     }
 
     /**
@@ -108,8 +106,10 @@ public class AutofillKeyboardAccessoryBridge
      */
     @CalledByNative
     private void dismiss() {
-        if (mAccessoryView != null) mAccessoryView.dismiss();
+        if (mKeyboardAccessory != null) mKeyboardAccessory.dismiss();
         mContext = null;
+        mKeyboardAccessory = null;
+        mAutofillSuggestions = null;
     }
 
     /**
@@ -118,8 +118,11 @@ public class AutofillKeyboardAccessoryBridge
      */
     @CalledByNative
     private void show(AutofillSuggestion[] suggestions, boolean isRtl) {
-        if (mAccessoryView != null) mAccessoryView.show();
         if (mAutofillSuggestions != null) mAutofillSuggestions.setSuggestions(suggestions, isRtl);
+        if (mKeyboardAccessory != null) {
+            mKeyboardAccessory.setSuggestions(mAutofillSuggestions);
+            mKeyboardAccessory.show();
+        }
     }
 
     // Helper methods for AutofillSuggestion. These are copied from AutofillPopupBridge (which

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

@@ -4,7 +4,15 @@
 
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
+import android.view.ViewStub;
+
 import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.browser.autofill.AutofillKeyboardSuggestions;
+import org.chromium.chrome.browser.modelutil.PropertyModelChangeProcessor;
+import org.chromium.chrome.browser.modelutil.RecyclerViewAdapter;
+import org.chromium.chrome.browser.modelutil.RecyclerViewModelChangeProcessor;
+import org.chromium.ui.UiUtils;
+import org.chromium.ui.base.WindowAndroid;
 
 /**
  * Creates and owns all elements which are part of the keyboard accessory component.
@@ -12,26 +20,52 @@ import org.chromium.base.VisibleForTesting;
  * or showing the accessory) to the {@link KeyboardAccessoryMediator}.
  */
 public class KeyboardAccessoryCoordinator {
-    private final KeyboardAccessoryMediator mMediator =
-            new KeyboardAccessoryMediator(new KeyboardAccessoryModel());
-
-    @VisibleForTesting
-    KeyboardAccessoryMediator getMediatorForTesting() {
-        return mMediator;
-    }
+    private final KeyboardAccessoryModel mModel = new KeyboardAccessoryModel();
+    private final KeyboardAccessoryMediator mMediator;
+    private KeyboardAccessoryViewBinder.AccessoryViewHolder mViewHolder;
+    private PropertyModelChangeProcessor<KeyboardAccessoryModel,
+            KeyboardAccessoryViewBinder.AccessoryViewHolder, KeyboardAccessoryModel.PropertyKey>
+            mModelChangeProcessor;
 
-    public void hide() {
-        mMediator.hide();
+    /**
+     * Initializes the component as soon as the native library is loaded by e.g. starting to listen
+     * to keyboard visibility events.
+     * @param windowAndroid The window connected to the activity this component lives in.
+     * @param viewStub the stub that will become the accessory.
+     */
+    public KeyboardAccessoryCoordinator(WindowAndroid windowAndroid, ViewStub viewStub) {
+        mMediator = new KeyboardAccessoryMediator(mModel, windowAndroid);
+        mViewHolder = new KeyboardAccessoryViewBinder.AccessoryViewHolder(viewStub);
+        mModelChangeProcessor = new PropertyModelChangeProcessor<>(
+                mModel, mViewHolder, new KeyboardAccessoryViewBinder());
+        mModel.addObserver(mModelChangeProcessor);
+        mModel.addTabListObserver(new RecyclerViewModelChangeProcessor<>(
+                new RecyclerViewAdapter<>(mModel.getTabList())));
+        mModel.addActionListObserver(new RecyclerViewModelChangeProcessor<>(
+                new RecyclerViewAdapter<>(mModel.getActionList())));
     }
 
+    /**
+     * Show the keyboard accessory. Independent from the keyboard.
+     */
     public void show() {
         mMediator.show();
     }
 
+    /**
+     * A {@link KeyboardAccessoryData.Tab} passed into this function will be represented as item at
+     * the start of the accessory. It is meant to trigger various bottom sheets.
+     * @param tab The tab which contains representation data and links back to a bottom sheet.
+     */
     public void addTab(KeyboardAccessoryData.Tab tab) {
         mMediator.addTab(tab);
     }
 
+    /**
+     * The {@link KeyboardAccessoryData.Tab} passed into this function will be completely removed
+     * from the accessory.
+     * @param tab The tab to be removed.
+     */
     public void removeTab(KeyboardAccessoryData.Tab tab) {
         mMediator.removeTab(tab);
     }
@@ -44,4 +78,36 @@ public class KeyboardAccessoryCoordinator {
     public void registerActionListProvider(KeyboardAccessoryData.ActionListProvider provider) {
         provider.addObserver(mMediator);
     }
+
+    /**
+     * Used to clean up the accessory by e.g. unregister listeners to keyboard visibility.
+     */
+    public void destroy() {
+        mMediator.destroy();
+    }
+
+    /**
+     * TODO(fhorschig): Remove this function. The suggestions bridge should become a provider.
+     * Sets a View that will be displayed in a scroll view at the end of the accessory.
+     * @param suggestions The suggestions to be rendered into the accessory.
+     */
+    public void setSuggestions(AutofillKeyboardSuggestions suggestions) {
+        mMediator.setSuggestions(suggestions);
+    }
+
+    /**
+     * TODO(fhorschig): Remove this function. The accessory probably shouldn't know this concept.
+     * Dismisses the accessory by hiding it's view, clearing potentially left over suggestions and
+     * hiding the keyboard.
+     */
+    public void dismiss() {
+        mMediator.dismiss();
+        // TODO(fhorschig): Move hiding to upcoming root controller, drop it or reassign to bauerb@.
+        UiUtils.hideKeyboard(mViewHolder.getView());
+    }
+
+    @VisibleForTesting
+    KeyboardAccessoryMediator getMediatorForTesting() {
+        return mMediator;
+    }
 }

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

@@ -5,6 +5,8 @@
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
 import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.browser.autofill.AutofillKeyboardSuggestions;
+import org.chromium.ui.base.WindowAndroid;
 
 /**
  * This is the second part of the controller of the keyboard accessory component.
@@ -14,11 +16,19 @@ import org.chromium.base.VisibleForTesting;
  * generating passwords) and lets the {@link KeyboardAccessoryModel} know of these actions and which
  * callback to trigger when selecting them.
  */
-class KeyboardAccessoryMediator implements KeyboardAccessoryData.ActionListObserver {
+class KeyboardAccessoryMediator implements KeyboardAccessoryData.ActionListObserver,
+                                           WindowAndroid.KeyboardVisibilityListener {
     private final KeyboardAccessoryModel mModel;
+    private final WindowAndroid mWindowAndroid;
 
-    KeyboardAccessoryMediator(KeyboardAccessoryModel keyboardAccessoryModel) {
-        mModel = keyboardAccessoryModel;
+    KeyboardAccessoryMediator(KeyboardAccessoryModel model, WindowAndroid windowAndroid) {
+        mModel = model;
+        mWindowAndroid = windowAndroid;
+        windowAndroid.addKeyboardVisibilityListener(this);
+    }
+
+    void destroy() {
+        mWindowAndroid.removeKeyboardVisibilityListener(this);
     }
 
     @Override
@@ -26,6 +36,13 @@ class KeyboardAccessoryMediator implements KeyboardAccessoryData.ActionListObser
         mModel.setActions(actions);
     }
 
+    @Override
+    public void keyboardVisibilityChanged(boolean isShowing) {
+        if (!isShowing) { // TODO(fhorschig): ... and no bottom sheet.
+            hide();
+        }
+    }
+
     void hide() {
         mModel.setVisible(false);
     }
@@ -42,6 +59,18 @@ class KeyboardAccessoryMediator implements KeyboardAccessoryData.ActionListObser
         mModel.removeTab(tab);
     }
 
+    void setSuggestions(AutofillKeyboardSuggestions suggestions) {
+        mModel.setAutofillSuggestions(suggestions);
+    }
+
+    void dismiss() {
+        hide();
+        if (mModel.getAutofillSuggestions() != null) {
+            mModel.getAutofillSuggestions().dismiss();
+        }
+        mModel.setAutofillSuggestions(null);
+    }
+
     @VisibleForTesting
     KeyboardAccessoryModel getModelForTesting() {
         return mModel;

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

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
+import org.chromium.chrome.browser.autofill.AutofillKeyboardSuggestions;
 import org.chromium.chrome.browser.modelutil.ListObservable;
 import org.chromium.chrome.browser.modelutil.PropertyObservable;
 
@@ -21,8 +22,16 @@ import java.util.List;
 class KeyboardAccessoryModel extends PropertyObservable<KeyboardAccessoryModel.PropertyKey> {
     /** Keys uniquely identifying model properties. */
     static class PropertyKey {
+        // This list contains all properties and is only necessary because the view needs to
+        // iterate over all properties when it's created to ensure it is in sync with this model.
+        static final List<PropertyKey> ALL_PROPERTIES = new ArrayList<>();
+
         static final PropertyKey VISIBLE = new PropertyKey();
-        private PropertyKey() {}
+        static final PropertyKey SUGGESTIONS = new PropertyKey();
+
+        private PropertyKey() {
+            ALL_PROPERTIES.add(this);
+        }
     }
 
     /** A {@link ListObservable} containing an {@link ArrayList} of Tabs or Actions. */
@@ -76,6 +85,9 @@ class KeyboardAccessoryModel extends PropertyObservable<KeyboardAccessoryModel.P
     private SimpleListObservable<KeyboardAccessoryData.Tab> mTabListObservable;
     private boolean mVisible;
 
+    // TODO(fhorschig): Ideally, make this a ListObservable populating a RecyclerView.
+    private AutofillKeyboardSuggestions mAutofillSuggestions;
+
     KeyboardAccessoryModel() {
         mActionListObservable = new SimpleListObservable<>();
         mTabListObservable = new SimpleListObservable<>();
@@ -110,6 +122,7 @@ class KeyboardAccessoryModel extends PropertyObservable<KeyboardAccessoryModel.P
     }
 
     void setVisible(boolean visible) {
+        if (mVisible == visible) return; // Nothing to do here: same value.
         mVisible = visible;
         notifyPropertyChanged(PropertyKey.VISIBLE);
     }
@@ -117,4 +130,14 @@ class KeyboardAccessoryModel extends PropertyObservable<KeyboardAccessoryModel.P
     boolean isVisible() {
         return mVisible;
     }
+
+    AutofillKeyboardSuggestions getAutofillSuggestions() {
+        return mAutofillSuggestions;
+    }
+
+    void setAutofillSuggestions(AutofillKeyboardSuggestions autofillSuggestions) {
+        if (autofillSuggestions == mAutofillSuggestions) return; // Nothing to do: same object.
+        mAutofillSuggestions = autofillSuggestions;
+        notifyPropertyChanged(PropertyKey.SUGGESTIONS);
+    }
 }

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

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
 import static org.chromium.ui.base.LocalizationUtils.isLayoutRtl;
 
+import android.content.Context;
+import android.util.AttributeSet;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.accessibility.AccessibilityEvent;
@@ -13,85 +15,70 @@ import android.widget.HorizontalScrollView;
 import android.widget.LinearLayout;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.chrome.R;
 import org.chromium.chrome.browser.autofill.AutofillKeyboardSuggestions;
-import org.chromium.ui.UiUtils;
-import org.chromium.ui.base.WindowAndroid;
+
+import javax.annotation.Nullable;
 
 /**
  * The Accessory sitting above the keyboard and below the content area. It is used for autofill
  * suggestions and manual entry points assisting the user in filling forms.
  */
-public class KeyboardAccessoryView
-        extends LinearLayout implements WindowAndroid.KeyboardVisibilityListener {
-    private final WindowAndroid mWindowAndroid;
-
-    private AutofillKeyboardSuggestions mAutofillSuggestions;
-
-    // Boolean to track if the keyboard accessory has just popped up or has already been showing.
-    private boolean mFirstAppearance;
+class KeyboardAccessoryView extends LinearLayout {
+    private HorizontalScrollView mSuggestionsView;
 
     /**
-     * Creates an AutofillKeyboardAccessory with specified parameters.
-     * @param windowAndroid The owning WindowAndroid.
+     * Constructor for inflating from XML.
      */
-    public KeyboardAccessoryView(WindowAndroid windowAndroid) {
-        super(windowAndroid.getActivity().get());
-        assert windowAndroid.getActivity().get() != null;
-        mWindowAndroid = windowAndroid;
-        mWindowAndroid.addKeyboardVisibilityListener(this);
+    public KeyboardAccessoryView(Context context, AttributeSet attrs) {
+        super(context, attrs);
     }
 
-    // TODO(crbug/722897): Check to handle RTL.
-    public void show() {
-        removeAllViews();
-        if (mAutofillSuggestions != null) {
-            HorizontalScrollView scrollView = new HorizontalScrollView(getContext());
-            scrollView.scrollTo(scrollView.getRight(), 0);
-            ApiCompatibilityUtils.setLayoutDirection(scrollView,
+    @Override
+    protected void onFinishInflate() {
+        super.onFinishInflate();
+        sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED);
+
+        mSuggestionsView = findViewById(R.id.suggestions_view);
+
+        // Apply RTL layout changes to the views childen:
+        ApiCompatibilityUtils.setLayoutDirection(mSuggestionsView,
                 isLayoutRtl() ? View.LAYOUT_DIRECTION_RTL : View.LAYOUT_DIRECTION_LTR);
-            scrollView.addView(mAutofillSuggestions);
-            addView(scrollView);
     }
 
-        final ViewGroup container = mWindowAndroid.getKeyboardAccessoryView();
-
-        if (getParent() == null) {
-            container.addView(this);
-            container.setVisibility(View.VISIBLE);
-            container.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED);
+    @Override
+    protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
+        super.onLayout(changed, left, top, right, bottom);
+        // When the size changes, the scrolling should be reset.
+        mSuggestionsView.fullScroll(isLayoutRtl() ? FOCUS_RIGHT : FOCUS_LEFT);
     }
-        if (mFirstAppearance) {
-            announceForAccessibility(container.getContentDescription());
+
+    void setVisible(boolean visible) {
+        if (visible) {
+            show();
+        } else {
+            hide();
         }
     }
 
+    // TODO(crbug/722897): Check to handle RTL.
+    // TODO(fhorschig): This should use a RecyclerView. The model should contain single suggestions.
     /**
-     * Shows the given suggestions.
+     * Shows the given suggestions. If set to null, it only removes existing suggestions.
      * @param suggestions Autofill suggestion data.
      */
-    public void setSuggestions(AutofillKeyboardSuggestions suggestions) {
-        mAutofillSuggestions = suggestions;
+    void updateSuggestions(@Nullable AutofillKeyboardSuggestions suggestions) {
+        mSuggestionsView.removeAllViews();
+        if (suggestions == null) return;
+        mSuggestionsView.addView(suggestions);
     }
 
-    /**
-     * Called to hide the suggestion view.
-     */
-    public void dismiss() {
-        UiUtils.hideKeyboard(this); // TODO(fhorschig): Double-check for autofill sheet.
-        if (mAutofillSuggestions != null) mAutofillSuggestions.dismiss();
-        ViewGroup container = mWindowAndroid.getKeyboardAccessoryView();
-        container.removeView(this);
-        container.setVisibility(View.GONE);
-        mWindowAndroid.removeKeyboardVisibilityListener(this);
-        container.getParent().requestLayout();
-        // Next time the keyboard accessory appears, do accessibility work.
-        mFirstAppearance = true;
+    private void show() {
+        setVisibility(View.VISIBLE);
+        announceForAccessibility(((ViewGroup) getParent()).getContentDescription());
     }
 
-    @Override
-    public void keyboardVisibilityChanged(boolean isShowing) {
-        if (!isShowing) { // TODO(fhorschig): ... and no bottom sheet.
-            dismiss();
-        }
+    private void hide() {
+        setVisibility(View.GONE);
     }
 }

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

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.autofill.keyboard_accessory;
+
+import android.view.ViewStub;
+
+import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryModel.PropertyKey;
+import org.chromium.chrome.browser.modelutil.PropertyModelChangeProcessor;
+
+import javax.annotation.Nullable;
+
+/**
+ * Observes {@link KeyboardAccessoryModel} changes (like a newly available tab) and triggers the
+ * {@link KeyboardAccessoryViewBinder} which will modify the view accordingly.
+ */
+class KeyboardAccessoryViewBinder
+        implements PropertyModelChangeProcessor.ViewBinder<KeyboardAccessoryModel,
+                KeyboardAccessoryViewBinder.AccessoryViewHolder, PropertyKey> {
+    public static class AccessoryViewHolder {
+        @Nullable
+        private KeyboardAccessoryView mView; // Remains null until |mViewStub| is inflated.
+        private final ViewStub mViewStub;
+
+        AccessoryViewHolder(ViewStub viewStub) {
+            mViewStub = viewStub;
+        }
+
+        @Nullable
+        public KeyboardAccessoryView getView() {
+            return mView;
+        }
+
+        public void initializeView() {
+            mView = (KeyboardAccessoryView) mViewStub.inflate();
+        }
+    }
+
+    @Override
+    public void bind(
+            KeyboardAccessoryModel model, AccessoryViewHolder viewHolder, PropertyKey propertyKey) {
+        KeyboardAccessoryView view = viewHolder.getView();
+        if (view != null) { // If the view was previously inflated, update it and return.
+            updateViewByProperty(model, view, propertyKey);
+            return;
+        }
+        if (propertyKey != PropertyKey.VISIBLE || !model.isVisible()) {
+            return; // Ignore model changes before the view is shown for the first time.
+        }
+
+        // If the view is visible for the first time, update ALL its properties.
+        viewHolder.initializeView();
+        for (PropertyKey key : PropertyKey.ALL_PROPERTIES) {
+            updateViewByProperty(model, viewHolder.getView(), key);
+        }
+    }
+
+    private void updateViewByProperty(
+            KeyboardAccessoryModel model, KeyboardAccessoryView view, PropertyKey propertyKey) {
+        if (propertyKey == PropertyKey.VISIBLE) {
+            view.setVisible(model.isVisible());
+            return;
+        }
+        if (propertyKey == PropertyKey.SUGGESTIONS) {
+            view.updateSuggestions(model.getAutofillSuggestions());
+            return;
+        }
+        assert false : "Every possible property update needs to be handled!";
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java

@@ -498,7 +498,7 @@ public class SuggestionsSection extends InnerNode {
         if (!hasSuggestions()) return true; // If we don't have any, we always accept updates.
 
         if (CardsVariationParameters.ignoreUpdatesForExistingSuggestions()) {
-            Log.d(TAG, "setSuggestions: replacing existing suggestion disabled");
+            Log.d(TAG, "updateSuggestions: replacing existing suggestion disabled");
             NewTabPageUma.recordUIUpdateResult(NewTabPageUma.UI_UPDATE_FAIL_DISABLED);
             return false;
         }
@@ -506,7 +506,7 @@ public class SuggestionsSection extends InnerNode {
         if (numberOfSuggestionsExposed >= getSuggestionsCount() || mHasAppended) {
             // In case that suggestions got removed, we assume they already were seen. This might
             // be over-simplifying things, but given the rare occurences it should be good enough.
-            Log.d(TAG, "setSuggestions: replacing existing suggestion not possible, all seen");
+            Log.d(TAG, "updateSuggestions: replacing existing suggestion not possible, all seen");
             NewTabPageUma.recordUIUpdateResult(NewTabPageUma.UI_UPDATE_FAIL_ALL_SEEN);
             return false;
         }

chrome/android/java_sources.gni

@@ -101,6 +101,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryMediator.java",
   "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java",
   "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryView.java",
+  "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewBinder.java",
   "java/src/org/chromium/chrome/browser/background_task_scheduler/NativeBackgroundTask.java",
   "java/src/org/chromium/chrome/browser/banners/AppBannerManager.java",
   "java/src/org/chromium/chrome/browser/banners/AppBannerUiDelegateAndroid.java",
@@ -1553,12 +1554,13 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/accessibility/FontSizePrefsTest.java",
   "javatests/src/org/chromium/chrome/browser/appmenu/AppMenuTest.java",
   "javatests/src/org/chromium/chrome/browser/appmenu/DataSaverAppMenuTest.java",
-  "javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryTest.java",
+  "javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryIntegrationTest.java",
   "javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupTest.java",
   "javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupWithKeyboardTest.java",
   "javatests/src/org/chromium/chrome/browser/autofill/AutofillTest.java",
   "javatests/src/org/chromium/chrome/browser/autofill/AutofillTestHelper.java",
   "javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java",
+  "javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewTest.java",
   "javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java",
   "javatests/src/org/chromium/chrome/browser/banners/InstallerDelegateTest.java",
   "javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkBridgeTest.java",

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryTest.java → chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryIntegrationTest.java

@@ -4,17 +4,24 @@
 
 package org.chromium.chrome.browser.autofill;
 
+import static android.support.test.espresso.Espresso.onView;
+import static android.support.test.espresso.action.ViewActions.click;
+import static android.support.test.espresso.action.ViewActions.scrollTo;
+import static android.support.test.espresso.matcher.ViewMatchers.withText;
+
 import static org.chromium.ui.base.LocalizationUtils.setRtlForTesting;
 
 import android.os.Build;
-import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 import android.view.View;
 import android.view.ViewGroup;
+import android.widget.HorizontalScrollView;
+import android.widget.LinearLayout;
 
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.ThreadUtils;
@@ -24,12 +31,16 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.MinAndroidSdkLevel;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.base.test.util.UrlUtils;
+import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.util.browser.Features;
+import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
 import org.chromium.content.browser.test.util.Criteria;
 import org.chromium.content.browser.test.util.CriteriaHelper;
 import org.chromium.content.browser.test.util.DOMUtils;
@@ -45,20 +56,20 @@ import java.util.concurrent.atomic.AtomicReference;
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
 @RetryOnFailure
-@CommandLineFlags.
-Add({"enable-features=AutofillKeyboardAccessory", ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
-public class AutofillKeyboardAccessoryTest {
+@EnableFeatures({ChromeFeatureList.AUTOFILL_KEYBOARD_ACCESSORY})
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
+public class AutofillKeyboardAccessoryIntegrationTest {
     @Rule
     public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
             new ChromeActivityTestRule<>(ChromeActivity.class);
+    @Rule
+    public TestRule mFeaturesProcessor = new Features.InstrumentationProcessor();
 
-    private final AtomicReference<WebContents> mWebContentsRef = new AtomicReference<WebContents>();
-    private final AtomicReference<ViewGroup> mContainerRef = new AtomicReference<ViewGroup>();
-    private final AtomicReference<ViewGroup> mKeyboardAccessoryRef =
-            new AtomicReference<ViewGroup>();
+    private final AtomicReference<WebContents> mWebContentsRef = new AtomicReference<>();
+    private final AtomicReference<ViewGroup> mContainerRef = new AtomicReference<>();
 
-    private void loadTestPage(boolean isRtl) throws InterruptedException, ExecutionException,
-            TimeoutException {
+    private void loadTestPage(boolean isRtl)
+            throws InterruptedException, ExecutionException, TimeoutException {
         mActivityTestRule.startMainActivityWithURL(UrlUtils.encodeHtmlDataUri("<html"
                 + (isRtl ? " dir=\"rtl\"" : "") + "><head>"
                 + "<meta name=\"viewport\""
@@ -94,9 +105,6 @@ public class AutofillKeyboardAccessoryTest {
             Tab tab = mActivityTestRule.getActivity().getActivityTab();
             mWebContentsRef.set(tab.getWebContents());
             mContainerRef.set(tab.getContentView());
-            mKeyboardAccessoryRef.set(mActivityTestRule.getActivity()
-                    .getWindowAndroid()
-                    .getKeyboardAccessoryView());
         });
         DOMUtils.waitForNonZeroNodeBounds(mWebContentsRef.get(), "fn");
     }
@@ -110,11 +118,7 @@ public class AutofillKeyboardAccessoryTest {
     public void testAutofocusedFieldDoesNotShowKeyboardAccessory()
             throws ExecutionException, InterruptedException, TimeoutException {
         loadTestPage(false);
-        Assert.assertTrue("Keyboard accessory should be hidden.",
-                ThreadUtils
-                        .runOnUiThreadBlocking(
-                                () -> mKeyboardAccessoryRef.get().getVisibility() == View.GONE)
-                        .booleanValue());
+        Assert.assertTrue("Keyboard accessory should be hidden.", isAccessoryGone());
     }
 
     /**
@@ -127,18 +131,12 @@ public class AutofillKeyboardAccessoryTest {
             throws ExecutionException, InterruptedException, TimeoutException {
         loadTestPage(false);
         DOMUtils.clickNode(mWebContentsRef.get(), "fn");
-        CriteriaHelper.pollUiThread(new Criteria("Keyboard should be showing.") {
-            @Override
-            public boolean isSatisfied() {
-                return UiUtils.isKeyboardShowing(
-                        mActivityTestRule.getActivity(), mContainerRef.get());
-            }
-        });
-        Assert.assertTrue("Keyboard accessory should be showing.",
-                ThreadUtils
-                        .runOnUiThreadBlocking(
-                                () -> mKeyboardAccessoryRef.get().getVisibility() == View.VISIBLE)
-                        .booleanValue());
+
+        CriteriaHelper.pollUiThread(Criteria.equals(true,
+                ()
+                        -> UiUtils.isKeyboardShowing(
+                                mActivityTestRule.getActivity(), mContainerRef.get())));
+        Assert.assertTrue("Keyboard accessory should be showing.", isAccessoryVisible());
     }
 
     /**
@@ -152,38 +150,19 @@ public class AutofillKeyboardAccessoryTest {
             throws ExecutionException, InterruptedException, TimeoutException {
         loadTestPage(false);
         DOMUtils.clickNode(mWebContentsRef.get(), "fn");
-        CriteriaHelper.pollUiThread(new Criteria("Keyboard should be showing.") {
-            @Override
-            public boolean isSatisfied() {
-                return UiUtils.isKeyboardShowing(
-                        mActivityTestRule.getActivity(), mContainerRef.get());
-            }
-        });
+
+        CriteriaHelper.pollUiThread(Criteria.equals(true,
+                ()
+                        -> UiUtils.isKeyboardShowing(
+                                mActivityTestRule.getActivity(), mContainerRef.get())));
+
         ThreadUtils.runOnUiThreadBlocking(() -> getSuggestionsComponent().scrollTo(2000, 0));
-        CriteriaHelper.pollUiThread(
-                new Criteria("First suggestion should be off the screen after manual scroll.") {
-                    @Override
-                    public boolean isSatisfied() {
-                        View suggestion = getSuggestionAt(0);
-                        if (suggestion != null) {
-                            int[] location = new int[2];
-                            suggestion.getLocationOnScreen(location);
-                            return location[0] < 0;
-                        } else {
-                            return false;
-                        }
-                    }
-                });
+        assertSuggestionScrollPosition(
+                false, "First suggestion should be off the screen after manual scroll.");
+
         DOMUtils.clickNode(mWebContentsRef.get(), "ln");
-        CriteriaHelper.pollUiThread(
-                new Criteria("First suggestion should be on the screen after switching fields.") {
-                    @Override
-                    public boolean isSatisfied() {
-                        int[] location = new int[2];
-                        getSuggestionAt(0).getLocationOnScreen(location);
-                        return location[0] > 0;
-                    }
-                });
+        assertSuggestionScrollPosition(
+                true, "First suggestion should be on the screen after switching fields.");
     }
 
     /**
@@ -201,48 +180,20 @@ public class AutofillKeyboardAccessoryTest {
             throws ExecutionException, InterruptedException, TimeoutException {
         loadTestPage(true);
         DOMUtils.clickNode(mWebContentsRef.get(), "fn");
-        CriteriaHelper.pollUiThread(new Criteria("Keyboard should be showing.") {
-            @Override
-            public boolean isSatisfied() {
-                return UiUtils.isKeyboardShowing(
-                        mActivityTestRule.getActivity(), mContainerRef.get());
-            }
-        });
-        ThreadUtils.runOnUiThreadBlocking(() -> getSuggestionsComponent().scrollTo(-1000, 0));
-        CriteriaHelper.pollUiThread(
-                new Criteria("Last suggestion should be on the screen after manual scroll.") {
-                    @Override
-                    public boolean isSatisfied() {
-                        View suggestion = getSuggestionAt(3);
-                        if (suggestion != null) {
-                            int[] location = new int[2];
-                            suggestion.getLocationOnScreen(location);
-                            return location[0] > 0;
-                        } else {
-                            return false;
-                        }
-                    }
-                });
-        // Simulates two clicks. Keyboard delay can often drops the first and doesn't set it again.
-        // TODO(fhorschig): Remove safety net as soon as Accessory is decoupled from suggestions.
-        DOMUtils.clickNode(mWebContentsRef.get(), "ln");
-        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+
+        CriteriaHelper.pollUiThread(Criteria.equals(true,
+                ()
+                        -> UiUtils.isKeyboardShowing(
+                                mActivityTestRule.getActivity(), mContainerRef.get())));
+        assertSuggestionScrollPosition(false, "Last suggestion should be off the screen intially.");
+
+        ThreadUtils.runOnUiThreadBlocking(() -> getSuggestionsComponent().scrollTo(-500, 0));
+        assertSuggestionScrollPosition(
+                true, "Last suggestion should be on the screen after manual scroll.");
+
         DOMUtils.clickNode(mWebContentsRef.get(), "ln");
-        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
-        CriteriaHelper.pollUiThread(
-                new Criteria("Last suggestion should be off the screen after switching fields.") {
-                    @Override
-                    public boolean isSatisfied() {
-                        View suggestion = getSuggestionAt(3);
-                        if (suggestion != null) {
-                            int[] location = new int[2];
-                            suggestion.getLocationOnScreen(location);
-                            return location[0] < 0;
-                        } else {
-                            return false;
-                        }
-                    }
-                });
+        assertSuggestionScrollPosition(
+                false, "Last suggestion should be off the screen after switching fields.");
     }
 
     /**
@@ -256,50 +207,66 @@ public class AutofillKeyboardAccessoryTest {
             throws ExecutionException, InterruptedException, TimeoutException {
         loadTestPage(false);
         DOMUtils.clickNode(mWebContentsRef.get(), "fn");
-        CriteriaHelper.pollUiThread(new Criteria("Keyboard should be showing.") {
-            @Override
-            public boolean isSatisfied() {
-                return UiUtils.isKeyboardShowing(
-                        mActivityTestRule.getActivity(), mContainerRef.get());
-            }
-        });
-        ThreadUtils.runOnUiThreadBlocking(() -> {
-            View suggestion = getSuggestionAt(0);
-            if (suggestion != null) {
-                suggestion.performClick();
+
+        CriteriaHelper.pollUiThread(Criteria.equals(true,
+                ()
+                        -> UiUtils.isKeyboardShowing(
+                                mActivityTestRule.getActivity(), mContainerRef.get())));
+        Assert.assertTrue("Keyboard accessory should be visible.", isAccessoryVisible());
+
+        onView(withText("Marcus")).perform(scrollTo(), click());
+
+        CriteriaHelper.pollUiThread(Criteria.equals(false,
+                ()
+                        -> UiUtils.isKeyboardShowing(
+                                mActivityTestRule.getActivity(), mContainerRef.get())));
+        Assert.assertTrue("Keyboard accessory should be hidden.", isAccessoryGone());
     }
-        });
-        CriteriaHelper.pollUiThread(new Criteria("Keyboard should be hidden.") {
+
+    private void assertSuggestionScrollPosition(boolean shouldBeOnScreen, String failureReason) {
+        CriteriaHelper.pollUiThread(new Criteria(failureReason) {
             @Override
             public boolean isSatisfied() {
-                return !UiUtils.isKeyboardShowing(
-                        mActivityTestRule.getActivity(), mContainerRef.get());
+                View suggestion = getSuggestionAt(0);
+                if (suggestion == null) return false;
+                int[] location = new int[2];
+                suggestion.getLocationOnScreen(location);
+                return shouldBeOnScreen ? location[0] > 0 : location[0] < 0;
             }
         });
-        Assert.assertTrue("Keyboard accessory should be hidden.",
-                ThreadUtils.runOnUiThreadBlocking(
-                        () -> mKeyboardAccessoryRef.get().getVisibility() == View.GONE));
     }
 
-    private AutofillKeyboardSuggestions getSuggestionsComponent() {
-        // The view hierarchy:
-        //   Keyboard accessory.
-        //    \--> A list of accessory components.
-        //        \--> A scroll view.
-        //            \--> A list of suggestions.
-        //                \--> A suggestion that can be clicked.
-        ViewGroup keyboardAccessory = mKeyboardAccessoryRef.get();
+    private HorizontalScrollView getSuggestionsComponent() {
+        final ViewGroup keyboardAccessory = ThreadUtils.runOnUiThreadBlockingNoException(
+                () -> mActivityTestRule.getActivity().findViewById(R.id.keyboard_accessory));
         if (keyboardAccessory == null) return null; // It might still be loading, so don't assert!
-        ViewGroup componentsList = (ViewGroup) mKeyboardAccessoryRef.get().getChildAt(0);
-        if (componentsList == null) return null; // It might still be loading, so don't assert!
-        ViewGroup scrollview = (ViewGroup) componentsList.getChildAt(0);
+
+        final View scrollview = keyboardAccessory.findViewById(R.id.suggestions_view);
         if (scrollview == null) return null; // It might still be loading, so don't assert!
-        return (AutofillKeyboardSuggestions) scrollview.getChildAt(0);
+
+        return (HorizontalScrollView) scrollview;
     }
 
     private View getSuggestionAt(int index) {
-        ViewGroup suggestionsList = getSuggestionsComponent();
-        if (suggestionsList == null) return null; // It might still be loading, so don't assert!
-        return suggestionsList.getChildAt(index);
+        ViewGroup scrollview = getSuggestionsComponent();
+        if (scrollview == null) return null; // It might still be loading, so don't assert!
+
+        return scrollview.getChildAt(index);
+    }
+
+    private boolean isAccessoryVisible() throws ExecutionException {
+        return ThreadUtils.runOnUiThreadBlocking(() -> {
+            LinearLayout keyboard =
+                    mActivityTestRule.getActivity().findViewById(R.id.keyboard_accessory);
+            return keyboard != null && keyboard.getVisibility() == View.VISIBLE;
+        });
+    }
+
+    private boolean isAccessoryGone() throws ExecutionException {
+        return ThreadUtils.runOnUiThreadBlocking(() -> {
+            LinearLayout keyboard =
+                    mActivityTestRule.getActivity().findViewById(R.id.keyboard_accessory);
+            return keyboard == null || keyboard.getVisibility() == View.GONE;
+        });
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewTest.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.autofill.keyboard_accessory;
+
+import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertNull;
+
+import static org.junit.Assert.assertTrue;
+
+import static org.chromium.chrome.R.id;
+
+import android.support.test.filters.MediumTest;
+import android.view.View;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.ChromeTabbedActivity;
+import org.chromium.chrome.browser.modelutil.PropertyModelChangeProcessor;
+import org.chromium.chrome.test.ChromeActivityTestRule;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+
+/**
+ * View tests for the keyboard accessory component.
+ *
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
+public class KeyboardAccessoryViewTest {
+    private KeyboardAccessoryModel mModel;
+    private KeyboardAccessoryViewBinder.AccessoryViewHolder mViewHolder;
+
+    @Rule
+    public ChromeActivityTestRule<ChromeTabbedActivity> mActivityTestRule =
+            new ChromeActivityTestRule<>(ChromeTabbedActivity.class);
+
+    @Before
+    public void setUp() throws InterruptedException {
+        mActivityTestRule.startMainActivityOnBlankPage();
+        mViewHolder = new KeyboardAccessoryViewBinder.AccessoryViewHolder(
+                mActivityTestRule.getActivity().findViewById(id.keyboard_accessory_stub));
+        mModel = new KeyboardAccessoryModel();
+        mModel.addObserver(new PropertyModelChangeProcessor<>(
+                mModel, mViewHolder, new KeyboardAccessoryViewBinder()));
+    }
+
+    @Test
+    @MediumTest
+    public void testAccessoryVisibilityChangedByModel() {
+        // Initially, there shouldn't be a view yet.
+        assertNull(mViewHolder.getView());
+
+        // After setting the visibility to true, the view should exist and be visible.
+        ThreadUtils.runOnUiThreadBlocking(() -> mModel.setVisible(true));
+        assertNotNull(mViewHolder.getView());
+        assertTrue(mViewHolder.getView().getVisibility() == View.VISIBLE);
+
+        // After hiding the view, the view should still exist but be invisible.
+        ThreadUtils.runOnUiThreadBlocking(() -> mModel.setVisible(false));
+        assertNotNull(mViewHolder.getView());
+        assertTrue(mViewHolder.getView().getVisibility() != View.VISIBLE);
+    }
+}
\ No newline at end of file

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/OWNERS

+file://chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/OWNERS
+
+# COMPONENT: UI>Browser>Autofill

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

@@ -11,8 +11,10 @@ import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
 
 import android.support.test.filters.SmallTest;
+import android.view.ViewStub;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -24,7 +26,9 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.modelutil.ListObservable;
+import org.chromium.chrome.browser.modelutil.PropertyModelChangeProcessor;
 import org.chromium.chrome.browser.modelutil.PropertyObservable.PropertyObserver;
+import org.chromium.ui.base.WindowAndroid;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -36,11 +40,20 @@ import java.util.List;
 @Config(manifest = Config.NONE)
 public class KeyboardAccessoryControllerTest {
     @Mock
-    PropertyObserver<KeyboardAccessoryModel.PropertyKey> mMockPropertyObserver;
+    private PropertyObserver<KeyboardAccessoryModel.PropertyKey> mMockPropertyObserver;
     @Mock
-    ListObservable.ListObserver mMockTabListObserver;
+    private ListObservable.ListObserver mMockTabListObserver;
     @Mock
-    ListObservable.ListObserver mMockActionListObserver;
+    private ListObservable.ListObserver mMockActionListObserver;
+    @Mock
+    private WindowAndroid mMockWindow;
+    @Mock
+    private ViewStub mMockViewStub;
+    @Mock
+    private KeyboardAccessoryView mMockView;
+    @Mock
+    private PropertyModelChangeProcessor<KeyboardAccessoryModel, KeyboardAccessoryView,
+            KeyboardAccessoryModel.PropertyKey> mMockModelChangeProcessor;
 
     private class TestActionListProvider implements KeyboardAccessoryData.ActionListProvider {
         private final List<KeyboardAccessoryData.ActionListObserver> mObservers = new ArrayList<>();
@@ -60,101 +73,117 @@ public class KeyboardAccessoryControllerTest {
     private static class FakeTab implements KeyboardAccessoryData.Tab {}
     private static class FakeAction implements KeyboardAccessoryData.Action {}
 
+    private KeyboardAccessoryCoordinator mCoordinator;
+    private KeyboardAccessoryModel mModel;
+    private KeyboardAccessoryMediator mMediator;
+
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
+        when(mMockViewStub.inflate()).thenReturn(mMockView);
+        mCoordinator = new KeyboardAccessoryCoordinator(mMockWindow, mMockViewStub);
+        mMediator = mCoordinator.getMediatorForTesting();
+        mModel = mMediator.getModelForTesting();
     }
 
     @Test
     @SmallTest
     @Feature({"keyboard-accessory"})
     public void testCreatesValidSubComponents() {
-        final KeyboardAccessoryCoordinator coordinator = new KeyboardAccessoryCoordinator();
-        final KeyboardAccessoryMediator mediator = coordinator.getMediatorForTesting();
-        assertThat(mediator, is(notNullValue()));
-        final KeyboardAccessoryModel model = mediator.getModelForTesting();
-        assertThat(model, is(notNullValue()));
+        assertThat(mCoordinator, is(notNullValue()));
+        assertThat(mMediator, is(notNullValue()));
+        assertThat(mModel, is(notNullValue()));
+        verify(mMockWindow).addKeyboardVisibilityListener(mMediator);
     }
 
     @Test
     @SmallTest
     @Feature({"keyboard-accessory"})
     public void testModelNotifiesVisibilityChangeOnShowAndHide() {
-        final KeyboardAccessoryCoordinator coordinator = new KeyboardAccessoryCoordinator();
-        final KeyboardAccessoryModel model =
-                coordinator.getMediatorForTesting().getModelForTesting();
-        model.addObserver(mMockPropertyObserver);
+        mModel.addObserver(mMockPropertyObserver);
 
-        // Calling show on the coordinator should make model propagate that it's visible.
-        coordinator.show();
+        // Calling show on the mediator should make model propagate that it's visible.
+        mMediator.show();
         verify(mMockPropertyObserver)
-                .onPropertyChanged(model, KeyboardAccessoryModel.PropertyKey.VISIBLE);
-        assertThat(model.isVisible(), is(true));
+                .onPropertyChanged(mModel, KeyboardAccessoryModel.PropertyKey.VISIBLE);
+        assertThat(mModel.isVisible(), is(true));
 
-        // Calling hide on the coordinator should make model propagate that it's invisible.
-        coordinator.hide();
+        // Calling hide on the mediator should make model propagate that it's invisible.
+        mMediator.hide();
         verify(mMockPropertyObserver, times(2))
-                .onPropertyChanged(model, KeyboardAccessoryModel.PropertyKey.VISIBLE);
-        assertThat(model.isVisible(), is(false));
+                .onPropertyChanged(mModel, KeyboardAccessoryModel.PropertyKey.VISIBLE);
+        assertThat(mModel.isVisible(), is(false));
     }
 
     @Test
     @SmallTest
     @Feature({"keyboard-accessory"})
     public void testChangingTabsNotifiesTabObserver() {
-        final KeyboardAccessoryCoordinator coordinator = new KeyboardAccessoryCoordinator();
-        final KeyboardAccessoryModel model =
-                coordinator.getMediatorForTesting().getModelForTesting();
         final FakeTab testTab = new FakeTab();
 
-        model.addTabListObserver(mMockTabListObserver);
+        mModel.addTabListObserver(mMockTabListObserver);
 
         // Calling addTab on the coordinator should make model propagate that it has a new tab.
-        coordinator.addTab(testTab);
-        verify(mMockTabListObserver).onItemRangeInserted(model.getTabList(), 0, 1);
-        assertThat(model.getTabList().getItemCount(), is(1));
-        assertThat(model.getTabList().get(0), is(equalTo(testTab)));
+        mCoordinator.addTab(testTab);
+        verify(mMockTabListObserver).onItemRangeInserted(mModel.getTabList(), 0, 1);
+        assertThat(mModel.getTabList().getItemCount(), is(1));
+        assertThat(mModel.getTabList().get(0), is(equalTo(testTab)));
 
         // Calling hide on the coordinator should make model propagate that it's invisible.
-        coordinator.removeTab(testTab);
-        verify(mMockTabListObserver).onItemRangeRemoved(model.getTabList(), 0, 1);
-        assertThat(model.getTabList().getItemCount(), is(0));
+        mCoordinator.removeTab(testTab);
+        verify(mMockTabListObserver).onItemRangeRemoved(mModel.getTabList(), 0, 1);
+        assertThat(mModel.getTabList().getItemCount(), is(0));
     }
 
     @Test
     @SmallTest
     @Feature({"keyboard-accessory"})
     public void testModelNotifiesAboutActionsChangedByProvider() {
-        final KeyboardAccessoryCoordinator coordinator = new KeyboardAccessoryCoordinator();
-        final KeyboardAccessoryModel model =
-                coordinator.getMediatorForTesting().getModelForTesting();
-
         final TestActionListProvider testProvider = new TestActionListProvider();
         final FakeAction testAction = new FakeAction();
 
-        model.addActionListObserver(mMockActionListObserver);
-        coordinator.registerActionListProvider(testProvider);
+        mModel.addActionListObserver(mMockActionListObserver);
+        mCoordinator.registerActionListProvider(testProvider);
 
-        // If the mediator receives an initial set of actions, the model should report an insertion.
+        // If the coordinator receives an initial actions, the model should report an insertion.
         testProvider.sendActionsToReceivers(new KeyboardAccessoryData.Action[] {testAction});
-        verify(mMockActionListObserver).onItemRangeInserted(model.getActionList(), 0, 1);
-        assertThat(model.getActionList().getItemCount(), is(1));
-        assertThat(model.getActionList().get(0), is(equalTo(testAction)));
+        verify(mMockActionListObserver).onItemRangeInserted(mModel.getActionList(), 0, 1);
+        assertThat(mModel.getActionList().getItemCount(), is(1));
+        assertThat(mModel.getActionList().get(0), is(equalTo(testAction)));
 
-        // If the mediator receives a new set of actions, the model should report a change.
+        // If the coordinator receives a new set of actions, the model should report a change.
         testProvider.sendActionsToReceivers(new KeyboardAccessoryData.Action[] {testAction});
         verify(mMockActionListObserver)
-                .onItemRangeChanged(model.getActionList(), 0, 1, model.getActionList());
-        assertThat(model.getActionList().getItemCount(), is(1));
-        assertThat(model.getActionList().get(0), is(equalTo(testAction)));
+                .onItemRangeChanged(mModel.getActionList(), 0, 1, mModel.getActionList());
+        assertThat(mModel.getActionList().getItemCount(), is(1));
+        assertThat(mModel.getActionList().get(0), is(equalTo(testAction)));
 
-        // If the mediator receives an empty set of actions, the model should report a deletion.
+        // If the coordinator receives an empty set of actions, the model should report a deletion.
         testProvider.sendActionsToReceivers(new KeyboardAccessoryData.Action[] {});
-        verify(mMockActionListObserver).onItemRangeRemoved(model.getActionList(), 0, 1);
-        assertThat(model.getActionList().getItemCount(), is(0));
+        verify(mMockActionListObserver).onItemRangeRemoved(mModel.getActionList(), 0, 1);
+        assertThat(mModel.getActionList().getItemCount(), is(0));
 
         // There should be no notification if no actions are reported repeatedly.
         testProvider.sendActionsToReceivers(new KeyboardAccessoryData.Action[] {});
         verifyNoMoreInteractions(mMockActionListObserver);
     }
+
+    @Test
+    @SmallTest
+    @Feature({"keyboard-accessory"})
+    public void testModelDoesntNotifyUnchangedData() {
+        mModel.addObserver(mMockPropertyObserver);
+
+        // Calling show on the coordinator should make model propagate that it's visible.
+        mCoordinator.show();
+        verify(mMockPropertyObserver)
+                .onPropertyChanged(mModel, KeyboardAccessoryModel.PropertyKey.VISIBLE);
+        assertThat(mModel.isVisible(), is(true));
+
+        // Marking it as visible again should not result in a second notification.
+        mCoordinator.show();
+        verify(mMockPropertyObserver) // Unchanged number of invocations.
+                .onPropertyChanged(mModel, KeyboardAccessoryModel.PropertyKey.VISIBLE);
+        assertThat(mModel.isVisible(), is(true));
+    }
 }
\ No newline at end of file

ui/android/java/src/org/chromium/ui/base/WindowAndroid.java

@@ -22,7 +22,6 @@ import android.os.Process;
 import android.util.SparseArray;
 import android.util.TypedValue;
 import android.view.View;
-import android.view.ViewGroup;
 import android.view.Window;
 import android.view.accessibility.AccessibilityManager;
 
@@ -99,7 +98,6 @@ public class WindowAndroid {
     private HashSet<Animator> mAnimationsOverContent = new HashSet<>();
     private View mAnimationPlaceholderView;
 
-    private ViewGroup mKeyboardAccessoryView;
 
     protected boolean mIsKeyboardShowing;
 
@@ -632,22 +630,6 @@ public class WindowAndroid {
         }
     }
 
-    /**
-     * Sets the keyboard accessory view.
-     * @param view This view sits at the bottom of the content area and pushes the content up rather
-     *             than overlaying it. Currently used as a container for Autofill suggestions.
-     */
-    public void setKeyboardAccessoryView(ViewGroup view) {
-        mKeyboardAccessoryView = view;
-    }
-
-    /**
-     * @see #setKeyboardAccessoryView(ViewGroup)
-     */
-    public ViewGroup getKeyboardAccessoryView() {
-        return mKeyboardAccessoryView;
-    }
-
     protected void registerKeyboardVisibilityCallbacks() {
     }