From a4ae2e1f43122e529f816fe3e50ca47586bafd8a Mon Sep 17 00:00:00 2001 From: Friedrich Horschig Date: Wed, 16 May 2018 17:59:49 +0000 Subject: [PATCH] Render Tabs into Bottom sheet This CL shapes the role of a KeyboardAccessoryData.Tab element: It is an element that provides a specific benefit to the Manual UI by... ... providing data that is used by the bar to render a tab (upcoming) ... providing the content for a separate tab in the bottom sheet. This means, that the bottom sheet provides space for tabs and manages the general visibility and active states of tabs. The exact content of a tab doesn't matter to the AccessorySheetComponent. That way, it should become fairly straight forward to implement a new tab that is rendered into bottom sheet and accessory. The first concrete instance of a bottom sheet will follow soon: a bottom sheet that contains password related actions and data. (It's important to not mix this specific data into the AccessorySheet component as other upcoming sheets are already planned, like one for payments and one for address data.) Bug: 811747 Change-Id: Id8bd8c389496246166d26a6c298056a142c36566 Reviewed-on: https://chromium-review.googlesource.com/1047285 Commit-Queue: Friedrich Horschig Reviewed-by: Bernhard Bauer Cr-Commit-Position: refs/heads/master@{#559180} --- .../java/res/layout/empty_accessory_sheet.xml | 13 +++ .../res/layout/keyboard_accessory_sheet.xml | 5 +- .../chrome/browser/ChromeActivity.java | 16 +-- .../AccessoryPagerAdapter.java | 70 ++++++++++++ .../AccessorySheetCoordinator.java | 32 +++++- .../AccessorySheetMediator.java | 14 ++- .../AccessorySheetModel.java | 31 +++++- .../AccessorySheetViewBinder.java | 17 ++- .../KeyboardAccessoryCoordinator.java | 14 +-- .../KeyboardAccessoryData.java | 28 +++++ .../KeyboardAccessoryModel.java | 49 +-------- .../KeyboardAccessoryViewBinder.java | 30 +++--- .../ManualFillingController.java | 35 ------ .../ManualFillingCoordinator.java | 70 ++++++++++++ .../modelutil/SimpleListObservable.java | 77 +++++++++++++ chrome/android/java_sources.gni | 5 +- .../AccessorySheetViewTest.java | 59 +++++++++- .../KeyboardAccessoryViewTest.java | 10 ++ .../AccessorySheetControllerTest.java | 61 +++++++++-- .../KeyboardAccessoryControllerTest.java | 2 +- .../ManualFillingControllerTest.java | 23 ++-- .../chromium/chrome/browser/modelutil/OWNERS | 1 + .../modelutil/SimpleListObservableTest.java | 101 ++++++++++++++++++ 23 files changed, 618 insertions(+), 145 deletions(-) create mode 100644 chrome/android/java/res/layout/empty_accessory_sheet.xml create mode 100644 chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessoryPagerAdapter.java delete mode 100644 chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingController.java create mode 100644 chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java create mode 100644 chrome/android/java/src/org/chromium/chrome/browser/modelutil/SimpleListObservable.java create mode 100644 chrome/android/junit/src/org/chromium/chrome/browser/modelutil/OWNERS create mode 100644 chrome/android/junit/src/org/chromium/chrome/browser/modelutil/SimpleListObservableTest.java diff --git a/chrome/android/java/res/layout/empty_accessory_sheet.xml b/chrome/android/java/res/layout/empty_accessory_sheet.xml new file mode 100644 index 00000000000000..831451cc575059 --- /dev/null +++ b/chrome/android/java/res/layout/empty_accessory_sheet.xml @@ -0,0 +1,13 @@ + + + + + diff --git a/chrome/android/java/res/layout/keyboard_accessory_sheet.xml b/chrome/android/java/res/layout/keyboard_accessory_sheet.xml index 5e6d7f28311c4e..50ae12aa237083 100644 --- a/chrome/android/java/res/layout/keyboard_accessory_sheet.xml +++ b/chrome/android/java/res/layout/keyboard_accessory_sheet.xml @@ -3,14 +3,11 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> - \ No newline at end of file diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java index 131f0974306f52..5c808884a79e29 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java @@ -59,6 +59,7 @@ 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.autofill.keyboard_accessory.ManualFillingCoordinator; import org.chromium.chrome.browser.bookmarks.BookmarkModel; import org.chromium.chrome.browser.bookmarks.BookmarkUtils; import org.chromium.chrome.browser.compositor.CompositorViewHolder; @@ -266,7 +267,7 @@ public AppMenuHandler get(Activity activity, AppMenuPropertiesDelegate delegate, private BottomSheet mBottomSheet; private ContextualSuggestionsCoordinator mContextualSuggestionsCoordinator; private FadingBackgroundView mFadingBackgroundView; - private KeyboardAccessoryCoordinator mKeyboardAccessoryCoordinator; + private ManualFillingCoordinator mManualFillingController; // Time in ms that it took took us to inflate the initial layout private long mInflateInitialLayoutDurationMs; @@ -391,8 +392,9 @@ public void postInflationStartup() { // SurfaceView's 'hole' clipping during animations that are notified to the window. getWindowAndroid().setAnimationPlaceholderView(mCompositorViewHolder.getCompositorView()); - mKeyboardAccessoryCoordinator = new KeyboardAccessoryCoordinator( - getWindowAndroid(), findViewById(R.id.keyboard_accessory_stub)); + mManualFillingController = new ManualFillingCoordinator(getWindowAndroid(), + findViewById(R.id.keyboard_accessory_stub), + findViewById(R.id.keyboard_accessory_sheet_stub)); initializeToolbar(); initializeTabModels(); @@ -676,7 +678,7 @@ public FindToolbarManager getFindToolbarManager() { * @return The {@link KeyboardAccessoryCoordinator} that belongs to this activity. */ public KeyboardAccessoryCoordinator getKeyboardAccessory() { - return mKeyboardAccessoryCoordinator; + return mManualFillingController.getKeyboardAccessory(); } /** @@ -1223,9 +1225,9 @@ protected final void onDestroy() { mTabContentManager = null; } - if (mKeyboardAccessoryCoordinator != null) { - mKeyboardAccessoryCoordinator.destroy(); - mKeyboardAccessoryCoordinator = null; + if (mManualFillingController != null) { + mManualFillingController.destroy(); + mManualFillingController = null; } AccessibilityManager manager = (AccessibilityManager) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessoryPagerAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessoryPagerAdapter.java new file mode 100644 index 00000000000000..4234db41477fd2 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessoryPagerAdapter.java @@ -0,0 +1,70 @@ +// 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.support.annotation.NonNull; +import android.support.annotation.Nullable; +import android.support.v4.view.PagerAdapter; +import android.support.v4.view.ViewPager; +import android.view.LayoutInflater; +import android.view.View; +import android.view.ViewGroup; + +import org.chromium.chrome.browser.modelutil.SimpleListObservable; + +import java.util.HashMap; +import java.util.Map; + +/** + * This {@link PagerAdapter} renders an observable list of {@link KeyboardAccessoryData.Tab}s into a + * {@link ViewPager}. It instantiates the tab views based on the layout they provide. + */ +class AccessoryPagerAdapter extends PagerAdapter { + private final SimpleListObservable mTabList; + private final Map mViews; + + /** + * Creates the PagerAdapter that populates a ViewPager based on a held list of tabs. + * @param tabList The list that contains the tabs to instantiate. + */ + public AccessoryPagerAdapter(SimpleListObservable tabList) { + mTabList = tabList; + mViews = new HashMap<>(mTabList.getItemCount()); + } + + @NonNull + @Override + public Object instantiateItem(@NonNull ViewGroup container, int position) { + KeyboardAccessoryData.Tab tab = mTabList.get(position); + ViewGroup layout = mViews.get(tab); + if (layout == null) { + layout = (ViewGroup) LayoutInflater.from(container.getContext()) + .inflate(tab.getTabLayout(), container, false); + mViews.put(tab, layout); + if (container.indexOfChild(layout) == -1) container.addView(layout); + if (tab.getListener() != null) { + tab.getListener().onTabCreated(layout); + } + } + return layout; + } + + @Override + public void destroyItem(@NonNull ViewGroup container, int position, @Nullable Object object) { + if (mViews.get(mTabList.get(position)) == null) return; + ViewGroup layout = mViews.get(mTabList.get(position)); + if (container.indexOfChild(layout) != -1) container.removeView(layout); + } + + @Override + public int getCount() { + return mTabList.getItemCount(); + } + + @Override + public boolean isViewFromObject(@NonNull View view, @NonNull Object o) { + return view == o; + } +} diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetCoordinator.java index bfe4a7e56b38f5..951bfab5726bc8 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetCoordinator.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetCoordinator.java @@ -4,7 +4,9 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory; -import android.view.View; +import android.support.annotation.Nullable; +import android.support.v4.view.PagerAdapter; +import android.support.v4.view.ViewPager; import android.view.ViewStub; import org.chromium.base.VisibleForTesting; @@ -14,7 +16,7 @@ /** * Creates and owns all elements which are part of the accessory sheet component. * It's part of the controller but will mainly forward events (like showing the sheet) and handle - * communication with the {@link ManualFillingController} (e.g. add a tab to trigger the sheet). + * communication with the {@link ManualFillingCoordinator} (e.g. add a tab to trigger the sheet). * to the {@link AccessorySheetMediator}. */ public class AccessorySheetCoordinator { @@ -26,7 +28,7 @@ public class AccessorySheetCoordinator { * @param viewStub The view stub that can be inflated into the accessory layout. */ public AccessorySheetCoordinator(ViewStub viewStub) { - LazyViewBinderAdapter.StubHolder stubHolder = + LazyViewBinderAdapter.StubHolder stubHolder = new LazyViewBinderAdapter.StubHolder<>(viewStub); AccessorySheetModel model = new AccessorySheetModel(); model.addObserver(new PropertyModelChangeProcessor<>( @@ -34,10 +36,34 @@ public AccessorySheetCoordinator(ViewStub viewStub) { mMediator = new AccessorySheetMediator(model); } + /** + * Creates the {@link PagerAdapter} for the newly inflated {@link ViewPager}. + * If any ListModelChangeProcessor<> is needed, it would be created here. Currently, connecting + * the model.getTabList() to the tabViewBinder would have no effect as only the change of + * ACTIVE_TAB affects the view. + * @param model The model containing the list of tabs to be displayed. + * @return A fully initialized {@link PagerAdapter}. + */ + static PagerAdapter createTabViewAdapter(AccessorySheetModel model) { + return new AccessoryPagerAdapter(model.getTabList()); + } + + /** + * Adds the contents of a given {@link KeyboardAccessoryData.Tab} to the accessory sheet. If it + * is the first Tab, it automatically becomes the active Tab. + * Careful, if you want to show this tab as icon in the KeyboardAccessory, use the method + * {@link ManualFillingCoordinator#addTab(KeyboardAccessoryData.Tab)} instead. + * @param tab The tab which should be added to the AccessorySheet. + */ + void addTab(KeyboardAccessoryData.Tab tab) { + mMediator.addTab(tab); + } + /** * Returns a {@link KeyboardAccessoryData.Tab} object that is used to display this bottom sheet. * @return Returns a {@link KeyboardAccessoryData.Tab}. */ + @Nullable public KeyboardAccessoryData.Tab getTab() { return mMediator.getTab(); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetMediator.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetMediator.java index f6cc210500a9ed..55bee9878c0555 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetMediator.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetMediator.java @@ -4,6 +4,8 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory; +import android.support.annotation.Nullable; + import org.chromium.base.VisibleForTesting; /** @@ -12,14 +14,15 @@ */ class AccessorySheetMediator { private final AccessorySheetModel mModel; - private KeyboardAccessoryData.Tab mTab; AccessorySheetMediator(AccessorySheetModel model) { mModel = model; } + @Nullable KeyboardAccessoryData.Tab getTab() { - return null; // TODO(fhorschig): Return the active tab. + if (mModel.getActiveTabIndex() == AccessorySheetModel.NO_ACTIVE_TAB) return null; + return mModel.getTabList().get(mModel.getActiveTabIndex()); } @VisibleForTesting @@ -34,4 +37,11 @@ public void show() { public void hide() { mModel.setVisible(false); } + + public void addTab(KeyboardAccessoryData.Tab tab) { + mModel.getTabList().add(tab); + if (mModel.getActiveTabIndex() == AccessorySheetModel.NO_ACTIVE_TAB) { + mModel.setActiveTabIndex(mModel.getTabList().getItemCount() - 1); + } + } } \ No newline at end of file diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetModel.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetModel.java index 43bec8a3091cc9..37feaf9b39e7b1 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetModel.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetModel.java @@ -4,7 +4,9 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory; +import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.Tab; import org.chromium.chrome.browser.modelutil.PropertyObservable; +import org.chromium.chrome.browser.modelutil.SimpleListObservable; /** * This model holds all view state of the accessory sheet. @@ -12,10 +14,22 @@ * like the view binder react. */ class AccessorySheetModel extends PropertyObservable { - public static class PropertyKey { public static final PropertyKey VISIBLE = new PropertyKey(); } + public static class PropertyKey { + public static final PropertyKey ACTIVE_TAB_INDEX = new PropertyKey(); + public static final PropertyKey VISIBLE = new PropertyKey(); + } + + public static final int NO_ACTIVE_TAB = -1; + + private int mActiveTabIndex = NO_ACTIVE_TAB; private boolean mVisible; + private final SimpleListObservable mTabList = new SimpleListObservable<>(); + + SimpleListObservable getTabList() { + return mTabList; + } - public void setVisible(boolean visible) { + void setVisible(boolean visible) { if (mVisible == visible) return; // Nothing to do here: same value. mVisible = visible; notifyPropertyChanged(PropertyKey.VISIBLE); @@ -24,4 +38,17 @@ public void setVisible(boolean visible) { boolean isVisible() { return mVisible; } + + int getActiveTabIndex() { + return mActiveTabIndex; + } + + void setActiveTabIndex(int activeTabPosition) { + if (mActiveTabIndex == activeTabPosition) return; + assert((activeTabPosition >= 0 && activeTabPosition < mTabList.getItemCount()) + || activeTabPosition == NO_ACTIVE_TAB) + : "Tried to set invalid index '" + activeTabPosition + "' as active tab!"; + mActiveTabIndex = activeTabPosition; + notifyPropertyChanged(PropertyKey.ACTIVE_TAB_INDEX); + } } \ No newline at end of file diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewBinder.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewBinder.java index f5f07d7f7be245..4a22ff7a3bc8e5 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewBinder.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewBinder.java @@ -4,6 +4,7 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory; +import android.support.v4.view.ViewPager; import android.view.View; import org.chromium.chrome.browser.autofill.keyboard_accessory.AccessorySheetModel.PropertyKey; @@ -14,7 +15,8 @@ * {@link AccessorySheetViewBinder} which will modify the view accordingly. */ class AccessorySheetViewBinder - implements LazyViewBinderAdapter.SimpleViewBinder { + implements LazyViewBinderAdapter + .SimpleViewBinder { @Override public PropertyKey getVisibilityProperty() { return PropertyKey.VISIBLE; @@ -26,14 +28,23 @@ public boolean isVisible(AccessorySheetModel model) { } @Override - public void onInitialInflation(AccessorySheetModel model, View inflatedView) {} + public void onInitialInflation(AccessorySheetModel model, ViewPager inflatedView) { + if (model.getActiveTabIndex() != -1) inflatedView.setCurrentItem(model.getActiveTabIndex()); + inflatedView.setAdapter(AccessorySheetCoordinator.createTabViewAdapter(model)); + } @Override - public void bind(AccessorySheetModel model, View inflatedView, PropertyKey propertyKey) { + public void bind(AccessorySheetModel model, ViewPager inflatedView, PropertyKey propertyKey) { if (propertyKey == PropertyKey.VISIBLE) { inflatedView.setVisibility(model.isVisible() ? View.VISIBLE : View.GONE); return; } + if (propertyKey == PropertyKey.ACTIVE_TAB_INDEX) { + if (model.getActiveTabIndex() != AccessorySheetModel.NO_ACTIVE_TAB) { + inflatedView.setCurrentItem(model.getActiveTabIndex()); + } + return; + } assert false : "Every possible property update needs to be handled!"; } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java index 47ed62fa14e8db..b8d744e3760034 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java @@ -16,6 +16,7 @@ import org.chromium.chrome.browser.modelutil.PropertyModelChangeProcessor; import org.chromium.chrome.browser.modelutil.RecyclerViewAdapter; import org.chromium.chrome.browser.modelutil.RecyclerViewModelChangeProcessor; +import org.chromium.chrome.browser.modelutil.SimpleListObservable; import org.chromium.ui.UiUtils; import org.chromium.ui.base.WindowAndroid; @@ -49,12 +50,11 @@ public KeyboardAccessoryCoordinator(WindowAndroid windowAndroid, ViewStub viewSt * @param model the {@link KeyboardAccessoryModel} the adapter gets its data from. * @return Returns a fully initialized and wired adapter to an ActionViewBinder. */ - static RecyclerViewAdapter, - ActionViewBinder.ViewHolder> + static RecyclerViewAdapter, ActionViewBinder.ViewHolder> createActionsAdapter(KeyboardAccessoryModel model) { - RecyclerViewAdapter, - ActionViewBinder.ViewHolder> actionsAdapter = - new RecyclerViewAdapter<>(model.getActionList(), new ActionViewBinder()); + RecyclerViewAdapter, ActionViewBinder.ViewHolder> + actionsAdapter = + new RecyclerViewAdapter<>(model.getActionList(), new ActionViewBinder()); model.addActionListObserver(new RecyclerViewModelChangeProcessor<>(actionsAdapter)); return actionsAdapter; } @@ -79,7 +79,7 @@ static TabViewBinder createTabViewBinder( * 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) { + void addTab(KeyboardAccessoryData.Tab tab) { mMediator.addTab(tab); } @@ -88,7 +88,7 @@ public void addTab(KeyboardAccessoryData.Tab tab) { * from the accessory. * @param tab The tab to be removed. */ - public void removeTab(KeyboardAccessoryData.Tab tab) { + void removeTab(KeyboardAccessoryData.Tab tab) { mMediator.removeTab(tab); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java index 1a191451919fed..afcc801808def8 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java @@ -5,6 +5,9 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory; import android.graphics.drawable.Drawable; +import android.support.annotation.LayoutRes; +import android.support.annotation.Nullable; +import android.view.ViewGroup; import java.util.ArrayList; import java.util.List; @@ -46,6 +49,17 @@ public interface Observer { * accessory. Typically, a tab is responsible to change the bottom sheet below the accessory. */ public interface Tab { + /** + * A Tab's Listener get's notified when e.g. the Tab was assigned a view. + */ + interface Listener { + /** + * Triggered when the tab was successfully created. + * @param view The newly created accessory sheet of the tab. + */ + void onTabCreated(ViewGroup view); + } + /** * Provides the icon that will be displayed in the {@link KeyboardAccessoryCoordinator}. * @return The small icon that identifies this tab uniquely. @@ -57,6 +71,20 @@ public interface Tab { * @return A short string describing the task of this tab. */ String getContentDescription(); + + /** + * Returns the tab layout which allows to create the tab's view on demand. + * @return The layout resource that allows to create the view necessary for this tab. + */ + @LayoutRes + int getTabLayout(); + + /** + * Returns the listener which might need to react on changes to this tab. + * @return A {@link Listener} to be called, e.g. when the tab is created. + */ + @Nullable + Listener getListener(); } /** diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java index 5657e0be4984b5..d452c36e927f7e 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java @@ -7,9 +7,9 @@ import org.chromium.chrome.browser.autofill.AutofillKeyboardSuggestions; import org.chromium.chrome.browser.modelutil.ListObservable; import org.chromium.chrome.browser.modelutil.PropertyObservable; +import org.chromium.chrome.browser.modelutil.SimpleListObservable; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; /** @@ -34,53 +34,6 @@ private PropertyKey() { } } - /** A {@link ListObservable} containing an {@link ArrayList} of Tabs or Actions. */ - class SimpleListObservable extends ListObservable { - private final List mItems = new ArrayList<>(); - - public T get(int index) { - return mItems.get(index); - } - - @Override - public int getItemCount() { - return mItems.size(); - } - - void add(T item) { - mItems.add(item); - notifyItemRangeInserted(mItems.size() - 1, 1); - } - - void remove(T item) { - int position = mItems.indexOf(item); - if (position == -1) { - return; - } - mItems.remove(position); - notifyItemRangeRemoved(position, 1); - } - - void set(T[] newItems) { - if (mItems.isEmpty()) { - if (newItems.length == 0) { - return; // Nothing to do, nothing changes. - } - mItems.addAll(Arrays.asList(newItems)); - notifyItemRangeInserted(0, mItems.size()); - return; - } - int oldSize = mItems.size(); - mItems.clear(); - if (newItems.length == 0) { - notifyItemRangeRemoved(0, oldSize); - return; - } - mItems.addAll(Arrays.asList(newItems)); - notifyItemRangeChanged(0, Math.max(oldSize, mItems.size()), this); - } - } - private SimpleListObservable mActionListObservable; private SimpleListObservable mTabListObservable; private boolean mVisible; diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewBinder.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewBinder.java index a82b80fe3cffb9..8998a00ddc0f60 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewBinder.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewBinder.java @@ -15,6 +15,7 @@ import org.chromium.chrome.browser.modelutil.LazyViewBinderAdapter; import org.chromium.chrome.browser.modelutil.ListModelChangeProcessor; import org.chromium.chrome.browser.modelutil.RecyclerViewAdapter; +import org.chromium.chrome.browser.modelutil.SimpleListObservable; import org.chromium.ui.widget.ButtonCompat; /** @@ -24,8 +25,9 @@ class KeyboardAccessoryViewBinder implements LazyViewBinderAdapter.SimpleViewBinder { - static class ActionViewBinder implements RecyclerViewAdapter.ViewBinder< - KeyboardAccessoryModel.SimpleListObservable, ActionViewBinder.ViewHolder> { + static class ActionViewBinder + implements RecyclerViewAdapter.ViewBinder, + ActionViewBinder.ViewHolder> { static class ViewHolder extends RecyclerView.ViewHolder { public ViewHolder(ButtonCompat actionView) { super(actionView); @@ -44,8 +46,8 @@ public ActionViewBinder.ViewHolder onCreateViewHolder(ViewGroup parent, int view } @Override - public void onBindViewHolder(KeyboardAccessoryModel.SimpleListObservable actions, - ViewHolder holder, int position) { + public void onBindViewHolder( + SimpleListObservable actions, ViewHolder holder, int position) { final Action action = actions.get(position); holder.getActionView().setText(action.getCaption()); holder.getActionView().setOnClickListener( @@ -53,11 +55,12 @@ public void onBindViewHolder(KeyboardAccessoryModel.SimpleListObservable } } - static class TabViewBinder implements ListModelChangeProcessor.ViewBinder< - KeyboardAccessoryModel.SimpleListObservable, KeyboardAccessoryView> { + static class TabViewBinder + implements ListModelChangeProcessor + .ViewBinder, KeyboardAccessoryView> { @Override - public void onItemsInserted(KeyboardAccessoryModel.SimpleListObservable model, - KeyboardAccessoryView view, int index, int count) { + public void onItemsInserted( + SimpleListObservable model, KeyboardAccessoryView view, int index, int count) { assert count > 0 : "Tried to insert invalid amount of tabs - must be at least one."; while (count-- > 0) { Tab tab = model.get(index); @@ -67,8 +70,8 @@ public void onItemsInserted(KeyboardAccessoryModel.SimpleListObservable mod } @Override - public void onItemsRemoved(KeyboardAccessoryModel.SimpleListObservable model, - KeyboardAccessoryView view, int index, int count) { + public void onItemsRemoved( + SimpleListObservable model, KeyboardAccessoryView view, int index, int count) { assert count > 0 : "Tried to remove invalid amount of tabs - must be at least one."; while (count-- > 0) { view.removeTabAt(index++); @@ -76,14 +79,13 @@ public void onItemsRemoved(KeyboardAccessoryModel.SimpleListObservable mode } @Override - public void onItemsChanged(KeyboardAccessoryModel.SimpleListObservable model, - KeyboardAccessoryView view, int index, int count) { + public void onItemsChanged( + SimpleListObservable model, KeyboardAccessoryView view, int index, int count) { // TODO(fhorschig): Implement fine-grained, ranged changes should the need arise. updateAllTabs(view, model); } - void updateAllTabs(KeyboardAccessoryView view, - KeyboardAccessoryModel.SimpleListObservable model) { + void updateAllTabs(KeyboardAccessoryView view, SimpleListObservable model) { view.clearTabs(); for (int i = 0; i < model.getItemCount(); ++i) { Tab tab = model.get(i); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingController.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingController.java deleted file mode 100644 index 3712f651442289..00000000000000 --- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingController.java +++ /dev/null @@ -1,35 +0,0 @@ -// 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; - -// TODO(fhorschig): Inline this class if the setting up/destroying is less overhead than expected. -// TODO(fhorschig): Otherwise, replace the KeyboardAccessorySheet in the ChromeActivity. -/** - * Handles requests to the manual UI for filling passwords, payments and other user data. Ideally, - * the caller has no access to Keyboard accessory or sheet and is only interacting with this - * component. - * For that, it facilitates the communication between {@link KeyboardAccessoryCoordinator} and - * {@link AccessorySheetCoordinator} to add and trigger surfaces that may assist users while filling - * fields. - */ -public class ManualFillingController { - private final KeyboardAccessoryCoordinator mKeyboardAccessoryCoordinator; - - /** - * Creates a the manual filling controller. - * @param keyboardAccessoryCoordinator The keybaord - */ - public ManualFillingController(KeyboardAccessoryCoordinator keyboardAccessoryCoordinator) { - mKeyboardAccessoryCoordinator = keyboardAccessoryCoordinator; - } - - /** - * Links an {@link AccessorySheetCoordinator} to the {@link KeyboardAccessoryCoordinator}. - * @param accessorySheetCoordinator The sheet component to be linked. - */ - public void addAccessorySheet(AccessorySheetCoordinator accessorySheetCoordinator) { - mKeyboardAccessoryCoordinator.addTab(accessorySheetCoordinator.getTab()); - } -} \ No newline at end of file diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java new file mode 100644 index 00000000000000..f146afd6992e89 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java @@ -0,0 +1,70 @@ +// 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.base.VisibleForTesting; +import org.chromium.ui.base.WindowAndroid; + +/** + * Handles requests to the manual UI for filling passwords, payments and other user data. Ideally, + * the caller has no access to Keyboard accessory or sheet and is only interacting with this + * component. + * For that, it facilitates the communication between {@link KeyboardAccessoryCoordinator} and + * {@link AccessorySheetCoordinator} to add and trigger surfaces that may assist users while filling + * fields. + */ +public class ManualFillingCoordinator { + private final KeyboardAccessoryCoordinator mKeyboardAccessory; + private final AccessorySheetCoordinator mAccessorySheet; + + /** + * Creates a the manual filling controller. + * @param windowAndroid The window needed to set up the sub components. + * @param keyboardAccessoryStub The view stub for keyboard accessory bar. + * @param accessorySheetStub The view stub for the keyboard accessory bottom sheet. + */ + public ManualFillingCoordinator(WindowAndroid windowAndroid, ViewStub keyboardAccessoryStub, + ViewStub accessorySheetStub) { + mKeyboardAccessory = new KeyboardAccessoryCoordinator(windowAndroid, keyboardAccessoryStub); + mAccessorySheet = new AccessorySheetCoordinator(accessorySheetStub); + } + + /** + * Cleans up the manual UI by destroying the accessory bar and its bottom sheet. + */ + public void destroy() { + mKeyboardAccessory.destroy(); + } + + /** + * Links a tab to the manual UI by adding it to the held {@link AccessorySheetCoordinator} and + * the {@link KeyboardAccessoryCoordinator}. + * @param tab The tab component to be added. + */ + public void addTab(KeyboardAccessoryData.Tab tab) { + mKeyboardAccessory.addTab(tab); + mAccessorySheet.addTab(tab); + } + + /** + * Allows access to the keyboard accessory. This can be used to explicitly modify the the bar of + * the keyboard accessory (e.g. by providing suggestions or actions). + * @return The coordinator of the Keyboard accessory component. + */ + public KeyboardAccessoryCoordinator getKeyboardAccessory() { + return mKeyboardAccessory; + } + + /** + * Allows access to the accessory sheet. + * @return The coordinator of the Accessory sheet component. + */ + @VisibleForTesting + public AccessorySheetCoordinator getAccessorySheetForTesting() { + return mAccessorySheet; + } +} \ No newline at end of file diff --git a/chrome/android/java/src/org/chromium/chrome/browser/modelutil/SimpleListObservable.java b/chrome/android/java/src/org/chromium/chrome/browser/modelutil/SimpleListObservable.java new file mode 100644 index 00000000000000..e382f36e45b9dd --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/modelutil/SimpleListObservable.java @@ -0,0 +1,77 @@ +// 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.modelutil; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * A {@link ListObservable} containing an {@link ArrayList} of Tabs or Actions. + * It allows models to compose different ListObservables. + * @param The object type that this class manages in a list. + */ +public class SimpleListObservable extends ListObservable { + private final List mItems = new ArrayList<>(); + + /** + * Returns the item at the given position. + * @param index The position to get the item from. + * @return Returns the found item. + */ + public T get(int index) { + return mItems.get(index); + } + + @Override + public int getItemCount() { + return mItems.size(); + } + + /** + * Appends a given item to the last position of the held {@link ArrayList}. Notifies observers + * about the inserted item. + * @param item The item to be stored. + */ + public void add(T item) { + mItems.add(item); + notifyItemRangeInserted(mItems.size() - 1, 1); + } + + /** + * Removes a given item from the held {@link ArrayList}. Notifies observers about the removal. + * @param item The item to be removed. + */ + public void remove(T item) { + int position = mItems.indexOf(item); + mItems.remove(position); + notifyItemRangeRemoved(position, 1); + } + + /** + * Replaces all held items with the given array of items. + * If the held list was empty, notify observers about inserted elements. + * If the held list isn't empty but the new list is, notify observer about the removal. + * If the new and old list aren't empty, notify observer about the complete exchange. + * @param newItems The set of items that should replace all held items. + */ + public void set(T[] newItems) { + if (mItems.isEmpty()) { + if (newItems.length == 0) { + return; // Nothing to do, nothing changes. + } + mItems.addAll(Arrays.asList(newItems)); + notifyItemRangeInserted(0, mItems.size()); + return; + } + int oldSize = mItems.size(); + mItems.clear(); + if (newItems.length == 0) { + notifyItemRangeRemoved(0, oldSize); + return; + } + mItems.addAll(Arrays.asList(newItems)); + notifyItemRangeChanged(0, Math.max(oldSize, mItems.size()), this); + } +} \ No newline at end of file diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni index 9b73ff92c099dc..9c4bd29f4f09db 100644 --- a/chrome/android/java_sources.gni +++ b/chrome/android/java_sources.gni @@ -97,6 +97,7 @@ chrome_java_sources = [ "java/src/org/chromium/chrome/browser/autofill/PasswordGenerationPopupBridge.java", "java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java", "java/src/org/chromium/chrome/browser/autofill/PhoneNumberUtil.java", + "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessoryPagerAdapter.java", "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetCoordinator.java", "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetMediator.java", "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetModel.java", @@ -107,7 +108,7 @@ chrome_java_sources = [ "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/autofill/keyboard_accessory/ManualFillingController.java", + "java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.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", @@ -701,6 +702,7 @@ chrome_java_sources = [ "java/src/org/chromium/chrome/browser/modelutil/PropertyObservable.java", "java/src/org/chromium/chrome/browser/modelutil/RecyclerViewModelChangeProcessor.java", "java/src/org/chromium/chrome/browser/modelutil/RecyclerViewAdapter.java", + "java/src/org/chromium/chrome/browser/modelutil/SimpleListObservable.java", "java/src/org/chromium/chrome/browser/mojo/ChromeInterfaceRegistrar.java", "java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceChromeTabbedActivity.java", "java/src/org/chromium/chrome/browser/multiwindow/MultiWindowUtils.java", @@ -2032,6 +2034,7 @@ chrome_junit_test_java_sources = [ "junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationTestTabHolder.java", "junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationTitleUpdatedTest.java", "junit/src/org/chromium/chrome/browser/metrics/VariationsSessionTest.java", + "junit/src/org/chromium/chrome/browser/modelutil/SimpleListObservableTest.java", "junit/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeUnitTest.java", "junit/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitionsTest.java", "junit/src/org/chromium/chrome/browser/ntp/NativePageFactoryTest.java", diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewTest.java index e45fba67e554d5..0ee2bbc735c388 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetViewTest.java @@ -4,13 +4,23 @@ package org.chromium.chrome.browser.autofill.keyboard_accessory; +import static android.support.test.espresso.Espresso.onView; +import static android.support.test.espresso.assertion.ViewAssertions.matches; +import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; +import static android.support.test.espresso.matcher.ViewMatchers.withText; + import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertNull; import static org.junit.Assert.assertTrue; +import android.graphics.drawable.Drawable; +import android.support.annotation.LayoutRes; import android.support.test.filters.MediumTest; +import android.support.v4.view.ViewPager; import android.view.View; +import android.widget.LinearLayout; +import android.widget.TextView; import org.junit.Before; import org.junit.Rule; @@ -34,7 +44,7 @@ @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE}) public class AccessorySheetViewTest { private AccessorySheetModel mModel; - private LazyViewBinderAdapter.StubHolder mStubHolder; + private LazyViewBinderAdapter.StubHolder mStubHolder; @Rule public ChromeActivityTestRule mActivityTestRule = @@ -66,4 +76,51 @@ public void testAccessoryVisibilityChangedByModel() { assertNotNull(mStubHolder.getView()); assertTrue(mStubHolder.getView().getVisibility() != View.VISIBLE); } + + @Test + @MediumTest + public void testAddingTabToModelRendersTabsView() { + final String kSampleAction = "Some Action"; + mModel.getTabList().add(createTestTab(view -> { + assertNotNull("The tab must have been created!", view); + assertTrue("Empty tab is a layout.", view instanceof LinearLayout); + LinearLayout baseLayout = (LinearLayout) view; + TextView sampleTextView = new TextView(mActivityTestRule.getActivity()); + sampleTextView.setText(kSampleAction); + baseLayout.addView(sampleTextView); + })); + mModel.setActiveTabIndex(0); + // Shouldn't cause the view to be inflated. + assertNull(mStubHolder.getView()); + + // Setting visibility should cause the Tab to be rendered. + ThreadUtils.runOnUiThreadBlocking(() -> mModel.setVisible(true)); + assertNotNull(mStubHolder.getView()); + + onView(withText(kSampleAction)).check(matches(isDisplayed())); + } + + private KeyboardAccessoryData.Tab createTestTab(KeyboardAccessoryData.Tab.Listener listener) { + return new KeyboardAccessoryData.Tab() { + @Override + public Drawable getIcon() { + return null; // Unused. + } + + @Override + public String getContentDescription() { + return null; // Unused. + } + + @Override + public @LayoutRes int getTabLayout() { + return R.layout.empty_accessory_sheet; + } + + @Override + public Listener getListener() { + return listener; + } + }; + } } \ No newline at end of file diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewTest.java index 42c2c2a5e9fcd4..5f1146ac017231 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryViewTest.java @@ -90,6 +90,16 @@ public Drawable getIcon() { public String getContentDescription() { return contentDescription; } + + @Override + public int getTabLayout() { + return R.layout.empty_accessory_sheet; // Unused. + } + + @Override + public Listener getListener() { + return null; // Unused. + } }; } diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetControllerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetControllerTest.java index 19527e193ce7c9..6652ad4997b303 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetControllerTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/AccessorySheetControllerTest.java @@ -6,14 +6,15 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.support.test.filters.SmallTest; +import android.support.v4.view.ViewPager; import android.view.ViewStub; -import android.widget.LinearLayout; import org.junit.Before; import org.junit.Test; @@ -24,6 +25,8 @@ import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.util.Feature; +import org.chromium.chrome.browser.autofill.keyboard_accessory.AccessorySheetModel.PropertyKey; +import org.chromium.chrome.browser.modelutil.ListObservable; import org.chromium.chrome.browser.modelutil.PropertyObservable; /** @@ -33,12 +36,17 @@ @Config(manifest = Config.NONE) public class AccessorySheetControllerTest { @Mock - private PropertyObservable - .PropertyObserver mMockPropertyObserver; + private PropertyObservable.PropertyObserver mMockPropertyObserver; + @Mock + private ListObservable.ListObserver mTabListObserver; @Mock private ViewStub mMockViewStub; @Mock - private LinearLayout mMockView; + private ViewPager mMockView; + @Mock + private KeyboardAccessoryData.Tab mMockTab; + @Mock + private KeyboardAccessoryData.Tab mSecondMockTab; private AccessorySheetCoordinator mCoordinator; private AccessorySheetMediator mMediator; @@ -69,19 +77,54 @@ public void testModelNotifiesAboutVisibilityOncePerChange() { // Calling show on the mediator should make model propagate that it's visible. mMediator.show(); - verify(mMockPropertyObserver) - .onPropertyChanged(mModel, AccessorySheetModel.PropertyKey.VISIBLE); + verify(mMockPropertyObserver).onPropertyChanged(mModel, PropertyKey.VISIBLE); assertThat(mModel.isVisible(), is(true)); // Calling show again does nothing. mMediator.show(); verify(mMockPropertyObserver) // Still the same call and no new one added. - .onPropertyChanged(mModel, AccessorySheetModel.PropertyKey.VISIBLE); + .onPropertyChanged(mModel, PropertyKey.VISIBLE); // Calling hide on the mediator should make model propagate that it's invisible. mMediator.hide(); - verify(mMockPropertyObserver, times(2)) - .onPropertyChanged(mModel, AccessorySheetModel.PropertyKey.VISIBLE); + verify(mMockPropertyObserver, times(2)).onPropertyChanged(mModel, PropertyKey.VISIBLE); assertThat(mModel.isVisible(), is(false)); } + + @Test + @SmallTest + @Feature({"keyboard-accessory"}) + public void testModelNotifiesChangesForNewSheet() { + mModel.addObserver(mMockPropertyObserver); + mModel.getTabList().addObserver(mTabListObserver); + + assertThat(mModel.getTabList().getItemCount(), is(0)); + mCoordinator.addTab(mMockTab); + verify(mTabListObserver).onItemRangeInserted(mModel.getTabList(), 0, 1); + assertThat(mModel.getTabList().getItemCount(), is(1)); + } + + @Test + @SmallTest + @Feature({"keyboard-accessory"}) + public void testFirstAddedTabBecomesActiveTab() { + mModel.addObserver(mMockPropertyObserver); + + // Initially, there is no active Tab. + assertThat(mModel.getTabList().getItemCount(), is(0)); + assertThat(mCoordinator.getTab(), is(nullValue())); + + // The first tab becomes the active Tab. + mCoordinator.addTab(mMockTab); + verify(mMockPropertyObserver).onPropertyChanged(mModel, PropertyKey.ACTIVE_TAB_INDEX); + assertThat(mModel.getTabList().getItemCount(), is(1)); + assertThat(mModel.getActiveTabIndex(), is(0)); + assertThat(mCoordinator.getTab(), is(mMockTab)); + + // A second tab is added but doesn't become automatically active. + mCoordinator.addTab(mSecondMockTab); + verify(mMockPropertyObserver).onPropertyChanged(mModel, PropertyKey.ACTIVE_TAB_INDEX); + assertThat(mModel.getTabList().getItemCount(), is(2)); + assertThat(mModel.getActiveTabIndex(), is(0)); + } } \ No newline at end of file diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java index 4aee578d96ed20..a06cd529a8df68 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java @@ -119,7 +119,7 @@ public void testChangingTabsNotifiesTabObserver() { mCoordinator.addTab(mMockTab); verify(mMockTabListObserver).onItemRangeInserted(mModel.getTabList(), 0, 1); assertThat(mModel.getTabList().getItemCount(), is(1)); - assertThat(mModel.getTabList().get(0), is(equalTo(mMockTab))); + assertThat(mModel.getTabList().get(0), is(mMockTab)); // Calling hide on the coordinator should make model propagate that it's invisible. mCoordinator.removeTab(mMockTab); diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java index db7001d7a699c2..2a8035674e7e78 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java @@ -38,36 +38,43 @@ public class ManualFillingControllerTest { private KeyboardAccessoryView mMockView; @Mock private ListObservable.ListObserver mMockTabListObserver; + @Mock + private KeyboardAccessoryData.Tab mMockTab; - private KeyboardAccessoryCoordinator mKeyboardAccessory; - private ManualFillingController mController; + private ManualFillingCoordinator mController; @Before public void setUp() { MockitoAnnotations.initMocks(this); when(mMockViewStub.inflate()).thenReturn(mMockView); - mKeyboardAccessory = new KeyboardAccessoryCoordinator(mMockWindow, mMockViewStub); - mController = new ManualFillingController(mKeyboardAccessory); + mController = new ManualFillingCoordinator(mMockWindow, mMockViewStub, mMockViewStub); } @Test @SmallTest public void testCreatesValidSubComponents() { - assertThat(mKeyboardAccessory, is(notNullValue())); assertThat(mController, is(notNullValue())); + assertThat(mController.getKeyboardAccessory(), is(notNullValue())); + assertThat(mController.getAccessorySheetForTesting(), is(notNullValue())); } @Test @SmallTest - public void testAddingBottomSheetsAddsTabsToAccessory() { + public void testAddingNewTabIsAddedToAccessoryAndSheet() { KeyboardAccessoryModel keyboardAccessoryModel = - mKeyboardAccessory.getMediatorForTesting().getModelForTesting(); + mController.getKeyboardAccessory().getMediatorForTesting().getModelForTesting(); keyboardAccessoryModel.addTabListObserver(mMockTabListObserver); + AccessorySheetModel accessorySheetModel = mController.getAccessorySheetForTesting() + .getMediatorForTesting() + .getModelForTesting(); + accessorySheetModel.getTabList().addObserver(mMockTabListObserver); assertThat(keyboardAccessoryModel.getTabList().getItemCount(), is(0)); - mController.addAccessorySheet(new AccessorySheetCoordinator(mMockViewStub)); + mController.addTab(mMockTab); verify(mMockTabListObserver).onItemRangeInserted(keyboardAccessoryModel.getTabList(), 0, 1); + verify(mMockTabListObserver).onItemRangeInserted(accessorySheetModel.getTabList(), 0, 1); assertThat(keyboardAccessoryModel.getTabList().getItemCount(), is(1)); + assertThat(accessorySheetModel.getTabList().getItemCount(), is(1)); } } \ No newline at end of file diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/modelutil/OWNERS b/chrome/android/junit/src/org/chromium/chrome/browser/modelutil/OWNERS new file mode 100644 index 00000000000000..17a573ea422c7a --- /dev/null +++ b/chrome/android/junit/src/org/chromium/chrome/browser/modelutil/OWNERS @@ -0,0 +1 @@ +file://chrome/android/java/src/org/chromium/chrome/browser/modelutil/OWNERS diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/modelutil/SimpleListObservableTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/modelutil/SimpleListObservableTest.java new file mode 100644 index 00000000000000..781a98c07b0d09 --- /dev/null +++ b/chrome/android/junit/src/org/chromium/chrome/browser/modelutil/SimpleListObservableTest.java @@ -0,0 +1,101 @@ +// 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.modelutil; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.verify; + +import android.support.test.filters.SmallTest; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.annotation.Config; + +import org.chromium.base.test.BaseRobolectricTestRunner; +import org.chromium.base.test.util.Feature; + +/** + * Basic test ensuring the {@link SimpleListObservable} notifies listeners properly. + */ +@RunWith(BaseRobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class SimpleListObservableTest { + @Mock + private ListObservable.ListObserver mObserver; + + private SimpleListObservable mIntegerList = new SimpleListObservable<>(); + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mIntegerList.addObserver(mObserver); + } + + @After + public void tearDown() { + mIntegerList.removeObserver(mObserver); + } + + @Test + @SmallTest + @Feature({"keyboard-accessory"}) + public void testNotifiesSuccessfulInsertions() { + // Setting an empty list ist always an insertion. + assertThat(mIntegerList.getItemCount(), is(0)); + mIntegerList.set(new Integer[] {333, 88888888, 22}); + verify(mObserver).onItemRangeInserted(mIntegerList, 0, 3); + assertThat(mIntegerList.getItemCount(), is(3)); + assertThat(mIntegerList.get(1), is(88888888)); + + // Adding Items is always an insertion. + mIntegerList.add(55555); + verify(mObserver).onItemRangeInserted(mIntegerList, 3, 1); + assertThat(mIntegerList.getItemCount(), is(4)); + assertThat(mIntegerList.get(3), is(55555)); + } + + @Test + @SmallTest + @Feature({"keyboard-accessory"}) + public void testModelNotifiesSuccessfulRemoval() { + Integer eightEights = 88888888; + mIntegerList.set(new Integer[] {333, eightEights, 22}); + assertThat(mIntegerList.getItemCount(), is(3)); + + // Removing any item by instance is always a removal. + mIntegerList.remove(eightEights); + verify(mObserver).onItemRangeRemoved(mIntegerList, 1, 1); + + // Setting an empty list is a removal of all items. + mIntegerList.set(new Integer[] {}); + verify(mObserver).onItemRangeRemoved(mIntegerList, 0, 2); + } + + @Test + @SmallTest + @Feature({"keyboard-accessory"}) + public void testModelNotifiesReplacedData() { + // The initial setting is an insertion. + mIntegerList.set(new Integer[] {333, 88888888, 22}); + verify(mObserver).onItemRangeInserted(mIntegerList, 0, 3); + + // Setting non-empty data is a change of all elements. + mIntegerList.set(new Integer[] {4444, 22}); + verify(mObserver).onItemRangeChanged(mIntegerList, 0, 3, mIntegerList); + + // Setting data of same values is a change. + mIntegerList.set(new Integer[] {4444, 22, 1, 666666}); + verify(mObserver).onItemRangeChanged(mIntegerList, 0, 4, mIntegerList); + + // Setting empty data is a removal. + mIntegerList.set(new Integer[] {}); + verify(mObserver).onItemRangeRemoved(mIntegerList, 0, 4); + } +} \ No newline at end of file