base: Make TimeDurationFormat* report failures.

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 ea62a34f51618cd65db89a97db7ac1057dfa2b7d..720920558268fa969976c214138e4c81290e678f 100644
--- a/base/i18n/time_formatting.cc
+++ b/base/i18n/time_formatting.cc
@@ -176,26 +176,49 @@ string16 TimeFormatWithPattern(const Time& time, const char* pattern) {
   return TimeFormat(&formatter, time);
 }
 
-string16 TimeDurationFormat(const TimeDelta time,
-                            const DurationFormatWidth width) {
+bool TimeDurationFormat(const TimeDelta time,
+                        const DurationFormatWidth width,
+                        string16* out) {
+  DCHECK(out);
   UErrorCode status = U_ZERO_ERROR;
   const int total_minutes = static_cast<int>(time.InSecondsF() / 60 + 0.5);
   const int hours = total_minutes / 60;
   const int minutes = total_minutes % 60;
   UMeasureFormatWidth u_width = DurationWidthToMeasureWidth(width);
 
+  // TODO(derat): Delete the |status| checks and LOG(ERROR) calls throughout
+  // this function once the cause of http://crbug.com/677043 is tracked down.
   const icu::Measure measures[] = {
       icu::Measure(hours, icu::MeasureUnit::createHour(status), status),
       icu::Measure(minutes, icu::MeasureUnit::createMinute(status), status)};
+  if (U_FAILURE(status)) {
+    LOG(ERROR) << "Creating MeasureUnit or Measure failed for " << hours << "h"
+               << minutes << "m: " << u_errorName(status);
+    return false;
+  }
+
   icu::MeasureFormat measure_format(icu::Locale::getDefault(), u_width, status);
+  if (U_FAILURE(status)) {
+    LOG(ERROR) << "Creating MeasureFormat failed: " << u_errorName(status);
+    return false;
+  }
+
   icu::UnicodeString formatted;
   icu::FieldPosition ignore(icu::FieldPosition::DONT_CARE);
   measure_format.formatMeasures(measures, 2, formatted, ignore, status);
-  return base::string16(formatted.getBuffer(), formatted.length());
+  if (U_FAILURE(status)) {
+    LOG(ERROR) << "formatMeasures failed: " << u_errorName(status);
+    return false;
+  }
+
+  *out = base::string16(formatted.getBuffer(), formatted.length());
+  return true;
 }
 
-string16 TimeDurationFormatWithSeconds(const TimeDelta time,
-                                       const DurationFormatWidth width) {
+bool TimeDurationFormatWithSeconds(const TimeDelta time,
+                                   const DurationFormatWidth width,
+                                   string16* out) {
+  DCHECK(out);
   UErrorCode status = U_ZERO_ERROR;
   const int64_t total_seconds = static_cast<int>(time.InSecondsF() + 0.5);
   const int hours = total_seconds / 3600;
@@ -211,7 +234,8 @@ string16 TimeDurationFormatWithSeconds(const TimeDelta time,
   icu::UnicodeString formatted;
   icu::FieldPosition ignore(icu::FieldPosition::DONT_CARE);
   measure_format.formatMeasures(measures, 3, formatted, ignore, status);
-  return base::string16(formatted.getBuffer(), formatted.length());
+  *out = base::string16(formatted.getBuffer(), formatted.length());
+  return U_SUCCESS(status);

[Greg Levin, 2017/03/06 23:43:21]

I think this UBool -> bool conversion (well, the previous one, technically) was
the source of the win_* compile failures in Patch #2.

e:\b\c\b\win\src\base\i18n\time_formatting.cc(197): error C2220: warning treated
as error - no 'object' file generated
e:\b\c\b\win\src\base\i18n\time_formatting.cc(197): warning C4800: 'UBool':
forcing value to bool 'true' or 'false' (performance warning)
e:\b\c\b\win\src\base\i18n\time_formatting.cc(220): warning C4800: 'UBool':
forcing value to bool 'true' or 'false' (performance warning)

I'm guessing this'll need to be fixed...?

[Daniel Erat, 2017/03/06 23:50:56]

ah, thanks; i didn't look at that failure closely enough, apparently. sigh,
ICU...

 }
 
 string16 DateIntervalFormat(const Time& begin_time,