Restore cached timestamp correctly

Bug: 33751444

When reinitializing the calculator after restarting in RESULT state,
we were reconstructing the last history entry from the main expression,
without correctly restoring the timestamp. Instead read the expression,
together with its timestamp, from the database.

Correctly distinguish between result-already-saved and
result-not-yet-saved states when evaluation completes.
There may be very odd cases in which we generate an error the first
time, setting the state to INIT, and successfully evaluate the
second time (because we need less precision) that the old logic
didn't handle. And testing for INIT_FOR_RESULT is cleaner anyway.
(We can still, under very weird conditions, put expressions in the
history that generate an error after evaluating correctly the
first time. We should be robust against that already.)

Add a couple of comments that would have made this easier to track
down.

Change-Id: I85756cca5d8fb76b2cbeb9ed32a788ae71b6d65e

2 files changed, 16 insertions, 13 deletions

diff --git a/src/com/android/calculator2/Calculator.java b/src/com/android/calculator2/Calculator.java
index 582f0c9..60731ae 100644
--- a/src/com/android/calculator2/Calculator.java
+++ b/src/com/android/calculator2/Calculator.java
@@ -934,7 +934,8 @@ public class Calculator extends Activity
         mResultText.onEvaluate(index, initDisplayPrec, msd, leastDigPos, truncatedWholeNumber);
         if (mCurrentState != CalculatorState.INPUT) {
             // In EVALUATE, INIT, or INIT_FOR_RESULT state.
-            onResult(mCurrentState == CalculatorState.EVALUATE);
+            onResult(mCurrentState == CalculatorState.EVALUATE /* animate */,
+                     mCurrentState == CalculatorState.INIT_FOR_RESULT /* previously preserved */);
         }
     }
 
@@ -1156,7 +1157,7 @@ public class Calculator extends Activity
     // formula and result displays back at the end of the animation.  We no longer do that,
     // so that we can continue to properly support scrolling of the result.
     // We assume the result already contains the text to be expanded.
-    private void onResult(boolean animate) {
+    private void onResult(boolean animate, boolean resultWasPreserved) {
         // Calculate the textSize that would be used to display the result in the formula.
         // For scrollable results just use the minimum textSize to maximize the number of digits
         // that are visible on screen.
@@ -1188,10 +1189,15 @@ public class Calculator extends Activity
         // Change the result's textColor to match the formula.
         final int formulaTextColor = mFormulaText.getCurrentTextColor();
 
-        if (animate) {
+        if (resultWasPreserved) {
+            // Result was previously addded to history.
+            mEvaluator.represerve();
+        } else {
             // Add current result to history.
             mEvaluator.preserve(true);
+        }
 
+        if (animate) {
             mResultText.announceForAccessibility(getResources().getString(R.string.desc_eq));
             mResultText.announceForAccessibility(mResultText.getText());
             setState(CalculatorState.ANIMATE);
@@ -1222,7 +1228,6 @@ public class Calculator extends Activity
             mResultText.setTranslationY(resultTranslationY);
             mResultText.setTextColor(formulaTextColor);
             mFormulaContainer.setTranslationY(formulaTranslationY);
-            mEvaluator.represerve();
             setState(CalculatorState.RESULT);
         }
     }
diff --git a/src/com/android/calculator2/Evaluator.java b/src/com/android/calculator2/Evaluator.java
index 9bdc291..148ca28 100644
--- a/src/com/android/calculator2/Evaluator.java
+++ b/src/com/android/calculator2/Evaluator.java
@@ -1375,6 +1375,7 @@ public class Evaluator implements CalculatorExpr.ExprResolver {
     /**
      * Return an ExprInfo for a copy of the expression with the given index.
      * We remove trailing binary operators in the copy.
+     * mTimeStamp is not copied.
      */
     private ExprInfo copy(long index, boolean copyValue) {
         ExprInfo fromEi = mExprs.get(index);
@@ -1432,6 +1433,7 @@ public class Evaluator implements CalculatorExpr.ExprResolver {
     /**
      * Add the expression described by the argument to the database.
      * Returns the new row id in the database.
+     * Fills in timestamp in ei, if it was not previously set.
      * If in_history is true, add it with a positive index, so it will appear in the history.
      */
     private long addToDB(boolean in_history, ExprInfo ei) {
@@ -1471,15 +1473,11 @@ public class Evaluator implements CalculatorExpr.ExprResolver {
      */
     public void represerve() {
         long resultIndex = getMaxIndex();
-        if (mExprs.get(resultIndex) != null) {
-            // We actually didn't lose the cache. Nothing to do.
-            return;
-        }
-        ExprInfo ei = copy(MAIN_INDEX, true);
-        if (resultIndex == MAIN_INDEX) {
-            throw new AssertionError("Should not store main expression");
-        }
-        mExprs.put(resultIndex, ei);
+        // This requires database access only if the local state was preserved, but we
+        // recreated the Evaluator.  That excludes the common cases of device rotation, etc.
+        // TODO: Revisit once we deal with database failures. We could just copy from
+        // MAIN_INDEX instead, but that loses the timestamp.
+        ensureExprIsCached(resultIndex);
     }
 
     /**