[Autofill Assistant] Fixed animation issues in action carousel.

Bug: b/144075373
Change-Id: I559fb7bd9ad4273e3360604c24c230ce5edc0ef9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2043799
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#742170}

chrome/android/features/autofill_assistant/BUILD.gn

@@ -91,13 +91,13 @@ android_library("java") {
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantServiceInjector.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/BottomSheetUtils.java",
-    "java/src/org/chromium/chrome/browser/autofill_assistant/EditDistance.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/FeedbackContext.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/SizeListenableLinearLayout.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantActionsCarouselCoordinator.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantActionsDecoration.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantCarouselModel.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantChip.java",
+    "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantChipAdapter.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantChipViewHolder.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/carousel/ButtonView.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/details/AssistantDetails.java",
@@ -242,7 +242,6 @@ android_library("test_java") {
     "javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTextUtilsTest.java",
     "javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTest.java",
     "javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTestUtil.java",
-    "javatests/src/org/chromium/chrome/browser/autofill_assistant/EditDistanceTest.java",
     "javatests/src/org/chromium/chrome/browser/autofill_assistant/TestingAutofillAssistantModuleEntryProvider.java",
   ]
 

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantBottomBarCoordinator.java

@@ -19,7 +19,7 @@ import org.chromium.base.ObserverList;
 import org.chromium.chrome.autofill_assistant.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantActionsCarouselCoordinator;
-import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip;
+import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantCarouselModel;
 import org.chromium.chrome.browser.autofill_assistant.details.AssistantDetailsCoordinator;
 import org.chromium.chrome.browser.autofill_assistant.form.AssistantFormCoordinator;
 import org.chromium.chrome.browser.autofill_assistant.form.AssistantFormModel;
@@ -35,7 +35,6 @@ import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetContent;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
 import org.chromium.chrome.browser.widget.bottomsheet.EmptyBottomSheetObserver;
 import org.chromium.content_public.browser.WebContents;
-import org.chromium.ui.modelutil.ListModel;
 
 /**
  * Coordinator responsible for the Autofill Assistant bottom bar.
@@ -149,7 +148,7 @@ class AssistantBottomBarCoordinator
         setChildMarginTop(mFormCoordinator.getView(), childSpacing);
 
         // Hide the carousels when they are empty.
-        hideWhenEmpty(mActionsCoordinator.getView(), model.getActionsModel().getChipsModel());
+        hideWhenEmpty(mActionsCoordinator.getView(), model.getActionsModel());
 
         // Set the horizontal margins of children. We don't set them on the payment request, the
         // carousels or the form to allow them to take the full width of the sheet.
@@ -325,18 +324,16 @@ class AssistantBottomBarCoordinator
     /**
      * Observe {@code model} such that the associated view is made invisible when it is empty.
      */
-    private void hideWhenEmpty(View carouselView, ListModel<AssistantChip> chipsModel) {
-        setCarouselVisibility(carouselView, chipsModel);
-        chipsModel.addObserver(new AbstractListObserver<Void>() {
-            @Override
-            public void onDataSetChanged() {
-                setCarouselVisibility(carouselView, chipsModel);
-            }
-        });
+    private void hideWhenEmpty(View carouselView, AssistantCarouselModel carouselModel) {
+        setCarouselVisibility(carouselView, carouselModel);
+        carouselModel.addObserver(
+                (source, propertyKey) -> setCarouselVisibility(carouselView, carouselModel));
     }
 
-    private void setCarouselVisibility(View carouselView, ListModel<AssistantChip> chipsModel) {
-        carouselView.setVisibility(chipsModel.size() > 0 ? View.VISIBLE : View.GONE);
+    private void setCarouselVisibility(View carouselView, AssistantCarouselModel carouselModel) {
+        carouselView.setVisibility(carouselModel.get(AssistantCarouselModel.CHIPS).size() > 0
+                        ? View.VISIBLE
+                        : View.GONE);
     }
 
     private void setHorizontalMargins(View view) {

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java

@@ -14,6 +14,7 @@ import org.chromium.base.task.PostTask;
 import org.chromium.chrome.autofill_assistant.R;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantCarouselModel;
 import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip;
 import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip.Type;
 import org.chromium.chrome.browser.autofill_assistant.header.AssistantHeaderModel;
@@ -296,15 +297,29 @@ class AutofillAssistantUiController {
 
     @CalledByNative
     private void setActions(List<AssistantChip> chips) {
-        setActionChips(chips);
+        // TODO(b/144075373): Move this to AssistantCarouselModel and AssistantHeaderModel. Move
+        // header chip logic to native.
+        AssistantCarouselModel model = getModel().getActionsModel();
+        model.setChips(chips);
         setHeaderChip(chips);
     }
 
     @CalledByNative
     private void setAllChipsVisibleExcept(String identifier, boolean visible) {
-        mCoordinator.getBottomBarCoordinator()
-                .getActionsCarouselCoordinator()
-                .setAllChipsVisibleExcept(identifier, visible);
+        AssistantCarouselModel model = getModel().getActionsModel();
+        List<AssistantChip> chips = model.get(AssistantCarouselModel.CHIPS);
+        // Copy the list and modify the copy. Modifying the actual list in-place will not fire the
+        // relevant change notifications. TODO(b/144075373): Refactor to avoid this deep copy,
+        // preferably by moving this to native.
+        List<AssistantChip> newChips = new ArrayList<>();
+        for (int i = 0; i < chips.size(); ++i) {
+            AssistantChip newChip = new AssistantChip(chips.get(i));
+            newChips.add(newChip);
+            if (!chips.get(i).getIdentifier().equals(identifier)) {
+                newChip.setVisible(visible);
+            }
+        }
+        model.setChips(newChips);
     }
 
     private void setHeaderChip(List<AssistantChip> chips) {
@@ -320,12 +335,6 @@ class AutofillAssistantUiController {
         getModel().getHeaderModel().set(AssistantHeaderModel.CHIP, headerChip);
     }
 
-    private void setActionChips(List<AssistantChip> newChips) {
-        // TODO(b/144075373): Move this to AssistantActionsCarouselCoordinator.
-        mCoordinator.getBottomBarCoordinator().getActionsCarouselCoordinator().updateChips(
-                newChips);
-    }
-
     @CalledByNative
     private void setViewportMode(@AssistantViewportMode int mode) {
         mCoordinator.getBottomBarCoordinator().setViewportMode(mode);

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantActionsCarouselCoordinator.java

@@ -13,12 +13,6 @@ import android.view.View;
 import android.view.ViewGroup;
 
 import org.chromium.chrome.autofill_assistant.R;
-import org.chromium.chrome.browser.autofill_assistant.EditDistance;
-import org.chromium.ui.modelutil.ListModel;
-import org.chromium.ui.modelutil.RecyclerViewAdapter;
-import org.chromium.ui.modelutil.SimpleRecyclerViewMcp;
-
-import java.util.List;
 
 /**
  * A coordinator responsible for suggesting chips to the user. If there is one chip to display, it
@@ -42,14 +36,13 @@ import java.util.List;
  * |               |
  */
 public class AssistantActionsCarouselCoordinator {
-    private final AssistantCarouselModel mModel;
     private final RecyclerView mView;
 
     public AssistantActionsCarouselCoordinator(Context context, AssistantCarouselModel model) {
-        mModel = model;
-
         mView = new RecyclerView(context);
         mView.setTag(RECYCLER_VIEW_TAG);
+        AssistantChipAdapter chipAdapter = new AssistantChipAdapter();
+        mView.setAdapter(chipAdapter);
 
         CustomLayoutManager layoutManager = new CustomLayoutManager();
         // Workaround for b/128679161.
@@ -70,77 +63,17 @@ public class AssistantActionsCarouselCoordinator {
                                 * context.getResources().getDimensionPixelSize(
                                         R.dimen.autofill_assistant_button_bg_vertical_inset)));
 
-        mView.setAdapter(new RecyclerViewAdapter<>(
-                new SimpleRecyclerViewMcp<>(model.getChipsModel(),
-                        AssistantChipViewHolder::getViewType, AssistantChipViewHolder::bind),
-                AssistantChipViewHolder::create));
+        model.addObserver((source, propertyKey) -> {
+            if (propertyKey == AssistantCarouselModel.CHIPS) {
+                chipAdapter.setChips(model.get(AssistantCarouselModel.CHIPS));
+            }
+        });
     }
 
     public RecyclerView getView() {
         return mView;
     }
 
-    public void updateChips(List<AssistantChip> newChips) {
-        ListModel<AssistantChip> oldChips = mModel.getChipsModel();
-        if (!areChipsEqual(oldChips, newChips)) {
-            // We apply the minimum set of operations on the current chips to transform it in the
-            // target list of chips. When testing for chip equivalence, we only compare their type
-            // and text but all substitutions will still be applied so we are sure we display the
-            // given {@code chips} with their associated callbacks.
-            EditDistance.transform(oldChips, newChips, AssistantChip::equals);
-        } else {
-            for (int i = 0; i < oldChips.size(); ++i) {
-                AssistantChip oldChip = oldChips.get(i);
-                AssistantChip newChip = newChips.get(i);
-
-                // We assume that the enabled state is the only thing that may change.
-                if (oldChip.isDisabled() == newChip.isDisabled()) {
-                    continue;
-                }
-
-                View view = mView.getLayoutManager().findViewByPosition(i);
-                if (view == null) {
-                    oldChips.update(i, newChip);
-                } else {
-                    oldChip.setDisabled(newChip.isDisabled());
-                    view.setEnabled(!newChip.isDisabled());
-                }
-            }
-        }
-    }
-
-    /** Changes the visibility of all chips not matching the identifier **/
-    public void setAllChipsVisibleExcept(String identifier, boolean visible) {
-        ListModel<AssistantChip> chips = mModel.getChipsModel();
-        for (int i = 0; i < chips.size(); ++i) {
-            View view = mView.getLayoutManager().findViewByPosition(i);
-            if (view != null && !chips.get(i).getIdentifier().equals(identifier)) {
-                view.setVisibility(visible ? View.VISIBLE : View.GONE);
-            }
-        }
-    }
-
-    /**
-     * Returns |true| if all chips in the list are considered equal, meaning the same except their
-     * |isDisabled()| state.
-     */
-    private boolean areChipsEqual(ListModel<AssistantChip> oldChips, List<AssistantChip> newChips) {
-        if (oldChips.size() != newChips.size()) {
-            return false;
-        }
-
-        for (int i = 0; i < oldChips.size(); ++i) {
-            AssistantChip oldChip = oldChips.get(i);
-            AssistantChip newChip = newChips.get(i);
-
-            if (!oldChip.equals(newChip)) {
-                return false;
-            }
-        }
-
-        return true;
-    }
-
     // TODO(crbug.com/806868): Handle RTL layouts.
     // TODO(crbug.com/806868): Recycle invisible children instead of laying all of them out.
     static class CustomLayoutManager extends RecyclerView.LayoutManager {

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantActionsDecoration.java

@@ -101,12 +101,28 @@ class AssistantActionsDecoration extends RecyclerView.ItemDecoration {
         }
 
         View lastChild = parent.getChildAt(parent.getChildCount() - 1);
+        View beforeLastChild = parent.getChildAt(parent.getChildCount() - 2);
         mLastChildRect.left = lastChild.getLeft() + lastChild.getTranslationX();
         mLastChildRect.top = lastChild.getTop() + lastChild.getTranslationY() + mVerticalInset;
         mLastChildRect.right = lastChild.getRight() + lastChild.getTranslationX();
         mLastChildRect.bottom =
                 lastChild.getBottom() + lastChild.getTranslationY() - mVerticalInset;
 
+        // Early return to avoid drawing last button decoration if no chip overlaps with the cancel
+        // chip. Note that there are spurious updates where the last chip's position is
+        // intermittently in the wrong position. The workaround is to check if the combined width of
+        // all chips fits without scrolling or not.
+        // TODO(b/144075373): Fix this properly by avoiding the spurious updates.
+        OrientationHelper orientationHelper = mLayoutManager.mOrientationHelper;
+        int sumDecoratedWidth = 0;
+        for (int i = 0; i < parent.getChildCount(); ++i) {
+            sumDecoratedWidth += orientationHelper.getDecoratedEnd(parent.getChildAt(i))
+                    - orientationHelper.getDecoratedStart(parent.getChildAt(i));
+        }
+        if (sumDecoratedWidth < mLayoutManager.getWidth()) {
+            return;
+        }
+
         canvas.save();
 
         // Don't draw on the last child.
@@ -116,7 +132,6 @@ class AssistantActionsDecoration extends RecyclerView.ItemDecoration {
         canvas.clipPath(mLastChildPath, Region.Op.DIFFERENCE);
 
         // Overlay children close to the last child with a semi-transparent paint.
-        OrientationHelper orientationHelper = mLayoutManager.mOrientationHelper;
         float lastChildRight = orientationHelper.getDecoratedEnd(lastChild);
         for (int i = parent.getChildCount() - 2; i >= 0; i--) {
             View child = parent.getChildAt(i);
@@ -136,8 +151,6 @@ class AssistantActionsDecoration extends RecyclerView.ItemDecoration {
             canvas.drawRect(mChildRect, mOverlayPaint);
         }
 
-        View beforeLastChild = parent.getChildAt(parent.getChildCount() - 2);
-
         // Draw a fixed size white-to-transparent linear gradient from left to right.
         mGradientDrawable.setBounds(
                 0, mVerticalSpacing, mGradientWidth, parent.getHeight() - mVerticalSpacing);

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantCarouselModel.java

@@ -4,15 +4,24 @@
 
 package org.chromium.chrome.browser.autofill_assistant.carousel;
 
-import org.chromium.ui.modelutil.ListModel;
+import org.chromium.ui.modelutil.PropertyModel;
+
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * State for the carousel of the Autofill Assistant.
  */
-public class AssistantCarouselModel {
-    private final ListModel<AssistantChip> mChipsModel = new ListModel<>();
+public class AssistantCarouselModel extends PropertyModel {
+    public static final WritableObjectPropertyKey<List<AssistantChip>> CHIPS =
+            new WritableObjectPropertyKey<>();
+
+    public AssistantCarouselModel() {
+        super(CHIPS);
+        set(CHIPS, new ArrayList<>());
+    }
 
-    public ListModel<AssistantChip> getChipsModel() {
-        return mChipsModel;
+    public void setChips(List<AssistantChip> chips) {
+        set(CHIPS, chips);
     }
 }

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantChip.java

@@ -58,6 +58,9 @@ public class AssistantChip {
     /** Whether this chip is enabled or not. */
     private boolean mDisabled;
 
+    /** Whether this chip is visible or not. */
+    private boolean mVisible;
+
     /**
      * Whether this chip is sticky. A sticky chip will be a candidate to be displayed in the header
      * if the peek mode of the sheet is HANDLE_HEADER.
@@ -75,11 +78,23 @@ public class AssistantChip {
         mIcon = icon;
         mText = text;
         mDisabled = disabled;
+        mVisible = true;
         mSticky = sticky;
         mIdentifier = identifier;
         mSelectedListener = selectedListener;
     }
 
+    public AssistantChip(AssistantChip other) {
+        mType = other.mType;
+        mIcon = other.mIcon;
+        mText = other.mText;
+        mDisabled = other.mDisabled;
+        mVisible = other.mVisible;
+        mSticky = other.mSticky;
+        mIdentifier = other.mIdentifier;
+        mSelectedListener = other.mSelectedListener;
+    }
+
     public int getType() {
         return mType;
     }
@@ -96,15 +111,18 @@ public class AssistantChip {
         return mDisabled;
     }
 
-    /**
-     * Set the disabled state of the {@link AssistantChip} object. Changing this flag will not
-     * affect the view this chip is bound to. Use this to keep the model and view in sync, if
-     * the view's state changes.
-     */
+    public boolean isVisible() {
+        return mVisible;
+    }
+
     public void setDisabled(boolean disabled) {
         mDisabled = disabled;
     }
 
+    public void setVisible(boolean visible) {
+        mVisible = visible;
+    }
+
     public boolean isSticky() {
         return mSticky;
     }
@@ -119,15 +137,14 @@ public class AssistantChip {
 
     @Override
     public boolean equals(Object other) {
-        if (other == this) {
-            return true;
-        }
         if (!(other instanceof AssistantChip)) {
             return false;
         }
 
         AssistantChip that = (AssistantChip) other;
         return this.getType() == that.getType() && this.getText().equals(that.getText())
-                && this.getIcon() == that.getIcon() && this.isSticky() == that.isSticky();
+                && this.getIcon() == that.getIcon() && this.isSticky() == that.isSticky()
+                && this.getIdentifier().equals(that.getIdentifier())
+                && this.isDisabled() == that.isDisabled() && this.isVisible() == that.isVisible();
     }
 }

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantChipAdapter.java

+// Copyright 2020 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_assistant.carousel;
+
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+import android.support.v7.util.DiffUtil;
+import android.support.v7.util.ListUpdateCallback;
+import android.support.v7.widget.RecyclerView;
+import android.view.View;
+import android.view.ViewGroup;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Custom RecyclerView adapter for instances of {@code AssistantChip}.
+ */
+public class AssistantChipAdapter extends RecyclerView.Adapter<AssistantChipViewHolder> {
+    private final List<AssistantChip> mChips = new ArrayList<>();
+
+    void setChips(List<AssistantChip> chips) {
+        DiffUtil.DiffResult diffResult = DiffUtil.calculateDiff(new DiffUtil.Callback() {
+            @Override
+            public int getOldListSize() {
+                return mChips.size();
+            }
+
+            @Override
+            public int getNewListSize() {
+                return chips.size();
+            }
+
+            @Override
+            public boolean areItemsTheSame(int oldItemPosition, int newItemPosition) {
+                AssistantChip oldChip = mChips.get(oldItemPosition);
+                AssistantChip newChip = chips.get(newItemPosition);
+                return newChip.getType() == oldChip.getType()
+                        && newChip.getText().equals(oldChip.getText())
+                        && newChip.getIcon() == oldChip.getIcon()
+                        && newChip.isSticky() == oldChip.isSticky();
+            }
+
+            @Override
+            public boolean areContentsTheSame(int oldItemPosition, int newItemPosition) {
+                return chips.get(newItemPosition).equals(mChips.get(oldItemPosition));
+            }
+
+            @Nullable
+            @Override
+            public Object getChangePayload(int oldItemPosition, int newItemPosition) {
+                return chips.get(newItemPosition);
+            }
+        });
+
+        // TODO(b/144075373): The following should work, but does not fire change notifications
+        //  properly, leading to missing change animations:
+        //   mChips.clear();
+        //   mChips.addAll(chips);
+        //   diffResult.dispatchUpdatesTo(this);
+        // The workaround is to update manually:
+        diffResult.dispatchUpdatesTo(new ListUpdateCallback() {
+            @Override
+            public void onInserted(int position, int count) {
+                for (int i = 0; i < count; ++i) {
+                    mChips.add(position + i, chips.get(i));
+                    notifyItemInserted(position);
+                }
+            }
+
+            @Override
+            public void onRemoved(int position, int count) {
+                for (int i = 0; i < count; ++i) {
+                    mChips.remove(position);
+                    notifyItemRemoved(position);
+                }
+            }
+
+            @Override
+            public void onMoved(int fromPosition, int toPosition) {
+                Collections.swap(mChips, fromPosition, toPosition);
+                notifyItemMoved(fromPosition, toPosition);
+            }
+
+            @Override
+            public void onChanged(int position, int count, @Nullable Object payload) {
+                assert payload instanceof AssistantChip;
+                AssistantChip newChip = (AssistantChip) payload;
+                mChips.set(position, newChip);
+                notifyItemChanged(position);
+            }
+        });
+    }
+
+    @NonNull
+    @Override
+    public AssistantChipViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
+        return AssistantChipViewHolder.create(parent, viewType);
+    }
+
+    @Override
+    public void onBindViewHolder(@NonNull AssistantChipViewHolder viewHolder, int position) {
+        viewHolder.bind(mChips.get(position));
+    }
+
+    @Override
+    public void onBindViewHolder(@NonNull AssistantChipViewHolder viewHolder, int position,
+            @Nullable List<Object> payloads) {
+        // Perform in-place update instead of full bind when possible.
+        if (payloads != null && payloads.size() > 0) {
+            AssistantChip changedChip = (AssistantChip) payloads.get(payloads.size() - 1);
+            viewHolder.getView().setEnabled(!changedChip.isDisabled());
+            viewHolder.getView().setVisibility(changedChip.isVisible() ? View.VISIBLE : View.GONE);
+            return;
+        }
+        onBindViewHolder(viewHolder, position);
+    }
+
+    @Override
+    public int getItemCount() {
+        return mChips.size();
+    }
+
+    @Override
+    public int getItemViewType(int position) {
+        return mChips.get(position).getType();
+    }
+}

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/carousel/AssistantChipViewHolder.java

@@ -30,7 +30,7 @@ public class AssistantChipViewHolder extends ViewHolder {
     public static AssistantChipViewHolder create(ViewGroup parent, int viewType) {
         LayoutInflater layoutInflater = LayoutInflater.from(parent.getContext());
         ButtonView view = null;
-        switch (viewType % AssistantChip.Type.NUM_ENTRIES) {
+        switch (viewType) {
             case AssistantChip.Type.CHIP_ASSISTIVE:
                 view = (ButtonView) layoutInflater.inflate(
                         R.layout.autofill_assistant_button_assistive, /* root= */ null);
@@ -54,12 +54,6 @@ public class AssistantChipViewHolder extends ViewHolder {
     }
 
     public static int getViewType(AssistantChip chip) {
-        // We add AssistantChip.Type.CHIP_TYPE_NUMBER to differentiate between enabled and disabled
-        // chips of the same type. Ideally, we should return a (type, disabled) tuple but
-        // RecyclerView does not allow that.
-        if (chip.isDisabled()) {
-            return chip.getType() + AssistantChip.Type.NUM_ENTRIES;
-        }
         return chip.getType();
     }
 
@@ -73,6 +67,7 @@ public class AssistantChipViewHolder extends ViewHolder {
 
     public void bind(AssistantChip chip) {
         mView.setEnabled(!chip.isDisabled());
+        mView.setVisibility(chip.isVisible() ? View.VISIBLE : View.GONE);
 
         String text = chip.getText();
         if (text.isEmpty()) {

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantActionsCarouselUiTest.java

@@ -34,6 +34,10 @@ import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 /**
  * Tests for the autofill assistant actions carousel.
  */
@@ -75,7 +79,7 @@ public class AutofillAssistantActionsCarouselUiTest {
         AssistantCarouselModel model = new AssistantCarouselModel();
         AssistantActionsCarouselCoordinator coordinator = createCoordinator(model);
 
-        assertThat(model.getChipsModel().size(), is(0));
+        assertThat(model.get(AssistantCarouselModel.CHIPS).size(), is(0));
         assertThat(coordinator.getView().getAdapter().getItemCount(), is(0));
     }
 
@@ -88,9 +92,10 @@ public class AutofillAssistantActionsCarouselUiTest {
 
         TestThreadUtils.runOnUiThreadBlocking(
                 ()
-                        -> model.getChipsModel().add(
-                                new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE,
-                                        AssistantChip.Icon.NONE, "Test", false, true, "", null)));
+                        -> model.set(AssistantCarouselModel.CHIPS,
+                                Collections.singletonList(new AssistantChip(
+                                        AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
+                                        "Test", false, true, "", null))));
 
         // Chip was created and is displayed on the screen.
         onView(is(coordinator.getView()))
@@ -108,14 +113,15 @@ public class AutofillAssistantActionsCarouselUiTest {
 
         // Note: this should be a small number that fits on screen without scrolling.
         int numChips = 3;
-        TestThreadUtils.runOnUiThreadBlocking(() -> {
+        List<AssistantChip> chips = new ArrayList<>();
         for (int i = 0; i < numChips; i++) {
-                model.getChipsModel().add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE,
-                        AssistantChip.Icon.NONE, "T" + i, false, false, "", null));
+            chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
+                    "T" + i, false, false, "", null));
         }
-            model.getChipsModel().add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE,
-                    AssistantChip.Icon.NONE, "X", false, true, "", null));
-        });
+        chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
+                "X", false, true, "", null));
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips));
 
         // Cancel chip is displayed to the user.
         onView(withText("X")).check(matches(isDisplayed()));
@@ -135,14 +141,14 @@ public class AutofillAssistantActionsCarouselUiTest {
 
         // Note: this should be a large number that does not fit on screen without scrolling.
         int numChips = 30;
-        TestThreadUtils.runOnUiThreadBlocking(() -> {
+        List<AssistantChip> chips = new ArrayList<>();
         for (int i = 0; i < numChips; i++) {
-                model.getChipsModel().add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE,
-                        AssistantChip.Icon.NONE, "Test" + i, false, false, "", null));
+            chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
+                    "Test" + i, false, false, "", null));
         }
-            model.getChipsModel().add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE,
-                    AssistantChip.Icon.NONE, "Cancel", false, true, "", null));
-        });
+        chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
+                "Cancel", false, true, "", null));
+        TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips));
 
         // Cancel chip is initially displayed to the user.
         onView(withText("Cancel")).check(matches(isDisplayed()));

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTest.java

