Change the DateFormatSymbols serialized form

Adding locale to the serialized form of
DateFormatSymbols.

Some duplicated code around the generation of
numeric / offset timezone strings (e.g. GMT+08:00) has
been removed. A new hidden method has been added
to TimeZone to create this string.

The timezone string lookup has been moved into
LocaleData and it now has a single path rather
than treating the customZoneStrings path as special.

customZoneStrings has been removed from
DateFormatSymbols because it would potentially
have an incorrect value after serialization and
it is no longer required.

Bug: 16502916
Change-Id: I2b317e34ba4772beb372a75dd08c95113408b9cc
diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/text/DateFormatSymbolsTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/text/DateFormatSymbolsTest.java
index 70e41a2..1a6a25e 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/text/DateFormatSymbolsTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/text/DateFormatSymbolsTest.java
@@ -16,8 +16,6 @@
  */
 package org.apache.harmony.tests.java.text;
 
-import java.io.File;
-import java.net.URL;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.ObjectInputStream;
@@ -25,7 +23,6 @@
 import java.text.DateFormatSymbols;
 import java.util.Arrays;
 import java.util.Locale;
-import java.util.ServiceConfigurationError;
 
 public class DateFormatSymbolsTest extends junit.framework.TestCase {
 
@@ -402,13 +399,6 @@
         dfs = new DateFormatSymbols(new Locale("en", "us"));
     }
 
-    /**
-     * Tears down the fixture, for example, close a network connection. This
-     * method is called after a test is executed.
-     */
-    protected void tearDown() {
-    }
-
     // Test serialization mechanism of DateFormatSymbols
     public void test_serialization() throws Exception {
         DateFormatSymbols symbols = new DateFormatSymbols(Locale.FRANCE);
@@ -426,7 +416,6 @@
         DateFormatSymbols symbolsD = (DateFormatSymbols) objectIStream
                 .readObject();
 
-        // The associated currency will not persist
         String[][] zoneStringsD = symbolsD.getZoneStrings();
         assertNotNull(zoneStringsD);
         assertEquals(symbols, symbolsD);
diff --git a/luni/src/main/java/java/text/DateFormatSymbols.java b/luni/src/main/java/java/text/DateFormatSymbols.java
index 54a1a3f..56c1e65 100644
--- a/luni/src/main/java/java/text/DateFormatSymbols.java
+++ b/luni/src/main/java/java/text/DateFormatSymbols.java
@@ -63,15 +63,13 @@
 
     // Localized display names.
     String[][] zoneStrings;
-    // Has the user called setZoneStrings?
-    transient boolean customZoneStrings;
 
-    /**
-     * Locale, necessary to lazily load time zone strings. We force the time
-     * zone names to load upon serialization, so this will never be needed
-     * post deserialization.
+    /*
+     * Locale, necessary to lazily load time zone strings. Added to the serialized form for
+     * Android's L release. May be null if the object was deserialized using data from older
+     * releases. See b/16502916.
      */
-    transient final Locale locale;
+    final Locale locale;
 
     /**
      * Gets zone strings, initializing them if necessary. Does not create
@@ -153,11 +151,12 @@
     private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
         ois.defaultReadObject();
 
-        // NOTE: We don't serialize the locale we were created with, so we can't
-        // get back the localeData object we want. This is broken for callers that
-        // access this field directly (i.e, SimpleDateFormat). We should ideally
-        // have serialized the locale we were created with. See b/16502916.
-        this.localeData = LocaleData.get(Locale.getDefault());
+        Locale locale = this.locale;
+        if (locale == null) {
+            // Before the L release Android did not serialize the locale. Handle its absence.
+            locale = Locale.getDefault();
+        }
+        this.localeData = LocaleData.get(locale);
     }
 
     private void writeObject(ObjectOutputStream oos) throws IOException {
@@ -220,7 +219,6 @@
         // 'zoneStrings' is so large, we just print a representative value.
         return getClass().getName() +
                 "[amPmStrings=" + Arrays.toString(ampms) +
-                ",customZoneStrings=" + customZoneStrings +
                 ",eras=" + Arrays.toString(eras) +
                 ",localPatternChars=" + localPatternChars +
                 ",months=" + Arrays.toString(months) +
@@ -488,6 +486,25 @@
             }
         }
         this.zoneStrings = clone2dStringArray(zoneStrings);
-        this.customZoneStrings = true;
+    }
+
+    /**
+     * Returns the display name of the timezone specified. Returns null if no name was found in the
+     * zone strings.
+     *
+     * @param daylight whether to return the daylight savings or the standard name
+     * @param style one of the {@link TimeZone} styles such as {@link TimeZone#SHORT}
+     *
+     * @hide used internally
+     */
+    String getTimeZoneDisplayName(TimeZone tz, boolean daylight, int style) {
+        if (style != TimeZone.SHORT && style != TimeZone.LONG) {
+            throw new IllegalArgumentException("Bad style: " + style);
+        }
+
+        // If custom zone strings have been set with setZoneStrings() we use those. Otherwise we
+        // use the ones from LocaleData.
+        String[][] zoneStrings = internalZoneStrings();
+        return TimeZoneNames.getDisplayName(zoneStrings, tz.getID(), daylight, style);
     }
 }
