From 3305bce79d5954d1f7fd8bd0cecfce23e8e3fe3a Mon Sep 17 00:00:00 2001 From: Victor Chang Date: Wed, 27 Dec 2017 11:35:59 +0000 Subject: [PATCH 1/2] Revert "Remove use of MeasureUnit.internalGetInstance" This reverts commit aa5629e60809e4775ca1f05e6f1f296a04a450dc. Test: m Bug: 70005649 Bug: 36994779 Change-Id: I4591870f564567c40fa450866c3050fd5a7a61ae --- core/java/android/text/format/Formatter.java | 29 ++++--------------- .../android/text/format/FormatterTest.java | 21 -------------- 2 files changed, 6 insertions(+), 44 deletions(-) diff --git a/core/java/android/text/format/Formatter.java b/core/java/android/text/format/Formatter.java index 8c90156d159d5..f0b613ee0468b 100644 --- a/core/java/android/text/format/Formatter.java +++ b/core/java/android/text/format/Formatter.java @@ -32,7 +32,6 @@ 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; @@ -195,29 +194,13 @@ public final class Formatter { /** * ICU doesn't support PETABYTE yet. Fake it so that we can treat all units the same way. + * {@hide} */ - private static final MeasureUnit PETABYTE = createPetaByte(); + public static final MeasureUnit PETABYTE = MeasureUnit.internalGetInstance( + "digital", "petabyte"); - /** - * 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. - *

This method uses reflection to create an instance of MeasureUnit to avoid leaking it. This - * instance is only to be used in this class. - */ - private static MeasureUnit createPetaByte() { - try { - Constructor 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 { + /** {@hide} */ + public static class RoundedBytesResult { public final float value; public final MeasureUnit units; public final int fractionDigits; @@ -235,7 +218,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. */ - static RoundedBytesResult roundBytes(long sizeBytes, int flags) { + public 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 9c544f47717ca..04d2dad4e2241 100644 --- a/core/tests/coretests/src/android/text/format/FormatterTest.java +++ b/core/tests/coretests/src/android/text/format/FormatterTest.java @@ -17,7 +17,6 @@ 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; @@ -210,24 +209,4 @@ 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 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()); - } - } } From d990345d8bba0f503bddd413bd33bdcea6adf7d6 Mon Sep 17 00:00:00 2001 From: Victor Chang Date: Wed, 27 Dec 2017 11:36:25 +0000 Subject: [PATCH 2/2] Revert "Switch file size formatters to use ICU's MeasureFormat" This reverts commit 4e5b71f084f62203adb732cefc2d2f5ecdaac1c1. Test: cts-tradefed run cts-dev -m CtsTextTestCases Bug: 70005649 Bug: 36994779 Change-Id: Ie9a2fd786d48e2a6c291e313cbb4072c7306af9f --- core/java/android/text/format/Formatter.java | 259 ++++++------------- core/res/res/values/strings.xml | 20 +- core/res/res/values/symbols.xml | 4 + 3 files changed, 95 insertions(+), 188 deletions(-) diff --git a/core/java/android/text/format/Formatter.java b/core/java/android/text/format/Formatter.java index f0b613ee0468b..ad3b4b6d63436 100644 --- a/core/java/android/text/format/Formatter.java +++ b/core/java/android/text/format/Formatter.java @@ -20,11 +20,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; import android.content.res.Resources; -import android.icu.text.DecimalFormat; import android.icu.text.MeasureFormat; -import android.icu.text.NumberFormat; -import android.icu.text.UnicodeSet; -import android.icu.text.UnicodeSetSpanner; import android.icu.util.Measure; import android.icu.util.MeasureUnit; import android.net.NetworkUtils; @@ -32,7 +28,6 @@ import android.text.BidiFormatter; import android.text.TextUtils; import android.view.View; -import java.math.BigDecimal; import java.util.Locale; /** @@ -41,8 +36,6 @@ import java.util.Locale; */ public final class Formatter { - /** {@hide} */ - public static final int FLAG_DEFAULT = 0; /** {@hide} */ public static final int FLAG_SHORTER = 1 << 0; /** {@hide} */ @@ -65,9 +58,7 @@ public final class Formatter { return context.getResources().getConfiguration().getLocales().get(0); } - /** - * Wraps the source string in bidi formatting characters in RTL locales. - */ + /* Wraps the source string in bidi formatting characters in RTL locales */ private static String bidiWrap(@NonNull Context context, String source) { final Locale locale = localeFromContext(context); if (TextUtils.getLayoutDirectionFromLocale(locale) == View.LAYOUT_DIRECTION_RTL) { @@ -96,7 +87,12 @@ public final class Formatter { * @return formatted string with the number */ public static String formatFileSize(@Nullable Context context, long sizeBytes) { - return formatFileSize(context, sizeBytes, FLAG_DEFAULT); + if (context == null) { + return ""; + } + final BytesResult res = formatBytes(context.getResources(), sizeBytes, 0); + return bidiWrap(context, context.getString(com.android.internal.R.string.fileSizeSuffix, + res.value, res.units)); } /** @@ -104,191 +100,88 @@ public final class Formatter { * (showing fewer digits of precision). */ public static String formatShortFileSize(@Nullable Context context, long sizeBytes) { - return formatFileSize(context, sizeBytes, FLAG_SHORTER); - } - - private static String formatFileSize(@Nullable Context context, long sizeBytes, int flags) { if (context == null) { return ""; } - final RoundedBytesResult res = RoundedBytesResult.roundBytes(sizeBytes, flags); - return bidiWrap(context, formatRoundedBytesResult(context, res)); - } - - private static String getSuffixOverride(@NonNull Resources res, MeasureUnit unit) { - if (unit == MeasureUnit.BYTE) { - return res.getString(com.android.internal.R.string.byteShort); - } else { // unit == PETABYTE - return res.getString(com.android.internal.R.string.petabyteShort); - } - } - - private static NumberFormat getNumberFormatter(Locale locale, int fractionDigits) { - final NumberFormat numberFormatter = NumberFormat.getInstance(locale); - numberFormatter.setMinimumFractionDigits(fractionDigits); - numberFormatter.setMaximumFractionDigits(fractionDigits); - numberFormatter.setGroupingUsed(false); - if (numberFormatter instanceof DecimalFormat) { - // We do this only for DecimalFormat, since in the general NumberFormat case, calling - // setRoundingMode may throw an exception. - numberFormatter.setRoundingMode(BigDecimal.ROUND_HALF_UP); - } - return numberFormatter; - } - - private static String deleteFirstFromString(String source, String toDelete) { - final int location = source.indexOf(toDelete); - if (location == -1) { - return source; - } else { - return source.substring(0, location) - + source.substring(location + toDelete.length(), source.length()); - } - } - - private static String formatMeasureShort(Locale locale, NumberFormat numberFormatter, - float value, MeasureUnit units) { - final MeasureFormat measureFormatter = MeasureFormat.getInstance( - locale, MeasureFormat.FormatWidth.SHORT, numberFormatter); - return measureFormatter.format(new Measure(value, units)); - } - - private static final UnicodeSetSpanner SPACES_AND_CONTROLS = - new UnicodeSetSpanner(new UnicodeSet("[[:Zs:][:Cf:]]").freeze()); - - private static String formatRoundedBytesResult( - @NonNull Context context, @NonNull RoundedBytesResult input) { - final Locale locale = localeFromContext(context); - final NumberFormat numberFormatter = getNumberFormatter(locale, input.fractionDigits); - if (input.units == MeasureUnit.BYTE || input.units == PETABYTE) { - // ICU spells out "byte" instead of "B", and can't format petabytes yet. - final String formattedNumber = numberFormatter.format(input.value); - return context.getString(com.android.internal.R.string.fileSizeSuffix, - formattedNumber, getSuffixOverride(context.getResources(), input.units)); - } else { - return formatMeasureShort(locale, numberFormatter, input.value, input.units); - } + final BytesResult res = formatBytes(context.getResources(), sizeBytes, FLAG_SHORTER); + return bidiWrap(context, context.getString(com.android.internal.R.string.fileSizeSuffix, + res.value, res.units)); } /** {@hide} */ public static BytesResult formatBytes(Resources res, long sizeBytes, int flags) { - final RoundedBytesResult rounded = RoundedBytesResult.roundBytes(sizeBytes, flags); - final Locale locale = res.getConfiguration().getLocales().get(0); - final NumberFormat numberFormatter = getNumberFormatter(locale, rounded.fractionDigits); - final String formattedNumber = numberFormatter.format(rounded.value); - final String units; - if (rounded.units == MeasureUnit.BYTE || rounded.units == PETABYTE) { - // ICU spells out "byte" instead of "B", and can't format petabytes yet. - units = getSuffixOverride(res, rounded.units); - } else { - // Since ICU does not give us access to the pattern, we need to extract the unit string - // from ICU, which we do by taking out the formatted number out of the formatted string - // and trimming the result of spaces and controls. - final String formattedMeasure = formatMeasureShort( - locale, numberFormatter, rounded.value, rounded.units); - final String numberRemoved = deleteFirstFromString(formattedMeasure, formattedNumber); - units = SPACES_AND_CONTROLS.trim(numberRemoved).toString(); + final boolean isNegative = (sizeBytes < 0); + float result = isNegative ? -sizeBytes : sizeBytes; + int suffix = com.android.internal.R.string.byteShort; + long mult = 1; + if (result > 900) { + suffix = com.android.internal.R.string.kilobyteShort; + mult = 1000; + result = result / 1000; } - return new BytesResult(formattedNumber, units, rounded.roundedBytes); - } - - /** - * 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"); - - /** {@hide} */ - public static class RoundedBytesResult { - public final float value; - public final MeasureUnit units; - public final int fractionDigits; - public final long roundedBytes; - - private RoundedBytesResult( - float value, MeasureUnit units, int fractionDigits, long roundedBytes) { - this.value = value; - this.units = units; - this.fractionDigits = fractionDigits; - this.roundedBytes = roundedBytes; + if (result > 900) { + suffix = com.android.internal.R.string.megabyteShort; + mult *= 1000; + result = result / 1000; } - - /** - * 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) { - final boolean isNegative = (sizeBytes < 0); - float result = isNegative ? -sizeBytes : sizeBytes; - MeasureUnit units = MeasureUnit.BYTE; - long mult = 1; - if (result > 900) { - units = MeasureUnit.KILOBYTE; - mult = 1000; - result = result / 1000; - } - if (result > 900) { - units = MeasureUnit.MEGABYTE; - mult *= 1000; - result = result / 1000; - } - if (result > 900) { - units = MeasureUnit.GIGABYTE; - mult *= 1000; - result = result / 1000; - } - if (result > 900) { - units = MeasureUnit.TERABYTE; - mult *= 1000; - result = result / 1000; - } - if (result > 900) { - units = PETABYTE; - mult *= 1000; - result = result / 1000; - } - // Note we calculate the rounded long by ourselves, but still let NumberFormat compute - // the rounded value. NumberFormat.format(0.1) might not return "0.1" due to floating - // point errors. - final int roundFactor; - final int roundDigits; - if (mult == 1 || result >= 100) { - roundFactor = 1; - roundDigits = 0; - } else if (result < 1) { + if (result > 900) { + suffix = com.android.internal.R.string.gigabyteShort; + mult *= 1000; + result = result / 1000; + } + if (result > 900) { + suffix = com.android.internal.R.string.terabyteShort; + mult *= 1000; + result = result / 1000; + } + if (result > 900) { + suffix = com.android.internal.R.string.petabyteShort; + mult *= 1000; + result = result / 1000; + } + // Note we calculate the rounded long by ourselves, but still let String.format() + // compute the rounded value. String.format("%f", 0.1) might not return "0.1" due to + // floating point errors. + final int roundFactor; + final String roundFormat; + if (mult == 1 || result >= 100) { + roundFactor = 1; + roundFormat = "%.0f"; + } else if (result < 1) { + roundFactor = 100; + roundFormat = "%.2f"; + } else if (result < 10) { + if ((flags & FLAG_SHORTER) != 0) { + roundFactor = 10; + roundFormat = "%.1f"; + } else { roundFactor = 100; - roundDigits = 2; - } else if (result < 10) { - if ((flags & FLAG_SHORTER) != 0) { - roundFactor = 10; - roundDigits = 1; - } else { - roundFactor = 100; - roundDigits = 2; - } - } else { // 10 <= result < 100 - if ((flags & FLAG_SHORTER) != 0) { - roundFactor = 1; - roundDigits = 0; - } else { - roundFactor = 100; - roundDigits = 2; - } + roundFormat = "%.2f"; } - - if (isNegative) { - result = -result; + } else { // 10 <= result < 100 + if ((flags & FLAG_SHORTER) != 0) { + roundFactor = 1; + roundFormat = "%.0f"; + } else { + roundFactor = 100; + roundFormat = "%.2f"; } - - // Note this might overflow if abs(result) >= Long.MAX_VALUE / 100, but that's like - // 80PB so it's okay (for now)... - final long roundedBytes = - (flags & FLAG_CALCULATE_ROUNDED) == 0 ? 0 - : (((long) Math.round(result * roundFactor)) * mult / roundFactor); - - return new RoundedBytesResult(result, units, roundDigits, roundedBytes); } + + if (isNegative) { + result = -result; + } + final String roundedString = String.format(roundFormat, result); + + // Note this might overflow if abs(result) >= Long.MAX_VALUE / 100, but that's like 80PB so + // it's okay (for now)... + final long roundedBytes = + (flags & FLAG_CALCULATE_ROUNDED) == 0 ? 0 + : (((long) Math.round(result * roundFactor)) * mult / roundFactor); + + final String units = res.getString(suffix); + + return new BytesResult(roundedString, units, roundedBytes); } /** diff --git a/core/res/res/values/strings.xml b/core/res/res/values/strings.xml index 5783435e13bde..c54f79983f024 100644 --- a/core/res/res/values/strings.xml +++ b/core/res/res/values/strings.xml @@ -20,13 +20,23 @@ B + + kB + + MB + + GB + + TB PB - - %1$s %2$s + + %1$s %2$s diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 8e391d3c0122a..4b758bea8820e 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -705,6 +705,7 @@ + @@ -760,6 +761,7 @@ + @@ -779,6 +781,7 @@ + @@ -981,6 +984,7 @@ +