Fix i18n number formats in tray power displays

base/i18n/time_formatting.cc

Index: base/i18n/time_formatting.cc
diff --git a/base/i18n/time_formatting.cc b/base/i18n/time_formatting.cc
index 666abd44dd6c03decc9ad5fce4228401fa31a31f..63bee433ff78e434c9a73755b08d9d5f4f78ceb3 100644
--- a/base/i18n/time_formatting.cc
+++ b/base/i18n/time_formatting.cc
@@ -13,6 +13,8 @@
 #include "base/time/time.h"
 #include "third_party/icu/source/i18n/unicode/datefmt.h"
 #include "third_party/icu/source/i18n/unicode/dtptngen.h"
+#include "third_party/icu/source/i18n/unicode/fmtable.h"
+#include "third_party/icu/source/i18n/unicode/measfmt.h"
 #include "third_party/icu/source/i18n/unicode/smpdtfmt.h"
 
 namespace base {
@@ -68,6 +70,17 @@ icu::SimpleDateFormat CreateSimpleDateFormatter(const char* pattern) {
   return formatter;
 }
 
+UMeasureFormatWidth DurationWidthToMeasureWidth(DurationFormatWidth width) {
+  switch (width) {
+    case DURATION_WIDTH_WIDE: return UMEASFMT_WIDTH_WIDE;
+    case DURATION_WIDTH_SHORT: return UMEASFMT_WIDTH_SHORT;
+    case DURATION_WIDTH_NARROW: return UMEASFMT_WIDTH_NARROW;
+    case DURATION_WIDTH_NUMERIC: return UMEASFMT_WIDTH_NUMERIC;
+  }
+  NOTREACHED();
+  return UMEASFMT_WIDTH_COUNT;
+}
+
 }  // namespace
 
 string16 TimeFormatTimeOfDay(const Time& time) {
@@ -140,6 +153,27 @@ string16 TimeFormatFriendlyDate(const Time& time) {
   return TimeFormat(formatter.get(), time);
 }
 
+string16 TimeDurationFormat(const TimeDelta& time,
+                            const DurationFormatWidth width) {
+  UErrorCode status = U_ZERO_ERROR;
+  const int total_minutes = static_cast<int>(time.InSecondsF() / 60 + 0.5);
+  // Using doubles instead of ints, and the 0.0001, causes formatMeasures() to
+  // write "1:00", instead of "1h", when using NUMERIC width.

[Greg Levin, 2016/05/11 19:43:02]

Yeah, this is hacky.  I'd rather have "1:00" than "1h" if possible.  Apparently
in Romanian (for example), "remaining" has singular and plural forms.  "1:00" is
arguably plural, which is how our string will be translated; "1h" is singular.
But I'm not dead set on this, and open to other ideas.

[jungshik at Google, 2016/05/11 21:11:45]

A potential issue with plural  is why I just left alone a not-so-pretty (but
more likely to be correct in terms of plural) ui/base/l10n/time_format instead
of converting to use measure format.  You might as well  consider extending that
API to support 'numeric' (2:07 instead of 2hrs and 7 mins and other forms). 

[Greg Levin, 2016/05/12 19:31:44]

I looked at it awhile, and will give it a shot if you like, but I'm not
convinced it fits in the current architecture:

1) It would only support [FORMAT_DURATION][LENGTH_NUMERIC], just as two-unit
times are currently only supported in DURATION (to avoid confusing pluralization
issues with "remaining" / "elapsed"?).

2) Formatter uses MessageFormat instead of MeasureFormat.  I'm not that familiar
with these classes, and how the interact / interchange.  I don't know if
MessageFormat would do what I currently have MeasureFormat doing, and/or how I
would easily rewrite Formatter to use both.

One easier option would be to add a third function to TimeFormat:

  base::string16 TimeFormat::Numeric(const base::TimeDelta& delta);

There would be no Format arg (and Length = LENGTH_NUMERIC is built in).  It
wouldn't deal with days at all (just large hour counts if needed), and output
H:MM:SS if SS != 0.  I'd mostly just copy my existing TimeDurationFormat() into
TimeFormat::Numeric(), and not use the Formatter class at all.  Let me know if
that makes more sense, API-wise, and I'll move it.

Except... it would need the DurationFormatWidth arg, because
battery_notification.cc needs the DURATION_WIDTH_NARROW ("3h 7m") format.  So
maybe call it Duration() instead?  But at that point, we would have

Duration(DURATION_WIDTH_WIDE, delta) ->
  "3 hours, 7 minutes"

Detailed(FORMAT_DURATION, LENGTH_LONG, -1, delta) ->
  "3 hours and 7 minutes"

which is weird.  I could just expose only the NARROW and NUMERIC options.  I
dunno... what do you think?  I'm inclined to leave as is if you don't have a
strong opinion.

[jungshik at Google, 2016/05/13 23:00:35]

Yeah... let's just leave it as it is for now. 

ICU has a separate Duration Format (but it's deprecated IIRC), but it'd not help
much (especially getting the inflection right around the duration ('left',
'until full; hope this one does not inflect differently in any language but well
languages are interesting', 'remaining' etc) because basically translators
wouldn't know what hour/minutes are in 'duration part' and cannot translate
separately. 

Using plural support in MessageFormat is one way, but when it comes to 'numeric'
forms, it's kinda odd, too (anyway, it's possible in theory as I suggested
earlier). 

+  double hours = total_minutes / 60;
+  double minutes = total_minutes % 60 + 0.0001;
+  UMeasureFormatWidth u_width = DurationWidthToMeasureWidth(width);
+
+  const icu::Measure measures[] = {
+      icu::Measure(hours, icu::MeasureUnit::createHour(status), status),
+      icu::Measure(minutes, icu::MeasureUnit::createMinute(status), status)};
+  icu::MeasureFormat measure_format(icu::Locale::getDefault(), u_width, status);
+  icu::UnicodeString formatted;
+  icu::FieldPosition ignore(icu::FieldPosition::DONT_CARE);
+  measure_format.formatMeasures(measures, minutes == 0 ? 1 : 2, formatted,

[jungshik at Google, 2016/05/11 21:11:45]

If you always use '2' for the 2nd argument, do you still have a problem of
getting '1h' instead of '1:00' ?  I guess not. 

[Greg Levin, 2016/05/12 19:31:44]

<head desk>  S'what I get for not reading carefully the code I copy-and-paste. 
Fixed.

+                                ignore, status);
+  return base::string16(formatted.getBuffer(), formatted.length());
+}
+
 HourClockType GetHourClockType() {
   // TODO(satorux,jshin): Rework this with ures_getByKeyWithFallback()
   // once it becomes public. The short time format can be found at