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;
 
+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);
   EXPECT_EQ(ASCIIToUTF16("Today"), today_str);
 
   Time yesterday = now - TimeDelta::FromDays(1);
   string16 yesterday_str = TimeFormat::RelativeDate(yesterday, NULL);
   EXPECT_EQ(ASCIIToUTF16("Yesterday"), yesterday_str);
 
   Time two_days_ago = now - TimeDelta::FromDays(2);
@@ skipping 35 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
+
+  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;
+}