Merge "Consider entity length when deciding whether to show actions" into tm-qpr-dev

packages/SystemUI/src/com/android/systemui/clipboardoverlay/ClipboardOverlayController.java

@@ -363,15 +363,15 @@ public class ClipboardOverlayController implements ClipboardListener.ClipboardOv
 
     private void classifyText(ClipboardModel model) {
         mBgExecutor.execute(() -> {
-            Optional<RemoteAction> remoteAction = mClipboardUtils.getAction(
+            Optional<RemoteAction> remoteAction =
-                            model.getText(), model.getTextLinks(), model.getSource());
+                    mClipboardUtils.getAction(model.getTextLinks(), model.getSource());
             if (model.equals(mClipboardModel)) {
                 remoteAction.ifPresent(action -> {
                     mClipboardLogger.logUnguarded(CLIPBOARD_OVERLAY_ACTION_SHOWN);
-                    mView.setActionChip(action, () -> {
+                    mView.post(() -> mView.setActionChip(action, () -> {
                         mClipboardLogger.logSessionComplete(CLIPBOARD_OVERLAY_ACTION_TAPPED);
                         animateOut();
-                    });
+                    }));
                 });
             }
         });

packages/SystemUI/src/com/android/systemui/clipboardoverlay/ClipboardOverlayUtils.java

@@ -39,6 +39,9 @@ import javax.inject.Inject;
 
 class ClipboardOverlayUtils {
 
+    // minimum proportion of entire text an entity must take up, to be considered for smart actions
+    private static final float MINIMUM_ENTITY_PROPORTION = .8f;
+
     private final TextClassifier mTextClassifier;
 
     @Inject
@@ -65,20 +68,24 @@ class ClipboardOverlayUtils {
         return false;
     }
 
-    public Optional<RemoteAction> getAction(CharSequence text, TextLinks textLinks, String source) {
+    public Optional<RemoteAction> getAction(TextLinks textLinks, String source) {
-        return getActions(text, textLinks).stream().filter(remoteAction -> {
+        return getActions(textLinks).stream().filter(remoteAction -> {
             ComponentName component = remoteAction.getActionIntent().getIntent().getComponent();
             return component != null && !TextUtils.equals(source, component.getPackageName());
         }).findFirst();
     }
 
-    private ArrayList<RemoteAction> getActions(CharSequence text, TextLinks textLinks) {
+    private ArrayList<RemoteAction> getActions(TextLinks textLinks) {
         ArrayList<RemoteAction> actions = new ArrayList<>();
         for (TextLinks.TextLink link : textLinks.getLinks()) {
+            // skip classification for incidental entities
+            if (link.getEnd() - link.getStart()
+                    >= textLinks.getText().length() * MINIMUM_ENTITY_PROPORTION) {
                 TextClassification classification = mTextClassifier.classifyText(
-                    text, link.getStart(), link.getEnd(), null);
+                        textLinks.getText(), link.getStart(), link.getEnd(), null);
                 actions.addAll(classification.getActions());
             }
+        }
         return actions;
     }
 
@@ -92,10 +99,14 @@ class ClipboardOverlayUtils {
     private ArrayList<RemoteAction> getActions(ClipData.Item item) {
         ArrayList<RemoteAction> actions = new ArrayList<>();
         for (TextLinks.TextLink link : item.getTextLinks().getLinks()) {
+            // skip classification for incidental entities
+            if (link.getEnd() - link.getStart()
+                    >= item.getText().length() * MINIMUM_ENTITY_PROPORTION) {
                 TextClassification classification = mTextClassifier.classifyText(
                         item.getText(), link.getStart(), link.getEnd(), null);
                 actions.addAll(classification.getActions());
             }
+        }
         return actions;
     }
 }

packages/SystemUI/tests/src/com/android/systemui/clipboardoverlay/ClipboardOverlayControllerTest.java

@@ -446,7 +446,7 @@ public class ClipboardOverlayControllerTest extends SysuiTestCase {
         mFeatureFlags.set(CLIPBOARD_REMOTE_BEHAVIOR, true);
         when(mClipboardUtils.isRemoteCopy(any(Context.class), any(ClipData.class), anyString()))
                 .thenReturn(true);
-        when(mClipboardUtils.getAction(any(CharSequence.class), any(TextLinks.class), anyString()))
+        when(mClipboardUtils.getAction(any(TextLinks.class), anyString()))
                 .thenReturn(Optional.of(Mockito.mock(RemoteAction.class)));
         when(mClipboardOverlayView.post(any(Runnable.class))).thenAnswer(new Answer<Object>() {
             @Override

packages/SystemUI/tests/src/com/android/systemui/clipboardoverlay/ClipboardOverlayUtilsTest.java

@@ -21,6 +21,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.when;
 
@@ -77,6 +78,74 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
 
     @Test
     public void test_getAction_noLinks_returnsEmptyOptional() {
+        Optional<RemoteAction> action =
+                mClipboardUtils.getAction(Mockito.mock(TextLinks.class), "abc");
+
+        assertTrue(action.isEmpty());
+    }
+
+    @Test
+    public void test_getAction_returnsFirstLink() {
+        TextLinks links = getFakeTextLinksBuilder().build();
+        RemoteAction actionA = constructRemoteAction("abc");
+        RemoteAction actionB = constructRemoteAction("def");
+        TextClassification classificationA = Mockito.mock(TextClassification.class);
+        when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
+        TextClassification classificationB = Mockito.mock(TextClassification.class);
+        when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
+        when(mTextClassifier.classifyText(anyString(), anyInt(), anyInt(), isNull())).thenReturn(
+                classificationA, classificationB);
+
+        RemoteAction result = mClipboardUtils.getAction(links, "test").orElse(null);
+
+        assertEquals(actionA, result);
+    }
+
+    @Test
+    public void test_getAction_skipsMatchingComponent() {
+        TextLinks links = getFakeTextLinksBuilder().build();
+        RemoteAction actionA = constructRemoteAction("abc");
+        RemoteAction actionB = constructRemoteAction("def");
+        TextClassification classificationA = Mockito.mock(TextClassification.class);
+        when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
+        TextClassification classificationB = Mockito.mock(TextClassification.class);
+        when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
+        when(mTextClassifier.classifyText(anyString(), anyInt(), anyInt(), isNull())).thenReturn(
+                classificationA, classificationB);
+
+        RemoteAction result = mClipboardUtils.getAction(links, "abc").orElse(null);
+
+        assertEquals(actionB, result);
+    }
+
+    @Test
+    public void test_getAction_skipsShortEntity() {
+        TextLinks.Builder textLinks = new TextLinks.Builder("test text of length 22");
+        final Map<String, Float> scores = new ArrayMap<>();
+        scores.put(TextClassifier.TYPE_EMAIL, 1f);
+        textLinks.addLink(20, 22, scores);
+        textLinks.addLink(0, 22, scores);
+
+        RemoteAction actionA = constructRemoteAction("abc");
+        RemoteAction actionB = constructRemoteAction("def");
+        TextClassification classificationA = Mockito.mock(TextClassification.class);
+        when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
+        TextClassification classificationB = Mockito.mock(TextClassification.class);
+        when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
+        when(mTextClassifier.classifyText(anyString(), eq(20), eq(22), isNull())).thenReturn(
+                classificationA);
+        when(mTextClassifier.classifyText(anyString(), eq(0), eq(22), isNull())).thenReturn(
+                classificationB);
+
+        RemoteAction result = mClipboardUtils.getAction(textLinks.build(), "test").orElse(null);
+
+        assertEquals(actionB, result);
+    }
+
+    // TODO(b/267162944): Next four tests (marked "legacy") are obsolete once
+    //  CLIPBOARD_MINIMIZED_LAYOUT flag is released and removed
+    @Test
+    public void test_getAction_noLinks_returnsEmptyOptional_legacy() {
         ClipData.Item item = new ClipData.Item("no text links");
         item.setTextLinks(Mockito.mock(TextLinks.class));
 
@@ -86,8 +155,8 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
     }
 
     @Test
-    public void test_getAction_returnsFirstLink() {
+    public void test_getAction_returnsFirstLink_legacy() {
-        when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinks());
+        when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinksBuilder().build());
         when(mClipDataItem.getText()).thenReturn("");
         RemoteAction actionA = constructRemoteAction("abc");
         RemoteAction actionB = constructRemoteAction("def");
@@ -98,14 +167,14 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
         when(mTextClassifier.classifyText(anyString(), anyInt(), anyInt(), isNull())).thenReturn(
                 classificationA, classificationB);
 
-        RemoteAction result = mClipboardUtils.getAction(mClipDataItem, "def").orElse(null);
+        RemoteAction result = mClipboardUtils.getAction(mClipDataItem, "test").orElse(null);
 
         assertEquals(actionA, result);
     }
 
     @Test
-    public void test_getAction_skipsMatchingComponent() {
+    public void test_getAction_skipsMatchingComponent_legacy() {
-        when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinks());
+        when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinksBuilder().build());
         when(mClipDataItem.getText()).thenReturn("");
         RemoteAction actionA = constructRemoteAction("abc");
         RemoteAction actionB = constructRemoteAction("def");
@@ -121,6 +190,33 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
         assertEquals(actionB, result);
     }
 
+    @Test
+    public void test_getAction_skipsShortEntity_legacy() {
+        TextLinks.Builder textLinks = new TextLinks.Builder("test text of length 22");
+        final Map<String, Float> scores = new ArrayMap<>();
+        scores.put(TextClassifier.TYPE_EMAIL, 1f);
+        textLinks.addLink(20, 22, scores);
+        textLinks.addLink(0, 22, scores);
+
+        when(mClipDataItem.getTextLinks()).thenReturn(textLinks.build());
+        when(mClipDataItem.getText()).thenReturn(textLinks.build().getText());
+
+        RemoteAction actionA = constructRemoteAction("abc");
+        RemoteAction actionB = constructRemoteAction("def");
+        TextClassification classificationA = Mockito.mock(TextClassification.class);
+        when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
+        TextClassification classificationB = Mockito.mock(TextClassification.class);
+        when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
+        when(mTextClassifier.classifyText(anyString(), eq(20), eq(22), isNull())).thenReturn(
+                classificationA);
+        when(mTextClassifier.classifyText(anyString(), eq(0), eq(22), isNull())).thenReturn(
+                classificationB);
+
+        RemoteAction result = mClipboardUtils.getAction(mClipDataItem, "test").orElse(null);
+
+        assertEquals(actionB, result);
+    }
+
     @Test
     public void test_extra_withPackage_returnsTrue() {
         PersistableBundle b = new PersistableBundle();
@@ -184,12 +280,12 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
         return action;
     }
 
-    private static TextLinks getFakeTextLinks() {
+    private static TextLinks.Builder getFakeTextLinksBuilder() {
-        TextLinks.Builder textLinks = new TextLinks.Builder("test");
+        TextLinks.Builder textLinks = new TextLinks.Builder("test text of length 22");
         final Map<String, Float> scores = new ArrayMap<>();
         scores.put(TextClassifier.TYPE_EMAIL, 1f);
-        textLinks.addLink(0, 0, scores);
+        textLinks.addLink(0, 22, scores);
-        textLinks.addLink(0, 0, scores);
+        textLinks.addLink(0, 22, scores);
-        return textLinks.build();
+        return textLinks;
     }
 }