@@ -221,7 +221,7 @@ public class AutofillAssistantUiTest {
                 new AssistantChip(AssistantChip.Type.CHIP_ASSISTIVE, AssistantChip.Icon.NONE,
                         "chip 1",
                         /* disabled= */ false, /* sticky= */ false, "", mRunnableMock));
-        ThreadUtils.runOnUiThreadBlocking(() -> carouselModel.getChipsModel().set(chips));
+        ThreadUtils.runOnUiThreadBlocking(() -> carouselModel.setChips(chips));
         RecyclerView chipsViewContainer = carouselCoordinator.getView();
         Assert.assertEquals(2, chipsViewContainer.getAdapter().getItemCount());
 

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/EditDistanceTest.java

-// Copyright 2019 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_assistant;
-
-import static org.mockito.Mockito.inOrder;
-import static org.mockito.Mockito.verify;
-
-import android.support.test.filters.SmallTest;
-
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.mockito.InOrder;
-import org.mockito.Mockito;
-import org.mockito.junit.MockitoJUnit;
-import org.mockito.junit.MockitoRule;
-
-import org.chromium.base.test.util.DisabledTest;
-import org.chromium.ui.modelutil.ListModel;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-
-/**
- * Unit test suite for {@link EditDistance}.
- */
-@RunWith(JUnit4.class)
-public class EditDistanceTest {
-    @Rule
-    public MockitoRule mMockitoRule = MockitoJUnit.rule();
-
-    @Test
-    @SmallTest
-    public void testEmptySource() {
-        ListModel<Integer> spiedListModel = createSpiedListModel();
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).add(0, 3);
-        inOrder.verify(spiedListModel).add(0, 2);
-        inOrder.verify(spiedListModel).add(0, 1);
-    }
-
-    @Test
-    @SmallTest
-    @DisabledTest(message = "crbug.com/963672")
-    public void testEmptyTarget() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3);
-        testTransformation(spiedListModel, Collections.emptyList());
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).removeAt(2);
-        inOrder.verify(spiedListModel).removeAt(1);
-        inOrder.verify(spiedListModel).removeAt(0);
-    }
-
-    @Test
-    @SmallTest
-    public void testEmptySourceAndEmptyTarget() {
-        testTransformation(createSpiedListModel(), Collections.emptyList());
-    }
-
-    @Test
-    @SmallTest
-    public void testEqualSourceAndTarget() {
-        testTransformation(createSpiedListModel(1, 2, 3), Arrays.asList(1, 2, 3));
-    }
-
-    @Test
-    @SmallTest
-    public void testInsertBeginning() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(2, 3);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        verify(spiedListModel).add(0, 1);
-    }
-
-    @Test
-    @SmallTest
-    public void testInsertMiddle() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 3);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        verify(spiedListModel).add(1, 2);
-    }
-
-    @Test
-    @SmallTest
-    public void testInsertEnd() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        verify(spiedListModel).add(2, 3);
-    }
-
-    @Test
-    @SmallTest
-    public void testDeleteBeginning() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3);
-        testTransformation(spiedListModel, Arrays.asList(2, 3));
-        verify(spiedListModel).removeAt(0);
-    }
-
-    @Test
-    @SmallTest
-    public void testDeleteMiddle() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3);
-        testTransformation(spiedListModel, Arrays.asList(1, 3));
-        verify(spiedListModel).removeAt(1);
-    }
-
-    @Test
-    @SmallTest
-    @DisabledTest(message = "crbug.com/963672")
-    public void testDeleteEnd() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3);
-        testTransformation(spiedListModel, Arrays.asList(1, 2));
-        verify(spiedListModel).removeAt(2);
-    }
-
-    @Test
-    @SmallTest
-    public void testSubstitutionBeginning() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(0, 2, 3);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        verify(spiedListModel).update(0, 1);
-    }
-
-    @Test
-    @SmallTest
-    public void testSubstitutionMiddle() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 0, 3);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        verify(spiedListModel).update(1, 2);
-    }
-
-    @Test
-    @SmallTest
-    public void testSubstitutionEnd() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 0);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3));
-        verify(spiedListModel).update(2, 3);
-    }
-
-    @Test
-    @SmallTest
-    public void testMultipleInsert() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(3, 6);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8));
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).add(2, 8);
-        inOrder.verify(spiedListModel).add(2, 7);
-        inOrder.verify(spiedListModel).add(1, 5);
-        inOrder.verify(spiedListModel).add(1, 4);
-        inOrder.verify(spiedListModel).add(0, 2);
-        inOrder.verify(spiedListModel).add(0, 1);
-    }
-
-    @Test
-    @SmallTest
-    @DisabledTest(message = "crbug.com/963672")
-    public void testMultipleDelete() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3, 4, 5, 6, 7, 8);
-        testTransformation(spiedListModel, Arrays.asList(3, 6));
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).removeAt(7);
-        inOrder.verify(spiedListModel).removeAt(6);
-        inOrder.verify(spiedListModel).removeAt(4);
-        inOrder.verify(spiedListModel).removeAt(3);
-        inOrder.verify(spiedListModel).removeAt(1);
-        inOrder.verify(spiedListModel).removeAt(0);
-    }
-
-    @Test
-    @SmallTest
-    public void testMultipleSubstitutions() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(0, 0, 3, 0, 0, 6, 0, 0);
-        testTransformation(spiedListModel, Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8));
-        verify(spiedListModel).update(0, 1);
-        verify(spiedListModel).update(1, 2);
-        verify(spiedListModel).update(3, 4);
-        verify(spiedListModel).update(4, 5);
-        verify(spiedListModel).update(6, 7);
-        verify(spiedListModel).update(7, 8);
-    }
-
-    @Test
-    @SmallTest
-    public void testPopAndAppend() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3);
-        testTransformation(spiedListModel, Arrays.asList(2, 3, 4));
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).add(3, 4);
-        inOrder.verify(spiedListModel).removeAt(0);
-    }
-
-    @Test
-    @SmallTest
-    public void testAppendAndPop() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3);
-        testTransformation(spiedListModel, Arrays.asList(0, 1, 2));
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).removeAt(2);
-        inOrder.verify(spiedListModel).add(0, 0);
-    }
-
-    @Test
-    @SmallTest
-    public void testMixedOperations() {
-        ListModel<Integer> spiedListModel = createSpiedListModel(1, 2, 3, 4, 5, 6);
-        testTransformation(spiedListModel, Arrays.asList(2, 8, 4, 5, 7, 6));
-
-        // 0-cost substitutions.
-        verify(spiedListModel).update(1, 2);
-        verify(spiedListModel).update(3, 4);
-        verify(spiedListModel).update(4, 5);
-        verify(spiedListModel).update(5, 6);
-
-        verify(spiedListModel).update(2, 8);
-        InOrder inOrder = inOrder(spiedListModel);
-        inOrder.verify(spiedListModel).add(5, 7);
-        inOrder.verify(spiedListModel).removeAt(0);
-    }
-
-    private void testTransformation(ListModel<Integer> source, List<Integer> target) {
-        EditDistance.transform(source, target, Integer::equals);
-
-        List<Integer> sourceList = new ArrayList<>();
-        for (Integer value : source) {
-            sourceList.add(value);
-        }
-        Assert.assertEquals(target, sourceList);
-    }
-
-    private static ListModel<Integer> createSpiedListModel(Integer... values) {
-        ListModel<Integer> model = new ListModel<>();
-        model.set(values);
-        return Mockito.spy(model);
-    }
-}