Added a test checking for TimeFormat:: producing NaN, rolled icu version

chrome/common/time_format_unittest.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <locale.h>
 #include <time.h>
 
 #include "base/basictypes.h"
+#include "base/logging.h"
 #include "base/string16.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/time_format.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::Time;
 using base::TimeDelta;
 
 TEST(TimeFormat, RelativeDate) {
@@ skipping 44 matching lines @@
   TestTimeFormats(one_sec + twohundred_millisecs, "1 sec");
   TestTimeFormats(five_secs + twohundred_millisecs, "5 secs");
   TestTimeFormats(one_min + five_secs, "1 min");
   TestTimeFormats(three_mins + twohundred_millisecs, "3 mins");
   TestTimeFormats(one_hour + five_secs, "1 hour");
   TestTimeFormats(four_hours + five_secs, "4 hours");
   TestTimeFormats(one_day + five_secs, "1 day");
   TestTimeFormats(three_days, "3 days");
   TestTimeFormats(three_days + four_hours, "3 days");
 }
+
+TEST(TimeFormat, FAILS_DecimalPointNotDot) {
+  // Some locales use a comma ',' instead of a dot '.' as the separator for
+  // decimal digits. The icu library wasn't handling this, leading to "1"
+  // being internally converted to "+1,0e00" and ultimately leading to "NaN".
+  // This showed up on the browser on estimated download time, for example.
+  // http://crbug.com/60476
+
+  const char *locales[] = { "fr_FR", "fr_FR.utf8", "fr_FR.UTF-8" };
+  const char *locale;
+  for (uint32 i = 0; i < arraysize(locales); ++i) {
+    locale = setlocale(LC_ALL, locales[i]);

[Paweł Hajdan Jr., 2011/05/11 14:47:13]

And now all following tests run with the changed locale. Ideally we should have
something like ScopedChangeLocale to make sure it is always set back to the
original value.

[Joao da Silva, 2011/05/11 15:35:07]

Yikes, true. Fixed by adding a ScopedChangeLocale.
Does it make sense to have that class available to other tests? In that case,
where should it be placed?

+    if (locale != NULL)
+      break;
+  }
+
+  if (locale == NULL) {
+    LOG(WARNING) << "Skipping test because machine doesn't have the fr_FR "
+                 << "locale installed.";
+    return;
+  }
+
+  string16 one_min = TimeFormat::TimeRemainingShort(TimeDelta::FromMinutes(1));
+  EXPECT_EQ(ASCIIToUTF16("1 min"), one_min) << "fr_FR locale generates "
+                                            << one_min;
+}