diff --git a/luni/src/main/java/java/text/SimpleDateFormat.java b/luni/src/main/java/java/text/SimpleDateFormat.java
index 259bfe0..c1a8ee9 100644
--- a/luni/src/main/java/java/text/SimpleDateFormat.java
+++ b/luni/src/main/java/java/text/SimpleDateFormat.java
@@ -740,15 +740,9 @@
             TimeZone tz = calendar.getTimeZone();
             boolean daylight = (calendar.get(Calendar.DST_OFFSET) != 0);
             int style = count < 4 ? TimeZone.SHORT : TimeZone.LONG;
-            if (!formatData.customZoneStrings) {
-                buffer.append(tz.getDisplayName(daylight, style, formatData.locale));
-                return;
-            }
-            // We can't call TimeZone.getDisplayName() because it would not use
-            // the custom DateFormatSymbols of this SimpleDateFormat.
-            String custom = TimeZoneNames.getDisplayName(formatData.zoneStrings, tz.getID(), daylight, style);
-            if (custom != null) {
-                buffer.append(custom);
+            String zoneString = formatData.getTimeZoneDisplayName(tz, daylight, style);
+            if (zoneString != null) {
+                buffer.append(zoneString);
                 return;
             }
         }
@@ -759,21 +753,11 @@
     // See http://www.unicode.org/reports/tr35/#Date_Format_Patterns for the different counts.
     // @param generalTimeZone "GMT-08:00" rather than "-0800".
     private void appendNumericTimeZone(StringBuffer buffer, int count, boolean generalTimeZone) {
-        int offset = calendar.get(Calendar.ZONE_OFFSET) + calendar.get(Calendar.DST_OFFSET);
-        char sign = '+';
-        if (offset < 0) {
-            sign = '-';
-            offset = -offset;
-        }
-        if (generalTimeZone || count == 4) {
-            buffer.append("GMT");
-        }
-        buffer.append(sign);
-        appendNumber(buffer, 2, offset / 3600000);
-        if (generalTimeZone || count >= 4) {
-            buffer.append(':');
-        }
-        appendNumber(buffer, 2, (offset % 3600000) / 60000);
+        int offsetMillis = calendar.get(Calendar.ZONE_OFFSET) + calendar.get(Calendar.DST_OFFSET);
+        boolean includeGmt = generalTimeZone || count == 4;
+        boolean includeMinuteSeparator = generalTimeZone || count >= 4;
+        buffer.append(TimeZone.createGmtOffsetString(includeGmt,  includeMinuteSeparator,
+                offsetMillis));
     }
 
     private void appendMilliseconds(StringBuffer buffer, int count, int value) {
diff --git a/luni/src/main/java/java/util/TimeZone.java b/luni/src/main/java/java/util/TimeZone.java
index e4c68c5..854a4a6 100644
--- a/luni/src/main/java/java/util/TimeZone.java
+++ b/luni/src/main/java/java/util/TimeZone.java
@@ -63,7 +63,7 @@
  *
  * @see Calendar
  * @see GregorianCalendar
- * @see SimpleDateFormat
+ * @see java.text.SimpleDateFormat
  */
 public abstract class TimeZone implements Serializable, Cloneable {
     private static final long serialVersionUID = 3581463369166924961L;
@@ -206,27 +206,48 @@
         // upgrade to icu4c 50 and rewrite the underlying native code. See also the
         // "element[j] != null" check in SimpleDateFormat.parseTimeZone, and the extra work in
         // DateFormatSymbols.getZoneStrings.
-
-        int offset = getRawOffset();
+        int offsetMillis = getRawOffset();
         if (daylightTime) {
-            offset += getDSTSavings();
+            offsetMillis += getDSTSavings();
         }
-        offset /= 60000;
+        return createGmtOffsetString(true /* includeGmt */, true /* includeMinuteSeparator */,
+                offsetMillis);
+    }
+
+    /**
+     * Returns a string representation of an offset from UTC.
+     *
+     * <p>The format is "[GMT](+|-)HH[:]MM". The output is not localized.
+     *
+     * @param includeGmt true to include "GMT", false to exclude
+     * @param includeMinuteSeparator true to include the separator between hours and minutes, false
+     *     to exclude.
+     * @param offsetMillis the offset from UTC
+     *
+     * @hide used internally by SimpleDateFormat
+     */
+    public static String createGmtOffsetString(boolean includeGmt,
+            boolean includeMinuteSeparator, int offsetMillis) {
+        int offsetMinutes = offsetMillis / 60000;
         char sign = '+';
-        if (offset < 0) {
+        if (offsetMinutes < 0) {
             sign = '-';
-            offset = -offset;
+            offsetMinutes = -offsetMinutes;
         }
         StringBuilder builder = new StringBuilder(9);
-        builder.append("GMT");
+        if (includeGmt) {
+            builder.append("GMT");
+        }
         builder.append(sign);
-        appendNumber(builder, 2, offset / 60);
-        builder.append(':');
-        appendNumber(builder, 2, offset % 60);
+        appendNumber(builder, 2, offsetMinutes / 60);
+        if (includeMinuteSeparator) {
+            builder.append(':');
+        }
+        appendNumber(builder, 2, offsetMinutes % 60);
         return builder.toString();
     }
 
-    private void appendNumber(StringBuilder builder, int count, int value) {
+    private static void appendNumber(StringBuilder builder, int count, int value) {
         String string = Integer.toString(value);
         for (int i = 0; i < count - string.length(); i++) {
             builder.append('0');
diff --git a/luni/src/test/java/libcore/java/text/DateFormatSymbolsTest.java b/luni/src/test/java/libcore/java/text/DateFormatSymbolsTest.java
index 9ab6f70..057cd17 100644
--- a/luni/src/test/java/libcore/java/text/DateFormatSymbolsTest.java
+++ b/luni/src/test/java/libcore/java/text/DateFormatSymbolsTest.java
@@ -46,8 +46,8 @@
     }
 
     public void testSerialization() throws Exception {
-        // Set the default locale. The default locale determines what strings are used by the
-        // DateFormatSymbols after deserialization.
+        // Set the default locale. The default locale used to determine what strings were used by
+        // the DateFormatSymbols after deserialization. See http://b/16502916
         Locale.setDefault(Locale.US);
 
         // The Polish language needs stand-alone month and weekday names.
@@ -64,17 +64,16 @@
         DateFormatSymbols deserializedDfs = (DateFormatSymbols) in.readObject();
         assertEquals(-1, in.read());
 
-        // The two objects should claim to be equal, even though they aren't really.
+        // The two objects be equal.
         assertEquals(originalDfs, deserializedDfs);
 
         // The original differentiates between regular month names and stand-alone month names...
         assertEquals("stycznia", formatDate(pl, "MMMM", originalDfs));
         assertEquals("stycze\u0144", formatDate(pl, "LLLL", originalDfs));
 
-        // But the deserialized object is screwed because the RI's serialized form doesn't
-        // contain the locale or the necessary strings. Don't serialize DateFormatSymbols, folks!
+        // And so does the deserialized version.
         assertEquals("stycznia", formatDate(pl, "MMMM", deserializedDfs));
-        assertEquals("January", formatDate(pl, "LLLL", deserializedDfs));
+        assertEquals("stycze\u0144", formatDate(pl, "LLLL", deserializedDfs));
     }
 
     private String formatDate(Locale l, String fmt, DateFormatSymbols dfs) {