Fixing android.text.format.Time for non-English locales

The test fixes for bug 17262063 showed up a real issue for
non-English locales with the Time.format() method:
If the Android string resources that contain the pattern use
non-ASCII characters then a '?' would be output instead of
those characters.

For example, in France the pattern for '%c' includes a 'à'
(a with a grave accent) and Japan includes 日.

The problem was due to converting the pattern to bytes using
the US_ASCII character set, which turns non-ASCII characters
into '?'. The code has been changed to use char throughout
and avoid bytes.

Internal documentation has been improved.

Some calls to modifyAndAppend() have been replaced with a
direct call to outputBuilder.append() because the
modify step is guaranteed to a no-op for the literals given.

The formatter has been changed to use Locale.US because it
is only used for outputting numbers. It has been renamed
to make this more obvious and the locale field has been
removed.

Bug: 17262063

(cherry picked from commit 788cb18f652fca380acefdadce305415bc0602b0)

Change-Id: I96ee158fbeb01827f0bbf022631625416f872fdb

1 files changed, 59 insertions, 78 deletions

diff --git a/core/java/android/text/format/TimeFormatter.java b/core/java/android/text/format/TimeFormatter.java
index ec79b36..3a63805 100644
--- a/core/java/android/text/format/TimeFormatter.java
+++ b/core/java/android/text/format/TimeFormatter.java
@@ -22,8 +22,7 @@ package android.text.format;
 
 import android.content.res.Resources;
 
-import java.nio.ByteBuffer;
-import java.nio.charset.StandardCharsets;
+import java.nio.CharBuffer;
 import java.util.Formatter;
 import java.util.Locale;
 import java.util.TimeZone;
