Also ignore IllegalStateException thrown by SQLiteClosable when coalescing fails.

Test: Manual
PiperOrigin-RevId: 202499434
Change-Id: Ie41eeb782072d82c5613b44be99649f43807498d

2 files changed, 73 insertions, 30 deletions

diff --git a/java/com/android/dialer/calllog/database/Coalescer.java b/java/com/android/dialer/calllog/database/Coalescer.java
index 2ad9f9a97..fd751e767 100644
--- a/java/com/android/dialer/calllog/database/Coalescer.java
+++ b/java/com/android/dialer/calllog/database/Coalescer.java
@@ -16,6 +16,7 @@
 package com.android.dialer.calllog.database;
 
 import android.database.Cursor;
+import android.database.StaleDataException;
 import android.provider.CallLog.Calls;
 import android.support.annotation.NonNull;
 import android.support.annotation.WorkerThread;
@@ -85,34 +86,72 @@ public class Coalescer {
   @WorkerThread
   @NonNull
   private ImmutableList<CoalescedRow> coalesceInternal(
-      Cursor allAnnotatedCallLogRowsSortedByTimestampDesc) {
+      Cursor allAnnotatedCallLogRowsSortedByTimestampDesc) throws ExpectedCoalescerException {
     Assert.isWorkerThread();
 
-    if (!allAnnotatedCallLogRowsSortedByTimestampDesc.moveToFirst()) {
-      return ImmutableList.of();
-    }
-
     ImmutableList.Builder<CoalescedRow> coalescedRowListBuilder = new ImmutableList.Builder<>();
 
-    RowCombiner rowCombiner = new RowCombiner(allAnnotatedCallLogRowsSortedByTimestampDesc);
-    rowCombiner.startNewGroup();
+    try {
+      if (!allAnnotatedCallLogRowsSortedByTimestampDesc.moveToFirst()) {
+        return ImmutableList.of();
+      }
 
-    long coalescedRowId = 0;
-    do {
-      boolean isRowMerged = rowCombiner.mergeRow(allAnnotatedCallLogRowsSortedByTimestampDesc);
+      RowCombiner rowCombiner = new RowCombiner(allAnnotatedCallLogRowsSortedByTimestampDesc);
+      rowCombiner.startNewGroup();
 
-      if (isRowMerged) {
-        allAnnotatedCallLogRowsSortedByTimestampDesc.moveToNext();
-      }
+      long coalescedRowId = 0;
+      do {
+        boolean isRowMerged = rowCombiner.mergeRow(allAnnotatedCallLogRowsSortedByTimestampDesc);
+
+        if (isRowMerged) {
+          allAnnotatedCallLogRowsSortedByTimestampDesc.moveToNext();
+        }
+
+        if (!isRowMerged || allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast()) {
+          coalescedRowListBuilder.add(
+              rowCombiner.combine().toBuilder().setId(coalescedRowId++).build());
+          rowCombiner.startNewGroup();
+        }
+      } while (!allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast());
+
+      return coalescedRowListBuilder.build();
 
-      if (!isRowMerged || allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast()) {
-        coalescedRowListBuilder.add(
-            rowCombiner.combine().toBuilder().setId(coalescedRowId++).build());
-        rowCombiner.startNewGroup();
+    } catch (Exception exception) {
+      // Coalescing can fail if cursor "allAnnotatedCallLogRowsSortedByTimestampDesc" is closed by
+      // its loader while the work is still in progress.
+      //
+      // This can happen when the loader restarts and finishes loading data before the coalescing
+      // work is completed.
+      //
+      // This kind of failure doesn't have to crash the app as coalescing will be restarted on the
+      // latest data obtained by the loader. Therefore, we inspect the exception here and throw an
+      // ExpectedCoalescerException if it is the case described above.
+      //
+      // The type of expected exception depends on whether AbstractWindowedCursor#checkPosition() is
+      // called when the cursor is closed.
+      //   (1) If it is called before the cursor is closed, we will get IllegalStateException thrown
+      //       by SQLiteClosable when it attempts to acquire a reference to the database.
+      //   (2) Otherwise, we will get StaleDataException thrown by AbstractWindowedCursor's
+      //       checkPosition() method.
+      //
+      // Note that it would be more accurate to inspect the stack trace to locate the origin of the
+      // exception. However, according to the documentation on Throwable#getStackTrace, "some
+      // virtual machines may, under some circumstances, omit one or more stack frames from the
+      // stack trace". "In the extreme case, a virtual machine that has no stack trace information
+      // concerning this throwable is permitted to return a zero-length array from this method."
+      // Therefore, the best we can do is to inspect the message in the exception.
+      // TODO(linyuh): try to avoid the expected failure.
+      String message = exception.getMessage();
+      if (message != null
+          && ((exception instanceof StaleDataException
+                  && message.startsWith("Attempting to access a closed CursorWindow"))
+              || (exception instanceof IllegalStateException
+                  && message.startsWith("attempt to re-open an already-closed object")))) {
+        throw new ExpectedCoalescerException(exception);
       }
-    } while (!allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast());
 
-    return coalescedRowListBuilder.build();
+      throw exception;
+    }
   }
 
   /** Combines rows from {@link AnnotatedCallLog} into a {@link CoalescedRow}. */
@@ -337,4 +376,11 @@ public class Coalescer {
       return dialerPhoneNumberUtil.isMatch(groupPhoneNumber, rowPhoneNumber);
     }
   }
+
+  /** A checked exception thrown when expected failure happens when coalescing is in progress. */
+  public static final class ExpectedCoalescerException extends Exception {
+    ExpectedCoalescerException(Throwable throwable) {
+      super("Expected coalescing exception", throwable);
+    }
+  }
 }
