commit f2e3808991751107666808627b88cb03b85d616f
author Marcus Hagerott <mhagerott@google.com> Wed Nov 16 15:47:20 2016 -0800
committer Marcus Hagerott <mhagerott@google.com> Wed Nov 16 16:04:12 2016 -0800
tree 9faa9df59816ae9172521d21776690bb2757aaf9
parent e649318166cdd0d701294c1d7b5e5995e0ed6172

Fix NPE when creating dynamic shortcuts

An NPE was being thrown if a strequent contact had a null display_name

test
* ran GoogleContactsTests
* manually verified that dynamic shortcuts still update correctly

Bug 32945535

Change-Id: I5fbf28c26930abe0f50035e6c7830f94a05e2612

src/com/android/contacts/DynamicShortcuts.java

diff --git a/src/com/android/contacts/DynamicShortcuts.java b/src/com/android/contacts/DynamicShortcuts.java
index 52307e4..49cc722 100644
--- a/src/com/android/contacts/DynamicShortcuts.java
+++ b/src/com/android/contacts/DynamicShortcuts.java
@@ -215,9 +215,14 @@
         final List<ShortcutInfo> result = new ArrayList<>();
 
         try {
-            // For some reason the limit query parameter is ignored for the strequent content uri
-            for (int i = 0; i < MAX_SHORTCUTS && cursor.moveToNext(); i++) {
-                result.add(createShortcutFromRow(cursor));
+            int i = 0;
+            while (i < MAX_SHORTCUTS && cursor.moveToNext()) {
+                final ShortcutInfo shortcut = createShortcutFromRow(cursor);
+                if (shortcut == null) {
+                    continue;
+                }
+                result.add(shortcut);
+                i++;
             }
         } finally {
             cursor.close();
@@ -229,6 +234,9 @@
     @VisibleForTesting
     ShortcutInfo createShortcutFromRow(Cursor cursor) {
         final ShortcutInfo.Builder builder = builderForContactShortcut(cursor);
+        if (builder == null) {
+            return null;
+        }
         addIconForContact(cursor, builder);
         return builder.build();
     }
@@ -243,6 +251,9 @@
 
     @VisibleForTesting
     ShortcutInfo.Builder builderForContactShortcut(long id, String lookupKey, String displayName) {
+        if (lookupKey == null || displayName == null) {
+            return null;
+        }
         final PersistableBundle extras = new PersistableBundle();
         extras.putLong(Contacts._ID, id);
 
@@ -392,8 +403,6 @@
                         JobInfo.TriggerContentUri.FLAG_NOTIFY_FOR_DESCENDANTS))
                 .setTriggerContentUpdateDelay(mContentChangeMinUpdateDelay)
                 .setTriggerContentMaxDelay(mContentChangeMaxUpdateDelay)
-                // Minimize impact on UX by waiting for idle before updating.
-                .setRequiresDeviceIdle(true)
                 .build();
         mJobScheduler.schedule(job);
     }

tests/src/com/android/contacts/DynamicShortcutsTests.java

diff --git a/tests/src/com/android/contacts/DynamicShortcutsTests.java b/tests/src/com/android/contacts/DynamicShortcutsTests.java
index 106b72a..34cd8c4 100644
--- a/tests/src/com/android/contacts/DynamicShortcutsTests.java
+++ b/tests/src/com/android/contacts/DynamicShortcutsTests.java
@@ -100,6 +100,14 @@
         assertEquals(1l, shortcut.getExtras().getLong(Contacts._ID));
     }
 
+    public void test_builderForContactShortcut_returnsNullWhenNameIsNull() {
+        final DynamicShortcuts sut = createDynamicShortcuts();
+
+        final ShortcutInfo.Builder shortcut = sut.builderForContactShortcut(1l, "lookup_key", null);
+
+        assertNull(shortcut);
+    }
+
     public void test_builderForContactShortcut_ellipsizesLongNamesForLabels() {
         final DynamicShortcuts sut = createDynamicShortcuts();
         sut.setShortLabelMaxLength(5);
@@ -183,6 +191,43 @@
         assertThat(arg.get(2), isShortcutForContact(3l, "starred_2", "Starred Two"));
     }
 
+    public void test_refresh_skipsContactsWithNullName() {
+        final ShortcutManager mockShortcutManager = mock(ShortcutManager.class);
+        when(mockShortcutManager.getPinnedShortcuts()).thenReturn(
+                Collections.<ShortcutInfo>emptyList());
+        final DynamicShortcuts sut = createDynamicShortcuts(resolverWithExpectedQueries(
+                queryFor(Contacts.CONTENT_STREQUENT_URI,
+                        1l, "key1", "first",
+                        2l, "key2", "second",
+                        3l, "key3", null,
+                        4l, null, null,
+                        5l, "key5", "fifth",
+                        6l, "key6", "sixth")), mockShortcutManager);
+
+        sut.refresh();
+
+        final ArgumentCaptor<List<ShortcutInfo>> updateArgs =
+                ArgumentCaptor.forClass((Class) List.class);
+
+        verify(mockShortcutManager).setDynamicShortcuts(updateArgs.capture());
+
+        final List<ShortcutInfo> arg = updateArgs.getValue();
+        assertThat(arg.size(), equalTo(3));
+        assertThat(arg.get(0), isShortcutForContact(1l, "key1", "first"));
+        assertThat(arg.get(1), isShortcutForContact(2l, "key2", "second"));
+        assertThat(arg.get(2), isShortcutForContact(5l, "key5", "fifth"));
+
+
+        // Also verify that it doesn't crash if there are fewer than 3 valid strequent contacts
+        createDynamicShortcuts(resolverWithExpectedQueries(
+                queryFor(Contacts.CONTENT_STREQUENT_URI,
+                        1l, "key1", "first",
+                        2l, "key2", "second",
+                        3l, "key3", null,
+                        4l, null, null)), mock(ShortcutManager.class)).refresh();
+    }
+
+
     public void test_handleFlagDisabled_stopsJob() {
         final ShortcutManager mockShortcutManager = mock(ShortcutManager.class);
         final JobScheduler mockJobScheduler = mock(JobScheduler.class);