@@ -31,15 +30,13 @@ import libcore.icu.LocaleData;
 import libcore.util.ZoneInfo;
 
 /**
- * Formatting logic for {@link Time}. Contains a port of Bionic's broken strftime_tz to Java. The
- * main issue with this implementation is the treatment of characters as ASCII, despite returning
- * localized (UTF-16) strings from the LocaleData.
+ * Formatting logic for {@link Time}. Contains a port of Bionic's broken strftime_tz to Java.
  *
  * <p>This class is not thread safe.
  */
 class TimeFormatter {
-    // An arbitrary value outside the range representable by a byte / ASCII character code.
-    private static final int FORCE_LOWER_CASE = 0x100;
+    // An arbitrary value outside the range representable by a char.
+    private static final int FORCE_LOWER_CASE = -1;
 
     private static final int SECSPERMIN = 60;
     private static final int MINSPERHOUR = 60;
@@ -62,10 +59,9 @@ class TimeFormatter {
     private final String dateTimeFormat;
     private final String timeOnlyFormat;
     private final String dateOnlyFormat;
-    private final Locale locale;
 
     private StringBuilder outputBuilder;
-    private Formatter outputFormatter;
+    private Formatter numberFormatter;
 
     public TimeFormatter() {
         synchronized (TimeFormatter.class) {
@@ -84,7 +80,6 @@ class TimeFormatter {
             this.dateTimeFormat = sDateTimeFormat;
             this.timeOnlyFormat = sTimeOnlyFormat;
             this.dateOnlyFormat = sDateOnlyFormat;
-            this.locale = locale;
             localeData = sLocaleData;
         }
     }
@@ -97,19 +92,21 @@ class TimeFormatter {
             StringBuilder stringBuilder = new StringBuilder();
 
             outputBuilder = stringBuilder;
-            outputFormatter = new Formatter(stringBuilder, locale);
+            // This uses the US locale because number localization is handled separately (see below)
+            // and locale sensitive strings are output directly using outputBuilder.
+            numberFormatter = new Formatter(stringBuilder, Locale.US);
 
             formatInternal(pattern, wallTime, zoneInfo);
             String result = stringBuilder.toString();
             // This behavior is the source of a bug since some formats are defined as being
-            // in ASCII. Generally localization is very broken.
+            // in ASCII and not localized.
             if (localeData.zeroDigit != '0') {
                 result = localizeDigits(result);
             }
             return result;
         } finally {
             outputBuilder = null;
-            outputFormatter = null;
+            numberFormatter = null;
         }
     }
 
@@ -132,38 +129,30 @@ class TimeFormatter {
      * {@link #outputBuilder}.
      */
     private void formatInternal(String pattern, ZoneInfo.WallTime wallTime, ZoneInfo zoneInfo) {
-        // Convert to ASCII bytes to be compatible with old implementation behavior.
-        byte[] bytes = pattern.getBytes(StandardCharsets.US_ASCII);
-        if (bytes.length == 0) {
-            return;
-        }
-
-        ByteBuffer formatBuffer = ByteBuffer.wrap(bytes);
+        CharBuffer formatBuffer = CharBuffer.wrap(pattern);
         while (formatBuffer.remaining() > 0) {
-            boolean outputCurrentByte = true;
-            char currentByteAsChar = convertToChar(formatBuffer.get(formatBuffer.position()));
-            if (currentByteAsChar == '%') {
-                outputCurrentByte = handleToken(formatBuffer, wallTime, zoneInfo);
+            boolean outputCurrentChar = true;
+            char currentChar = formatBuffer.get(formatBuffer.position());
+            if (currentChar == '%') {
+                outputCurrentChar = handleToken(formatBuffer, wallTime, zoneInfo);
             }
-            if (outputCurrentByte) {
-                currentByteAsChar = convertToChar(formatBuffer.get(formatBuffer.position()));
-                outputBuilder.append(currentByteAsChar);
+            if (outputCurrentChar) {
+                outputBuilder.append(formatBuffer.get(formatBuffer.position()));
             }
-
             formatBuffer.position(formatBuffer.position() + 1);
         }
     }
 
-    private boolean handleToken(ByteBuffer formatBuffer, ZoneInfo.WallTime wallTime,
+    private boolean handleToken(CharBuffer formatBuffer, ZoneInfo.WallTime wallTime,
             ZoneInfo zoneInfo) {
 
-        // The byte at formatBuffer.position() is expected to be '%' at this point.
+        // The char at formatBuffer.position() is expected to be '%' at this point.
         int modifier = 0;
         while (formatBuffer.remaining() > 1) {
-            // Increment the position then get the new current byte.
+            // Increment the position then get the new current char.
             formatBuffer.position(formatBuffer.position() + 1);
-            char currentByteAsChar = convertToChar(formatBuffer.get(formatBuffer.position()));
-            switch (currentByteAsChar) {
+            char currentChar = formatBuffer.get(formatBuffer.position());
+            switch (currentChar) {
                 case 'A':
                     modifyAndAppend((wallTime.getWeekDay() < 0
                                     || wallTime.getWeekDay() >= DAYSPERWEEK)
@@ -206,7 +195,7 @@ class TimeFormatter {
                     formatInternal("%m/%d/%y", wallTime, zoneInfo);
                     return false;
                 case 'd':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getMonthDay());
                     return false;
                 case 'E':
@@ -218,46 +207,46 @@ class TimeFormatter {
                 case '0':
                 case '^':
                 case '#':
-                    modifier = currentByteAsChar;
+                    modifier = currentChar;
                     continue;
                 case 'e':
-                    outputFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
                             wallTime.getMonthDay());
                     return false;
                 case 'F':
                     formatInternal("%Y-%m-%d", wallTime, zoneInfo);
                     return false;
                 case 'H':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getHour());
                     return false;
                 case 'I':
                     int hour = (wallTime.getHour() % 12 != 0) ? (wallTime.getHour() % 12) : 12;
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), hour);
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), hour);
                     return false;
                 case 'j':
                     int yearDay = wallTime.getYearDay() + 1;
-                    outputFormatter.format(getFormat(modifier, "%03d", "%3d", "%d", "%03d"),
+                    numberFormatter.format(getFormat(modifier, "%03d", "%3d", "%d", "%03d"),
                             yearDay);
                     return false;
                 case 'k':
-                    outputFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"),
                             wallTime.getHour());
                     return false;
                 case 'l':
                     int n2 = (wallTime.getHour() % 12 != 0) ? (wallTime.getHour() % 12) : 12;
-                    outputFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"), n2);
+                    numberFormatter.format(getFormat(modifier, "%2d", "%2d", "%d", "%02d"), n2);
                     return false;
                 case 'M':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getMinute());
                     return false;
                 case 'm':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getMonth() + 1);
                     return false;
                 case 'n':
-                    modifyAndAppend("\n", modifier);
+                    outputBuilder.append('\n');
                     return false;
                 case 'p':
                     modifyAndAppend((wallTime.getHour() >= (HOURSPERDAY / 2)) ? localeData.amPm[1]
@@ -274,27 +263,27 @@ class TimeFormatter {
                     formatInternal("%I:%M:%S %p", wallTime, zoneInfo);
                     return false;
                 case 'S':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             wallTime.getSecond());
                     return false;
                 case 's':
                     int timeInSeconds = wallTime.mktime(zoneInfo);
-                    modifyAndAppend(Integer.toString(timeInSeconds), modifier);
+                    outputBuilder.append(Integer.toString(timeInSeconds));
                     return false;
                 case 'T':
                     formatInternal("%H:%M:%S", wallTime, zoneInfo);
                     return false;
                 case 't':
-                    modifyAndAppend("\t", modifier);
+                    outputBuilder.append('\t');
                     return false;
                 case 'U':
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"),
                             (wallTime.getYearDay() + DAYSPERWEEK - wallTime.getWeekDay())
                                     / DAYSPERWEEK);
                     return false;
                 case 'u':
                     int day = (wallTime.getWeekDay() == 0) ? DAYSPERWEEK : wallTime.getWeekDay();
-                    outputFormatter.format("%d", day);
+                    numberFormatter.format("%d", day);
                     return false;
                 case 'V':   /* ISO 8601 week number */
                 case 'G':   /* ISO 8601 year (four digits) */
@@ -326,9 +315,9 @@ class TimeFormatter {
                         --year;
                         yday += isLeap(year) ? DAYSPERLYEAR : DAYSPERNYEAR;
                     }
-                    if (currentByteAsChar == 'V') {
-                        outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), w);
-                    } else if (currentByteAsChar == 'g') {
+                    if (currentChar == 'V') {
+                        numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), w);
+                    } else if (currentChar == 'g') {
                         outputYear(year, false, true, modifier);
                     } else {
                         outputYear(year, true, true, modifier);
@@ -342,10 +331,10 @@ class TimeFormatter {
                     int n = (wallTime.getYearDay() + DAYSPERWEEK - (
                                     wallTime.getWeekDay() != 0 ? (wallTime.getWeekDay() - 1)
                                             : (DAYSPERWEEK - 1))) / DAYSPERWEEK;
-                    outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
+                    numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
                     return false;
                 case 'w':
-                    outputFormatter.format("%d", wallTime.getWeekDay());
+                    numberFormatter.format("%d", wallTime.getWeekDay());
                     return false;
                 case 'X':
                     formatInternal(timeOnlyFormat, wallTime, zoneInfo);
@@ -371,17 +360,17 @@ class TimeFormatter {
                         return false;
                     }
                     int diff = wallTime.getGmtOffset();
-                    String sign;
+                    char sign;
                     if (diff < 0) {
-                        sign = "-";
+                        sign = '-';
                         diff = -diff;
                     } else {
-                        sign = "+";
+                        sign = '+';
                     }
-                    modifyAndAppend(sign, modifier);
+                    outputBuilder.append(sign);
                     diff /= SECSPERMIN;
                     diff = (diff / MINSPERHOUR) * 100 + (diff % MINSPERHOUR);
-                    outputFormatter.format(getFormat(modifier, "%04d", "%4d", "%d", "%04d"), diff);
+                    numberFormatter.format(getFormat(modifier, "%04d", "%4d", "%d", "%04d"), diff);
                     return false;
                 }
                 case '+':
@@ -422,7 +411,6 @@ class TimeFormatter {
                 break;
             default:
                 outputBuilder.append(str);
-
         }
     }
 
@@ -443,14 +431,14 @@ class TimeFormatter {
         }
         if (outputTop) {
             if (lead == 0 && trail < 0) {
-                modifyAndAppend("-0", modifier);
+                outputBuilder.append("-0");
             } else {
-                outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), lead);
+                numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), lead);
             }
         }
         if (outputBottom) {
             int n = ((trail < 0) ? -trail : trail);
-            outputFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
+            numberFormatter.format(getFormat(modifier, "%02d", "%2d", "%d", "%02d"), n);
         }
     }
 
