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

chrome/common/time_format_unittest.cc

Index: chrome/common/time_format_unittest.cc
diff --git a/chrome/common/time_format_unittest.cc b/chrome/common/time_format_unittest.cc
index 31976e3bd40838f24bf9f31e7429f12b64f3b351..3a2004891783f4c0581aacb29faba63d08670722 100644
--- a/chrome/common/time_format_unittest.cc
+++ b/chrome/common/time_format_unittest.cc
@@ -2,9 +2,11 @@
 // 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"
@@ -14,6 +16,27 @@
 using base::Time;
 using base::TimeDelta;
 
+namespace {
+
+// A helper class that gets the current locale on construction, and restores
+// it on destruction.
+class ScopedChangeLocale {

[Paweł Hajdan Jr., 2011/05/11 19:10:44]

Sorry for not finding it earlier (I tried, but failed), but now I found an
existing class which does more or less the same:
http://codesearch.google.com/codesearch/p#OAMlx_jo-ck/src/base/sys_string_con...

The code using it is in base/ so I'd suggest extracting it to base/test.

+ public:
+  ScopedChangeLocale() {
+    stored_locale_ = setlocale(LC_ALL, NULL);
+  }
+  virtual ~ScopedChangeLocale() {
+    setlocale(LC_ALL, stored_locale_);
+  }
+
+ private:
+  const char *stored_locale_;

[Paweł Hajdan Jr., 2011/05/11 19:10:44]

nit: Star on the wrong side, should be const char* stored_locale_;

+
+  DISALLOW_COPY_AND_ASSIGN(ScopedChangeLocale);
+};
+
+} // namespace

[Paweł Hajdan Jr., 2011/05/11 19:10:44]

nit: Two spaces between "}" and "// namespace".

+
 TEST(TimeFormat, RelativeDate) {
   Time now = Time::Now();
   string16 today_str = TimeFormat::RelativeDate(now, NULL);
@@ -69,3 +92,31 @@ TEST(TimeFormat, FormatTime) {
   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
+
+  ScopedChangeLocale scoped_locale;
+
+  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]);
+    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;
+}