[TouchToFill][Android] Move url formatting to mediator

Non-functional change. This CL moves the URL formatting into the
mediator which separates the logic from the view binder.

Bug: 957532
Change-Id: I172f20f00ab3e9fa998c82bd8c41ef8159cf08d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857129
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706001}

chrome/browser/touch_to_fill/android/internal/BUILD.gn

@@ -13,6 +13,7 @@ android_library("java") {
     "//chrome/android:chrome_java",
     "//chrome/browser/touch_to_fill/android:public_java",
     "//chrome/browser/util/android:java",
+    "//components/url_formatter/android:url_formatter_java",
     "//third_party/android_deps:com_android_support_recyclerview_v7_java",
     "//ui/android:ui_java",
   ]

chrome/browser/touch_to_fill/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillMediator.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.touch_to_fill;
 
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.CREDENTIAL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FAVICON;
+import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FORMATTED_ORIGIN;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.ON_CLICK_LISTENER;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FORMATTED_URL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ORIGIN_SECURE;
@@ -16,6 +17,7 @@ import androidx.annotation.Px;
 
 import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties;
 import org.chromium.chrome.browser.touch_to_fill.data.Credential;
+import org.chromium.components.url_formatter.UrlFormatter;
 import org.chromium.ui.modelutil.ListModel;
 import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -52,6 +54,9 @@ class TouchToFillMediator implements TouchToFillProperties.ViewEventListener {
             PropertyModel propertyModel =
                     new PropertyModel.Builder(CredentialProperties.ALL_KEYS)
                             .with(CREDENTIAL, credential)
+                            .with(FORMATTED_ORIGIN,
+                                    UrlFormatter.formatUrlForDisplayOmitScheme(
+                                            credential.getOriginUrl()))
                             .with(ON_CLICK_LISTENER, this::onSelectedCredential)
                             .build();
             sheetItems.add(new ListItem(TouchToFillProperties.ItemType.CREDENTIAL, propertyModel));

chrome/browser/touch_to_fill/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillProperties.java

@@ -52,11 +52,14 @@ class TouchToFillProperties {
                 new PropertyModel.WritableObjectPropertyKey<>("favicon");
         static final PropertyModel.ReadableObjectPropertyKey<Credential> CREDENTIAL =
                 new PropertyModel.ReadableObjectPropertyKey<>("credential");
+        static final PropertyModel.ReadableObjectPropertyKey<String> FORMATTED_ORIGIN =
+                new PropertyModel.ReadableObjectPropertyKey<>("formatted_url");
         static final PropertyModel
                 .ReadableObjectPropertyKey<Callback<Credential>> ON_CLICK_LISTENER =
                 new PropertyModel.ReadableObjectPropertyKey<>("on_click_listener");
 
-        static final PropertyKey[] ALL_KEYS = {FAVICON, CREDENTIAL, ON_CLICK_LISTENER};
+        static final PropertyKey[] ALL_KEYS = {
+                FAVICON, CREDENTIAL, FORMATTED_ORIGIN, ON_CLICK_LISTENER};
 
         private CredentialProperties() {}
     }

chrome/browser/touch_to_fill/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillViewBinder.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.touch_to_fill;
 
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.CREDENTIAL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FAVICON;
+import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FORMATTED_ORIGIN;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.ON_CLICK_LISTENER;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FORMATTED_URL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ORIGIN_SECURE;
@@ -100,16 +101,19 @@ class TouchToFillViewBinder {
      */
     private static void bindCredentialView(
             PropertyModel model, ViewGroup view, PropertyKey propertyKey) {
+        Credential credential = model.get(CREDENTIAL);
         if (propertyKey == FAVICON) {
             ImageView imageView = view.findViewById(R.id.favicon);
             imageView.setImageBitmap(model.get(FAVICON));
         } else if (propertyKey == ON_CLICK_LISTENER) {
-            view.setOnClickListener(clickedView -> {
-                model.get(ON_CLICK_LISTENER).onResult(model.get(CREDENTIAL));
-            });
+            view.setOnClickListener(
+                    clickedView -> { model.get(ON_CLICK_LISTENER).onResult(credential); });
+        } else if (propertyKey == FORMATTED_ORIGIN) {
+            TextView pslOriginText = view.findViewById(R.id.credential_origin);
+            pslOriginText.setText(model.get(FORMATTED_ORIGIN));
+            pslOriginText.setVisibility(
+                    credential.isPublicSuffixMatch() ? View.VISIBLE : View.GONE);
         } else if (propertyKey == CREDENTIAL) {
-            Credential credential = model.get(CREDENTIAL);
-
             TextView pslOriginText = view.findViewById(R.id.credential_origin);
             String formattedOrigin = stripScheme(credential.getOriginUrl());
             formattedOrigin =

chrome/browser/touch_to_fill/android/javatests/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillViewTest.java

@@ -13,6 +13,7 @@ import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
 
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.CREDENTIAL;
+import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FORMATTED_ORIGIN;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.ON_CLICK_LISTENER;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FORMATTED_URL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ORIGIN_SECURE;
@@ -60,9 +61,9 @@ import java.util.Collections;
 public class TouchToFillViewTest {
     private static final Credential ANA = new Credential("Ana", "S3cr3t", "Ana", "", false);
     private static final Credential NO_ONE =
-            new Credential("", "***", "No Username", "http://m.example.xyz/", true);
+            new Credential("", "***", "No Username", "m.example.xyz", true);
     private static final Credential BOB =
-            new Credential("Bob", "***", "Bob", "http://mobile.example.xyz", true);
+            new Credential("Bob", "***", "Bob", "mobile.example.xyz", true);
 
     @Mock
     private TouchToFillProperties.ViewEventListener mMockListener;
@@ -219,6 +220,7 @@ public class TouchToFillViewTest {
                 new PropertyModel.Builder(TouchToFillProperties.CredentialProperties.ALL_KEYS)
                         .with(CREDENTIAL, credential)
                         .with(ON_CLICK_LISTENER, mCredentialCallback)
+                        .with(FORMATTED_ORIGIN, credential.getOriginUrl())
                         .build());
     }
 }

chrome/browser/touch_to_fill/android/junit/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillControllerTest.java

@@ -9,12 +9,14 @@ import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.CREDENTIAL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FAVICON;
+import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FORMATTED_ORIGIN;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.ON_CLICK_LISTENER;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FORMATTED_URL;
 import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ORIGIN_SECURE;
@@ -27,6 +29,7 @@ import android.graphics.Bitmap;
 import androidx.annotation.Px;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
@@ -36,8 +39,11 @@ import org.mockito.MockitoAnnotations;
 
 import org.chromium.base.Callback;
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ItemType;
 import org.chromium.chrome.browser.touch_to_fill.data.Credential;
+import org.chromium.components.url_formatter.UrlFormatter;
+import org.chromium.components.url_formatter.UrlFormatterJni;
 import org.chromium.ui.modelutil.ListModel;
 import org.chromium.ui.modelutil.MVCListAdapter;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -53,13 +59,19 @@ import java.util.Collections;
 public class TouchToFillControllerTest {
     private static final String TEST_URL = "www.example.xyz";
     private static final String TEST_SUBDOMAIN_URL = "subdomain.example.xyz";
-    private static final Credential ANA = new Credential("Ana", "S3cr3t", "Ana", TEST_URL, false);
+    private static final String TEST_MOBILE_URL = "www.example.xyz";
+    private static final Credential ANA =
+            new Credential("Ana", "S3cr3t", "Ana", "https://m.a.xyz/", true);
     private static final Credential BOB =
             new Credential("Bob", "*****", "Bob", TEST_SUBDOMAIN_URL, true);
     private static final Credential CARL =
             new Credential("Carl", "G3h3!m", "Carl", TEST_URL, false);
     private static final @Px int DESIRED_FAVICON_SIZE = 64;
 
+    @Rule
+    public JniMocker mJniMocker = new JniMocker();
+    @Mock
+    private UrlFormatter.Natives mUrlFormatterJniMock;
     @Mock
     private TouchToFillComponent.Delegate mMockDelegate;
 
@@ -72,6 +84,9 @@ public class TouchToFillControllerTest {
 
     public TouchToFillControllerTest() {
         MockitoAnnotations.initMocks(this);
+        mJniMocker.mock(UrlFormatterJni.TEST_HOOKS, mUrlFormatterJniMock);
+        when(mUrlFormatterJniMock.formatUrlForDisplayOmitScheme(anyString()))
+                .then(inv -> format(inv.getArgument(0)));
     }
 
     @Before
@@ -99,7 +114,6 @@ public class TouchToFillControllerTest {
     public void testShowCredentialsSetsCredentialListAndRequestsFavicons() {
         mMediator.showCredentials(TEST_URL, true, Arrays.asList(ANA, CARL, BOB));
         ListModel<MVCListAdapter.ListItem> credentialList = mModel.get(SHEET_ITEMS);
-        assertThat(credentialList.size(), is(3));
         // TODO(https://crbug.com/1013209): Simplify this after adding equals to ModelList.
         assertThat(credentialList.size(), is(3));
         assertThat(credentialList.get(0).type, is(ItemType.CREDENTIAL));
@@ -112,8 +126,9 @@ public class TouchToFillControllerTest {
         assertThat(credentialList.get(2).model.get(CREDENTIAL), is(BOB));
         assertThat(credentialList.get(2).model.get(FAVICON), is(nullValue()));
 
-        // ANA and CARL both have TEST_URL as their origin URL
-        verify(mMockDelegate, times(2)).fetchFavicon(eq(TEST_URL), eq(DESIRED_FAVICON_SIZE), any());
+        verify(mMockDelegate).fetchFavicon(eq("https://m.a.xyz/"), eq(DESIRED_FAVICON_SIZE), any());
+        verify(mMockDelegate).fetchFavicon(eq(TEST_URL), eq(DESIRED_FAVICON_SIZE), any());
+        verify(mMockDelegate).fetchFavicon(eq(TEST_SUBDOMAIN_URL), eq(DESIRED_FAVICON_SIZE), any());
         verify(mMockDelegate).fetchFavicon(eq(BOB.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
     }
 
@@ -138,6 +153,18 @@ public class TouchToFillControllerTest {
         assertThat(credentialList.get(0).model.get(FAVICON), is(bitmap));
     }
 
+    @Test
+    public void testShowCredentialsFormatPslOrigins() {
+        mMediator.showCredentials(TEST_URL, true, Arrays.asList(ANA, BOB));
+        assertThat(mModel.get(SHEET_ITEMS).size(), is(2));
+        assertThat(mModel.get(SHEET_ITEMS).get(0).type, is(ItemType.CREDENTIAL));
+        assertThat(mModel.get(SHEET_ITEMS).get(0).model.get(FORMATTED_ORIGIN),
+                is(format(ANA.getOriginUrl())));
+        assertThat(mModel.get(SHEET_ITEMS).get(1).type, is(ItemType.CREDENTIAL));
+        assertThat(mModel.get(SHEET_ITEMS).get(1).model.get(FORMATTED_ORIGIN),
+                is(format(BOB.getOriginUrl())));
+    }
+
     @Test
     public void testClearsCredentialListWhenShowingAgain() {
         mMediator.showCredentials(TEST_URL, true, Collections.singletonList(ANA));
@@ -182,4 +209,14 @@ public class TouchToFillControllerTest {
         verify(mMockDelegate).onDismissed();
         assertThat(mModel.get(VISIBLE), is(false));
     }
+
+    /**
+     * Helper to verify formatted URLs. The real implementation calls {@link UrlFormatter}. It's not
+     * useful to actually reimplement the formatter, so just modify the string in a trivial way.
+     * @param originUrl A URL {@link String} to "format".
+     * @return A "formatted" URL {@link String}.
+     */
+    private static String format(String originUrl) {
+        return "formatted_" + originUrl + "_formatted";
+    }
 }