Remove use of MeasureUnit.internalGetInstance

MeasureUnit.internalGetUnit() is a method on ICU MeasureUnit which is
used to construct and register MeasureUnits. Calling it from non-ICU
code makes future calls to MeasureUnit.getAvailable(type) return the
newly-created MeasureUnit, but that MeasureUnit will not be fully
supported by ICU (no translations, ...).

This code creates a MeasureUnit by calling a constructor reflectively to
avoid the registration, which is a workaround.

The correct long-term fix is for ICU/CLDR to support petabyte correctly
(http://bugs.icu-project.org/trac/ticket/13355) and for us to just use
that instead.

Bug: 65632959
Test: bit CtsIcuTestCases:android.icu.dev.test.format.MeasureUnitTest
Test: coretests android.text.format.FormatterTest
Change-Id: I494fb59a3b3742f35cbdf6b8705817f404a2c6b0

2 files changed, 47 insertions, 6 deletions

diff --git a/core/java/android/text/format/Formatter.java b/core/java/android/text/format/Formatter.java
index fc56455236a2..2c83fc4d9049 100644
--- a/core/java/android/text/format/Formatter.java
+++ b/core/java/android/text/format/Formatter.java
@@ -32,6 +32,7 @@ import android.text.BidiFormatter;
 import android.text.TextUtils;
 import android.view.View;
 
+import java.lang.reflect.Constructor;
 import java.math.BigDecimal;
 import java.util.Locale;
 
@@ -194,13 +195,29 @@ public final class Formatter {
 
     /**
      * ICU doesn't support PETABYTE yet. Fake it so that we can treat all units the same way.
-     * {@hide}
      */
-    public static final MeasureUnit PETABYTE = MeasureUnit.internalGetInstance(
-            "digital", "petabyte");
+    private static final MeasureUnit PETABYTE = createPetaByte();
 
-    /** {@hide} */
-    public static class RoundedBytesResult {
+    /**
+     * Create a petabyte MeasureUnit without registering it with ICU.
+     * ICU doesn't support user-create MeasureUnit and the only public (but hidden) method to do so
+     * is {@link MeasureUnit#internalGetInstance(String, String)} which also registers the unit as
+     * an available type and thus leaks it to code that doesn't expect or support it.
+     * <p>This method uses reflection to create an instance of MeasureUnit to avoid leaking it. This
+     * instance is <b>only</b> to be used in this class.
+     */
+    private static MeasureUnit createPetaByte() {
+        try {
+            Constructor<MeasureUnit> constructor = MeasureUnit.class
+                    .getDeclaredConstructor(String.class, String.class);
+            constructor.setAccessible(true);
+            return constructor.newInstance("digital", "petabyte");
+        } catch (ReflectiveOperationException e) {
+            throw new RuntimeException("Failed to create petabyte MeasureUnit", e);
+        }
+    }
+
+    private static class RoundedBytesResult {
         public final float value;
         public final MeasureUnit units;
         public final int fractionDigits;
@@ -218,7 +235,7 @@ public final class Formatter {
          * Returns a RoundedBytesResult object based on the input size in bytes and the rounding
          * flags. The result can be used for formatting.
          */
-        public static RoundedBytesResult roundBytes(long sizeBytes, int flags) {
+        static RoundedBytesResult roundBytes(long sizeBytes, int flags) {
             final boolean isNegative = (sizeBytes < 0);
             float result = isNegative ? -sizeBytes : sizeBytes;
             MeasureUnit units = MeasureUnit.BYTE;
diff --git a/core/tests/coretests/src/android/text/format/FormatterTest.java b/core/tests/coretests/src/android/text/format/FormatterTest.java
index ff75c29cc112..24e3646e02fc 100644
--- a/core/tests/coretests/src/android/text/format/FormatterTest.java
+++ b/core/tests/coretests/src/android/text/format/FormatterTest.java
@@ -17,10 +17,12 @@
 package android.text.format;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
 import android.content.Context;
 import android.content.res.Configuration;
 import android.content.res.Resources;
+import android.icu.util.MeasureUnit;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
@@ -32,6 +34,8 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.util.Locale;
+import java.util.Set;
+
 
 @SmallTest
 @RunWith(AndroidJUnit4.class)
@@ -205,4 +209,24 @@ public class FormatterTest {
 
         Locale.setDefault(locale);
     }
+
+    /**
+     * Verifies that Formatter doesn't "leak" the locally defined petabyte unit into ICU via the
+     * {@link MeasureUnit} registry. This test can fail for two reasons:
+     * 1. we regressed and started leaking again. In this case the code needs to be fixed.
+     * 2. ICU started supporting petabyte as a unit, in which case change one needs to revert this
+     * change (I494fb59a3b3742f35cbdf6b8705817f404a2c6b0), remove Formatter.PETABYTE and replace any
+     * usages of that field with just MeasureUnit.PETABYTE.
+     */
+    // http://b/65632959
+    @Test
+    public void doesNotLeakPetabyte() {
+        // Ensure that the Formatter class is loaded when we call .getAvailable().
+        Formatter.formatFileSize(mContext, Long.MAX_VALUE);
+        Set<MeasureUnit> digitalUnits = MeasureUnit.getAvailable("digital");
+        for (MeasureUnit unit : digitalUnits) {
+            // This assert can fail if we don't leak PETABYTE, but ICU has added it, see #2 above.
+            assertNotEquals("petabyte", unit.getSubtype());
+        }
+    }
 }