Merge "Fix TimeZone.getAvailableIDs(int)."

2 files changed, 45 insertions, 13 deletions

diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/TimeZoneTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/TimeZoneTest.java
index 7e6cade..0d16786 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/TimeZoneTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/TimeZoneTest.java
@@ -220,16 +220,32 @@ public class TimeZoneTest extends junit.framework.TestCase {
         TimeZone.setDefault(processDefault);
     }
 
-    /**
-     * @add test {@link java.util.TimeZone#getAvailableIDs(int)}
-     */
+    public void test_getAvailableIDs_I_16947622() {
+        TimeZone tz = TimeZone.getTimeZone("America/Los_Angeles");
+        int rawOffset = tz.getRawOffset();
+        assertEquals(-8 * 60 * 60 * 1000, rawOffset);
+        List<String> ids = Arrays.asList(TimeZone.getAvailableIDs(rawOffset));
+
+        // Obviously, for all time zones, the time zone whose raw offset we started with
+        // should be one of the available ids for that offset.
+        assertTrue(ids.toString(), ids.contains("America/Los_Angeles"));
+
+        // Any one of these might legitimately change its raw offset, though that's
+        // fairly unlikely, and the chances of more than one changing are very slim.
+        assertTrue(ids.toString(), ids.contains("America/Dawson"));
+        assertTrue(ids.toString(), ids.contains("America/Tijuana"));
+        assertTrue(ids.toString(), ids.contains("America/Vancouver"));
+        assertTrue(ids.toString(), ids.contains("Canada/Pacific"));
+        assertTrue(ids.toString(), ids.contains("Canada/Yukon"));
+        assertTrue(ids.toString(), ids.contains("Pacific/Pitcairn"));
+    }
+
     public void test_getAvailableIDs_I() {
         TimeZone tz = TimeZone.getTimeZone("Asia/Shanghai");
         int rawoffset = tz.getRawOffset();
         String[] ids = TimeZone.getAvailableIDs(rawoffset);
         List<String> idList = Arrays.asList(ids);
-        assertTrue("Asia/shanghai and Hongkong should have the same rawoffset",
-                idList.contains("Hongkong"));
+        assertTrue(idList.toString(), idList.contains("Asia/Hong_Kong"));
     }
 
     /**
diff --git a/luni/src/main/java/libcore/util/ZoneInfoDB.java b/luni/src/main/java/libcore/util/ZoneInfoDB.java
index 07aaf04..a9d06a4 100644
--- a/luni/src/main/java/libcore/util/ZoneInfoDB.java
+++ b/luni/src/main/java/libcore/util/ZoneInfoDB.java
@@ -62,11 +62,11 @@ public final class ZoneInfoDB {
     /**
      * The 'ids' array contains time zone ids sorted alphabetically, for binary searching.
      * The other two arrays are in the same order. 'byteOffsets' gives the byte offset
-     * of each time zone, and 'rawUtcOffsets' gives the time zone's raw UTC offset.
+     * of each time zone, and 'rawUtcOffsetsCache' gives the time zone's raw UTC offset.
      */
     private String[] ids;
     private int[] byteOffsets;
-    private int[] rawUtcOffsets;
+    private int[] rawUtcOffsetsCache; // Access this via getRawUtcOffsets instead.
 
     /**
      * ZoneInfo objects are worth caching because they are expensive to create.
@@ -104,7 +104,7 @@ public final class ZoneInfoDB {
       version = "missing";
       zoneTab = "# Emergency fallback data.\n";
       ids = new String[] { "GMT" };
-      byteOffsets = rawUtcOffsets = new int[1];
+      byteOffsets = rawUtcOffsetsCache = new int[1];
     }
 
     private boolean loadData(String path) {
@@ -171,7 +171,6 @@ public final class ZoneInfoDB {
       int idOffset = 0;
 
       byteOffsets = new int[entryCount];
-      rawUtcOffsets = new int[entryCount];
 
       for (int i = 0; i < entryCount; i++) {
         it.readByteArray(idBytes, 0, idBytes.length);
@@ -183,7 +182,7 @@ public final class ZoneInfoDB {
         if (length < 44) {
           throw new AssertionError("length in index file < sizeof(tzhead)");
         }
-        rawUtcOffsets[i] = it.readInt();
+        it.skip(4); // Skip the unused 4 bytes that used to be the raw offset.
 
         // Don't include null chars in the String
         int len = idBytes.length;
@@ -210,16 +209,33 @@ public final class ZoneInfoDB {
       return ids.clone();
     }
 
-    public String[] getAvailableIDs(int rawOffset) {
+    public String[] getAvailableIDs(int rawUtcOffset) {
       List<String> matches = new ArrayList<String>();
-      for (int i = 0, end = rawUtcOffsets.length; i < end; ++i) {
-        if (rawUtcOffsets[i] == rawOffset) {
+      int[] rawUtcOffsets = getRawUtcOffsets();
+      for (int i = 0; i < rawUtcOffsets.length; ++i) {
+        if (rawUtcOffsets[i] == rawUtcOffset) {
           matches.add(ids[i]);
         }
       }
       return matches.toArray(new String[matches.size()]);
     }
 
+    private synchronized int[] getRawUtcOffsets() {
+      if (rawUtcOffsetsCache != null) {
+        return rawUtcOffsetsCache;
+      }
+      rawUtcOffsetsCache = new int[ids.length];
+      for (int i = 0; i < ids.length; ++i) {
+        // This creates a TimeZone, which is quite expensive. Hence the cache.
+        // Note that icu4c does the same (without the cache), so if you're
+        // switching this code over to icu4j you should check its performance.
+        // Telephony shouldn't care, but someone converting a bunch of calendar
+        // events might.
+        rawUtcOffsetsCache[i] = cache.get(ids[i]).getRawOffset();
+      }
+      return rawUtcOffsetsCache;
+    }
+
     public String getVersion() {
       return version;
     }