diff --git a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
index 4141fe723..5e72a1af3 100644
--- a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
+++ b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
@@ -17,7 +17,6 @@ package com.android.dialer.calllog.ui;
 
 import android.app.Activity;
 import android.database.Cursor;
-import android.database.StaleDataException;
 import android.os.Bundle;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
@@ -34,6 +33,7 @@ import android.view.ViewGroup;
 import com.android.dialer.calllog.CallLogComponent;
 import com.android.dialer.calllog.RefreshAnnotatedCallLogReceiver;
 import com.android.dialer.calllog.database.CallLogDatabaseComponent;
+import com.android.dialer.calllog.database.Coalescer;
 import com.android.dialer.calllog.model.CoalescedRow;
 import com.android.dialer.common.Assert;
 import com.android.dialer.common.LogUtil;
@@ -337,16 +337,13 @@ public final class NewCallLogFragment extends Fragment implements LoaderCallback
           }
         },
         throwable -> {
-          if (throwable instanceof StaleDataException) {
-            // Coalescing can fail if the cursor passed to Coalescer is closed by the loader while
-            // the work is still in progress.
-            // This can happen when the loader restarts and finishes loading data before the
-            // coalescing work is completed.
-            // This failure doesn't need to be thrown as coalescing will be restarted on the latest
-            // data obtained by the loader.
-            // TODO(linyuh): Also throw an exception if the failure above can be avoided.
-            LogUtil.e("NewCallLogFragment.onLoadFinished", "coalescing failed", throwable);
-          } else {
+          // Coalescing can fail if the cursor passed to Coalescer is closed by the loader while
+          // the work is still in progress.
+          // This can happen when the loader restarts and finishes loading data before the
+          // coalescing work is completed.
+          // This failure is identified by ExpectedCoalescerException and doesn't need to be
+          // thrown as coalescing will be restarted on the latest data obtained by the loader.
+          if (!(throwable instanceof Coalescer.ExpectedCoalescerException)) {
             throw new AssertionError(throwable);
           }
         });