@@ -472,24 +460,24 @@ class TimeFormatter {
     }
 
     /**
-     * A broken implementation of {@link Character#isUpperCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#isUpperCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static boolean brokenIsUpper(char toCheck) {
         return toCheck >= 'A' && toCheck <= 'Z';
     }
 
     /**
-     * A broken implementation of {@link Character#isLowerCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#isLowerCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static boolean brokenIsLower(char toCheck) {
         return toCheck >= 'a' && toCheck <= 'z';
     }
 
     /**
-     * A broken implementation of {@link Character#toLowerCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#toLowerCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static char brokenToLower(char input) {
         if (input >= 'A' && input <= 'Z') {
@@ -499,8 +487,8 @@ class TimeFormatter {
     }
 
     /**
-     * A broken implementation of {@link Character#toUpperCase(char)} that assumes ASCII in order to
-     * be compatible with the old native implementation.
+     * A broken implementation of {@link Character#toUpperCase(char)} that assumes ASCII codes in
+     * order to be compatible with the old native implementation.
      */
     private static char brokenToUpper(char input) {
         if (input >= 'a' && input <= 'z') {
@@ -509,11 +497,4 @@ class TimeFormatter {
         return input;
     }
 
-    /**
-     * Safely convert a byte containing an ASCII character to a char, even for character codes
-     * > 127.
-     */
-    private static char convertToChar(byte b) {
-        return (char) (b & 0xFF);
-    }
 }