Commit 63b2822b authored by Tomasz Wiszkowski's avatar Tomasz Wiszkowski Committed by Commit Bot

Invert class relation for suggestion ViewBinders and Views.

This change replaces inheritance with generics and, in turn,
flips relation between Base- and Specialized ViewBinder classes.

While more verbose, this gives us a better control and protection
over how views and view binders are created and used.
With this change we get:
- better control over what individual viewbinders see,
- better compile-time diagnostics for accidental typos and copy-paste
  errors,
- better support for selective view substitution (enabling us to
  further optimize suggestions mechanism while staying safe).

For the most part this ensures Compiler will catch and report an
error early. In the extreme case where an error is repeated everywhere
the error will be caught and thrown at runtime upon view creation.

This change aligns with the original design proposal for suggestions
refactor.

More details: https://crrev.com/c/2007943
Doc: http://doc/1aL_UcW1gdeSLqNzuJZlQPBqMG0XAwVaoa05UTyVulp8
Bug: 982818

Change-Id: Ic8425512f3904cc29ea410723a9b7a717492e191
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2017980
Commit-Queue: Ender <ender@google.com>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735094}
parent fe6aca29
......@@ -26,6 +26,7 @@ import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteController.On
import org.chromium.chrome.browser.omnibox.suggestions.SuggestionListViewBinder.SuggestionListViewHolder;
import org.chromium.chrome.browser.omnibox.suggestions.answer.AnswerSuggestionViewBinder;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionView;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewBinder;
import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewViewBinder;
import org.chromium.chrome.browser.omnibox.suggestions.editurl.EditUrlSuggestionProcessor;
import org.chromium.chrome.browser.omnibox.suggestions.editurl.EditUrlSuggestionViewBinder;
......@@ -119,9 +120,9 @@ public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
// clang-format off
adapter.registerType(
OmniboxSuggestionUiType.DEFAULT,
() -> new BaseSuggestionView(mListView.getContext(),
() -> new BaseSuggestionView<View>(mListView.getContext(),
R.layout.omnibox_basic_suggestion),
new SuggestionViewViewBinder());
new BaseSuggestionViewBinder<View>(SuggestionViewViewBinder::bind));
adapter.registerType(
OmniboxSuggestionUiType.EDIT_URL_SUGGESTION,
......@@ -130,20 +131,22 @@ public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
adapter.registerType(
OmniboxSuggestionUiType.ANSWER_SUGGESTION,
() -> new BaseSuggestionView(mListView.getContext(),
() -> new BaseSuggestionView<View>(mListView.getContext(),
R.layout.omnibox_answer_suggestion),
new AnswerSuggestionViewBinder());
new BaseSuggestionViewBinder<View>(AnswerSuggestionViewBinder::bind));
adapter.registerType(
OmniboxSuggestionUiType.ENTITY_SUGGESTION,
() -> new BaseSuggestionView(mListView.getContext(),
() -> new BaseSuggestionView<View>(mListView.getContext(),
R.layout.omnibox_entity_suggestion),
new EntitySuggestionViewBinder());
new BaseSuggestionViewBinder<View>(EntitySuggestionViewBinder::bind));
adapter.registerType(
OmniboxSuggestionUiType.TAIL_SUGGESTION,
() -> new BaseSuggestionView(new TailSuggestionView(mListView.getContext())),
new TailSuggestionViewBinder());
() -> new BaseSuggestionView<TailSuggestionView>(
new TailSuggestionView(mListView.getContext())),
new BaseSuggestionViewBinder<TailSuggestionView>(
TailSuggestionViewBinder::bind));
// clang-format on
mHolder = new SuggestionListViewHolder(container, list);
......
......@@ -4,42 +4,38 @@
package org.chromium.chrome.browser.omnibox.suggestions.answer;
import android.view.View;
import android.widget.TextView;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionView;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewBinder;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
/** A mechanism binding AnswerSuggestion properties to its view. */
public class AnswerSuggestionViewBinder extends BaseSuggestionViewBinder {
public class AnswerSuggestionViewBinder {
/** @see PropertyModelChangeProcessor.ViewBinder#bind(Object, Object, Object) */
@Override
public void bind(PropertyModel model, BaseSuggestionView view, PropertyKey propertyKey) {
super.bind(model, view, propertyKey);
public static void bind(PropertyModel model, View view, PropertyKey propertyKey) {
if (AnswerSuggestionViewProperties.TEXT_LINE_1_TEXT == propertyKey) {
TextView tv = view.findContentView(R.id.omnibox_answer_line_1);
TextView tv = view.findViewById(R.id.omnibox_answer_line_1);
tv.setText(model.get(AnswerSuggestionViewProperties.TEXT_LINE_1_TEXT));
} else if (AnswerSuggestionViewProperties.TEXT_LINE_2_TEXT == propertyKey) {
TextView tv = view.findContentView(R.id.omnibox_answer_line_2);
TextView tv = view.findViewById(R.id.omnibox_answer_line_2);
tv.setText(model.get(AnswerSuggestionViewProperties.TEXT_LINE_2_TEXT));
} else if (AnswerSuggestionViewProperties.TEXT_LINE_1_ACCESSIBILITY_DESCRIPTION
== propertyKey) {
TextView tv = view.findContentView(R.id.omnibox_answer_line_1);
TextView tv = view.findViewById(R.id.omnibox_answer_line_1);
tv.setContentDescription(model.get(
AnswerSuggestionViewProperties.TEXT_LINE_1_ACCESSIBILITY_DESCRIPTION));
} else if (AnswerSuggestionViewProperties.TEXT_LINE_2_ACCESSIBILITY_DESCRIPTION
== propertyKey) {
TextView tv = view.findContentView(R.id.omnibox_answer_line_2);
TextView tv = view.findViewById(R.id.omnibox_answer_line_2);
tv.setContentDescription(model.get(
AnswerSuggestionViewProperties.TEXT_LINE_2_ACCESSIBILITY_DESCRIPTION));
} else if (AnswerSuggestionViewProperties.TEXT_LINE_1_MAX_LINES == propertyKey) {
TextView tv = view.findContentView(R.id.omnibox_answer_line_1);
TextView tv = view.findViewById(R.id.omnibox_answer_line_1);
tv.setMaxLines(model.get(AnswerSuggestionViewProperties.TEXT_LINE_1_MAX_LINES));
} else if (AnswerSuggestionViewProperties.TEXT_LINE_2_MAX_LINES == propertyKey) {
TextView tv = view.findContentView(R.id.omnibox_answer_line_2);
TextView tv = view.findViewById(R.id.omnibox_answer_line_2);
tv.setMaxLines(model.get(AnswerSuggestionViewProperties.TEXT_LINE_2_MAX_LINES));
}
}
......
......@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.omnibox.suggestions.base;
import android.content.Context;
import android.support.annotation.IdRes;
import android.support.v7.widget.AppCompatImageView;
import android.util.TypedValue;
import android.view.KeyEvent;
......@@ -26,9 +25,9 @@ import org.chromium.components.browser_ui.widget.RoundedCornerImageView;
* Base layout for common suggestion types. Includes support for a configurable suggestion content
* and the common suggestion patterns shared across suggestion formats.
*/
public class BaseSuggestionView extends SimpleHorizontalLayoutView {
public class BaseSuggestionView<T extends View> extends SimpleHorizontalLayoutView {
protected final ImageView mActionView;
protected final DecoratedSuggestionView mDecoratedView;
protected final DecoratedSuggestionView<T> mDecoratedView;
private SuggestionViewDelegate mDelegate;
......@@ -37,7 +36,7 @@ public class BaseSuggestionView extends SimpleHorizontalLayoutView {
*
* @param context The context used to construct the suggestion view.
*/
public BaseSuggestionView(View view) {
public BaseSuggestionView(T view) {
super(view.getContext());
TypedValue themeRes = new TypedValue();
......@@ -79,7 +78,7 @@ public class BaseSuggestionView extends SimpleHorizontalLayoutView {
* @param layoutId Layout ID to be inflated as the contents view.
*/
public BaseSuggestionView(Context context, @LayoutRes int layoutId) {
this(LayoutInflater.from(context).inflate(layoutId, null));
this((T) LayoutInflater.from(context).inflate(layoutId, null));
}
@Override
......@@ -116,12 +115,12 @@ public class BaseSuggestionView extends SimpleHorizontalLayoutView {
*
* @param view View to be displayed as suggestion content.
*/
void setContentView(View view) {
void setContentView(T view) {
mDecoratedView.setContentView(view);
}
/** @return Embedded suggestion content view. */
public View getContentView() {
public T getContentView() {
return mDecoratedView.getContentView();
}
......@@ -148,17 +147,4 @@ public class BaseSuggestionView extends SimpleHorizontalLayoutView {
ImageView getActionImageView() {
return mActionView;
}
/**
* Find content view by view id.
*
* Scoped {@link #findViewById(int)} search for the view specified in
* {@link #setContentView(View)}.
*
* @param id View ID of the sought view.
* @return View with the specified ID or null, if view could not be found.
*/
public <T extends View> T findContentView(@IdRes int id) {
return mDecoratedView.findContentView(id);
}
}
......@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.omnibox.suggestions.base;
import android.content.res.ColorStateList;
import android.content.res.Resources;
import android.support.annotation.CallSuper;
import android.support.annotation.ColorRes;
import android.support.v4.view.ViewCompat;
import android.support.v7.content.res.AppCompatResources;
......@@ -24,12 +23,22 @@ import org.chromium.ui.modelutil.PropertyModelChangeProcessor.ViewBinder;
/**
* Binds base suggestion view properties.
*
* This binder should be used by all suggestions that also utilize BaseSuggestionView<T> to
* construct the view, and manages shared suggestion properties (such as decorations or theme).
*/
public class BaseSuggestionViewBinder
implements ViewBinder<PropertyModel, BaseSuggestionView, PropertyKey> {
public final class BaseSuggestionViewBinder<T extends View>
implements ViewBinder<PropertyModel, BaseSuggestionView<T>, PropertyKey> {
private final ViewBinder<PropertyModel, T, PropertyKey> mContentBinder;
public BaseSuggestionViewBinder(ViewBinder<PropertyModel, T, PropertyKey> contentBinder) {
mContentBinder = contentBinder;
}
@Override
@CallSuper
public void bind(PropertyModel model, BaseSuggestionView view, PropertyKey propertyKey) {
public void bind(PropertyModel model, BaseSuggestionView<T> view, PropertyKey propertyKey) {
mContentBinder.bind(model, view.getContentView(), propertyKey);
if (BaseSuggestionViewProperties.SUGGESTION_DELEGATE == propertyKey) {
view.setDelegate(model.get(BaseSuggestionViewProperties.SUGGESTION_DELEGATE));
updateContentViewPadding(model, view.getDecoratedSuggestionView());
......
......@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.omnibox.suggestions.base;
import android.content.Context;
import android.support.annotation.DrawableRes;
import android.support.annotation.IdRes;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
......@@ -17,9 +16,9 @@ import org.chromium.components.browser_ui.widget.RoundedCornerImageView;
/**
* Container view for omnibox suggestions supplying icon decoration.
*/
class DecoratedSuggestionView extends SimpleHorizontalLayoutView {
class DecoratedSuggestionView<T extends View> extends SimpleHorizontalLayoutView {
private final RoundedCornerImageView mSuggestionIcon;
private View mContentView;
private T mContentView;
/**
* Constructs a new suggestion view.
......@@ -44,7 +43,7 @@ class DecoratedSuggestionView extends SimpleHorizontalLayoutView {
}
/** Specify content view (suggestion body). */
void setContentView(View view) {
void setContentView(T view) {
if (mContentView != null) removeView(view);
mContentView = view;
mContentView.setLayoutParams(LayoutParams.forDynamicView());
......@@ -52,7 +51,7 @@ class DecoratedSuggestionView extends SimpleHorizontalLayoutView {
}
/** @return Embedded suggestion content view. */
View getContentView() {
T getContentView() {
return mContentView;
}
......@@ -65,11 +64,4 @@ class DecoratedSuggestionView extends SimpleHorizontalLayoutView {
public boolean isFocused() {
return super.isFocused() || (isSelected() && !isInTouchMode());
}
<ViewType extends View> ViewType findContentView(@IdRes int id) {
if (mContentView == null) {
return null;
}
return mContentView.findViewById(id);
}
}
......@@ -12,21 +12,16 @@ import android.widget.TextView;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.omnibox.suggestions.SuggestionCommonProperties;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionView;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewBinder;
import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionSpannable;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
/** Properties associated with the basic suggestion view. */
public class SuggestionViewViewBinder extends BaseSuggestionViewBinder {
/** @see BaseSuggestionViewBinder#bind(PropertyModel, BaseSuggestionView, PropertyKey) */
@Override
public void bind(PropertyModel model, BaseSuggestionView view, PropertyKey propertyKey) {
super.bind(model, view, propertyKey);
public class SuggestionViewViewBinder {
/** @see PropertyModelChangeProcessor.ViewBinder#bind(Object, Object, Object) */
public static void bind(PropertyModel model, View view, PropertyKey propertyKey) {
if (propertyKey == SuggestionViewProperties.TEXT_LINE_1_TEXT) {
TextView tv = view.findContentView(R.id.line_1);
TextView tv = view.findViewById(R.id.line_1);
tv.setText(model.get(SuggestionViewProperties.TEXT_LINE_1_TEXT));
} else if (propertyKey == SuggestionCommonProperties.USE_DARK_COLORS) {
updateSuggestionTextColor(view, model);
......@@ -35,11 +30,11 @@ public class SuggestionViewViewBinder extends BaseSuggestionViewBinder {
// https://crbug.com/609680: ensure URLs are always composed LTR and that their
// components are not re-ordered.
final boolean isSearch = model.get(SuggestionViewProperties.IS_SEARCH_SUGGESTION);
final TextView tv = view.findContentView(R.id.line_2);
final TextView tv = view.findViewById(R.id.line_2);
tv.setTextDirection(
isSearch ? TextView.TEXT_DIRECTION_INHERIT : TextView.TEXT_DIRECTION_LTR);
} else if (propertyKey == SuggestionViewProperties.TEXT_LINE_2_TEXT) {
TextView tv = view.findContentView(R.id.line_2);
TextView tv = view.findViewById(R.id.line_2);
final SuggestionSpannable span = model.get(SuggestionViewProperties.TEXT_LINE_2_TEXT);
if (!TextUtils.isEmpty(span)) {
tv.setText(span);
......@@ -50,11 +45,11 @@ public class SuggestionViewViewBinder extends BaseSuggestionViewBinder {
}
}
private static void updateSuggestionTextColor(BaseSuggestionView view, PropertyModel model) {
private static void updateSuggestionTextColor(View view, PropertyModel model) {
final boolean isSearch = model.get(SuggestionViewProperties.IS_SEARCH_SUGGESTION);
final boolean useDarkMode = model.get(SuggestionCommonProperties.USE_DARK_COLORS);
final TextView line1 = view.findContentView(R.id.line_1);
final TextView line2 = view.findContentView(R.id.line_2);
final TextView line1 = view.findViewById(R.id.line_1);
final TextView line2 = view.findViewById(R.id.line_2);
@ColorRes
final int color1 =
......
......@@ -9,23 +9,18 @@ import android.view.View;
import android.widget.TextView;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionView;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewBinder;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
/** A mechanism binding EntitySuggestion properties to its view. */
public class EntitySuggestionViewBinder extends BaseSuggestionViewBinder {
/** @see BaseSuggestionViewBinder#bind(PropertyModel, BaseSuggestionView, PropertyKey) */
@Override
public void bind(PropertyModel model, BaseSuggestionView view, PropertyKey propertyKey) {
super.bind(model, view, propertyKey);
public class EntitySuggestionViewBinder {
/** @see PropertyModelChangeProcessor.ViewBinder#bind(Object, Object, Object) */
public static void bind(PropertyModel model, View view, PropertyKey propertyKey) {
if (EntitySuggestionViewProperties.SUBJECT_TEXT == propertyKey) {
final TextView tv = view.findContentView(R.id.entity_subject);
final TextView tv = view.findViewById(R.id.entity_subject);
tv.setText(model.get(EntitySuggestionViewProperties.SUBJECT_TEXT));
} else if (EntitySuggestionViewProperties.DESCRIPTION_TEXT == propertyKey) {
final TextView tv = view.findContentView(R.id.entity_description);
final TextView tv = view.findViewById(R.id.entity_description);
final String text = model.get(EntitySuggestionViewProperties.DESCRIPTION_TEXT);
if (TextUtils.isEmpty(text)) {
tv.setVisibility(View.GONE);
......
......@@ -9,31 +9,25 @@ import android.support.annotation.ColorRes;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.omnibox.suggestions.SuggestionCommonProperties;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionView;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewBinder;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
/** Properties associated with the tail suggestion view. */
public class TailSuggestionViewBinder extends BaseSuggestionViewBinder {
/** @see BaseSuggestionViewBinder#bind(PropertyModel, BaseSuggestionView, PropertyKey) */
@Override
public void bind(PropertyModel model, BaseSuggestionView view, PropertyKey propertyKey) {
super.bind(model, view, propertyKey);
final TailSuggestionView tailView = (TailSuggestionView) view.getContentView();
public class TailSuggestionViewBinder {
/** @see PropertyModelChangeProcessor.ViewBinder#bind(Object, Object, Object) */
public static void bind(PropertyModel model, TailSuggestionView view, PropertyKey propertyKey) {
if (TailSuggestionViewProperties.ALIGNMENT_MANAGER == propertyKey) {
tailView.setAlignmentManager(model.get(TailSuggestionViewProperties.ALIGNMENT_MANAGER));
view.setAlignmentManager(model.get(TailSuggestionViewProperties.ALIGNMENT_MANAGER));
} else if (propertyKey == TailSuggestionViewProperties.TEXT) {
tailView.setTailText(model.get(TailSuggestionViewProperties.TEXT));
view.setTailText(model.get(TailSuggestionViewProperties.TEXT));
} else if (propertyKey == TailSuggestionViewProperties.FILL_INTO_EDIT) {
tailView.setFullText(model.get(TailSuggestionViewProperties.FILL_INTO_EDIT));
view.setFullText(model.get(TailSuggestionViewProperties.FILL_INTO_EDIT));
} else if (propertyKey == SuggestionCommonProperties.USE_DARK_COLORS) {
final boolean useDarkMode = model.get(SuggestionCommonProperties.USE_DARK_COLORS);
@ColorRes
final int color = useDarkMode ? R.color.default_text_color_dark
: R.color.default_text_color_light;
tailView.setTextColor(ApiCompatibilityUtils.getColor(view.getResources(), color));
view.setTextColor(ApiCompatibilityUtils.getColor(view.getResources(), color));
}
}
}
......@@ -15,6 +15,7 @@ import android.content.res.Resources;
import android.view.View;
import android.widget.ImageView;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
......@@ -48,6 +49,9 @@ public class BaseSuggestionViewBinderUnitTest {
@Mock
ImageView mActionView;
@Mock
ImageView mContentView;
private Activity mActivity;
private Resources mResources;
private PropertyModel mModel;
......@@ -65,11 +69,14 @@ public class BaseSuggestionViewBinderUnitTest {
when(mBaseView.getDecoratedSuggestionView()).thenReturn(mDecoratedView);
when(mBaseView.getSuggestionImageView()).thenReturn(mIconView);
when(mBaseView.getActionImageView()).thenReturn(mActionView);
when(mDecoratedView.getContentView()).thenReturn(mContentView);
when(mBaseView.getContentView()).thenReturn(mContentView);
when(mDecoratedView.getResources()).thenReturn(mResources);
mModel = new PropertyModel(BaseSuggestionViewProperties.ALL_KEYS);
PropertyModelChangeProcessor.create(mModel, mBaseView, new BaseSuggestionViewBinder());
PropertyModelChangeProcessor.create(mModel, mBaseView,
new BaseSuggestionViewBinder(
(m, v, p) -> { Assert.assertEquals(mContentView, v); }));
}
@Test
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment