[Payments] Currency formatter for order amounts.

components/payments/currency_formatter.cc

+// Copyright 2017 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 "components/payments/currency_formatter.h"
+
+#include <memory>
+
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "third_party/icu/source/common/unicode/unistr.h"
+#include "third_party/icu/source/common/unicode/utypes.h"
+#include "third_party/icu/source/i18n/unicode/numfmt.h"
+
+namespace payments {
+
+namespace {
+
+// Max currency code length. Length of currency code can be at most 2048.
+const static size_t kMaxCurrencyCodeLength = 2048;
+
+}  // namespace
+
+const std::string FormatAmountForCurrency(const std::string& amount,
+                                          const std::string& currency_code,
+                                          const icu::Locale& locale) {
+  // Lightweight validation first, checking for basic parameter validity.
+  if (currency_code.empty() || currency_code.size() > kMaxCurrencyCodeLength) {
+    LOG(ERROR) << "Invalid currency code length: " << currency_code;
+    return amount;
+  }
+  double amount_double = 0.0;

[please use gerrit instead, 2017/01/11 18:29:30]

Do you think it may be better to not parse the "amount" into a double, because
the "amount" can have a huge number of digits, up to 30 in some cases? I wonder
if an overflow can occur.

[Mathieu, 2017/01/11 22:22:25]

I would think base::StringToDouble does the right thing. For example there is
one test case that I copied from Java that has a lot of digits. It's parsed up
to the precision that double supports. We have to have a double because that's
what ICU needs...

+  if (!base::StringToDouble(amount, &amount_double)) {
+    LOG(ERROR) << "Could not convert currency amount to double: " << amount;
+    return amount;
+  }
+
+  UErrorCode ignored = U_ZERO_ERROR;
+  std::unique_ptr<icu::NumberFormat> fmt(
+      icu::NumberFormat::createCurrencyInstance(locale, ignored));

[please use gerrit instead, 2017/01/11 18:29:30]

Why ignore the error?

[Mathieu, 2017/01/11 22:22:25]

a null result is ultimately returned when there is an error, but indeed checking
for failure is more consistent with the API.

+  if (!fmt) {
+    icu::UnicodeString name;
+    std::string locale_name;
+    locale.getDisplayName(name).toUTF8String(locale_name);
+    LOG(ERROR) << "Failed to initialize the currency formatter for "
+               << locale_name;
+    return amount;
+  }
+
+  icu::UnicodeString u_currency_code(currency_code.c_str(),
+                                     currency_code.length());
+  UErrorCode error_code = U_ZERO_ERROR;
+  fmt->setCurrency(u_currency_code.getBuffer(), error_code);
+  if (U_FAILURE(error_code)) {
+    LOG(ERROR) << "Could not set currency on amount formatter: "

[please use gerrit instead, 2017/01/11 18:29:30]

Will print an error for non-ISO currency codes? An example of a non-ISO currency
code is "BITCOIN". These are allowed by the spec, so it's not technically an
error. Perhaps you could check whether currency_code matches ^[A-Z]{3}$ regex
beforehand? src/third_party/re2 should be able to do that efficiently.

[Mathieu, 2017/01/11 22:22:26]

I've changed currency code handling. This function/class will no longer early
return for odd currency codes such as "BITCOIN". The number will be formatted
after all. I think this achieves what you need. Validation of the actual code
structure (^[A-Z]{3}$) is done in the
PaymentsValidators::isValidCurrencyCodeFormat which this class assumes was run.

+               << currency_code;
+    return amount;
+  }
+
+  icu::UnicodeString output;
+  fmt->format(amount_double, output);
+
+  if (!output.isEmpty()) {

[please use gerrit instead, 2017/01/11 18:29:30]

If the output is empty, you should probably return |amount| as well, right?

[Mathieu, 2017/01/11 22:22:25]

Done. fair enough!

+    // Explicitely removes the currency code (truncated to its 3-letter and

[please use gerrit instead, 2017/01/11 18:29:30]

Explicitly

[Mathieu, 2017/01/11 22:22:26]

Done.

+    // 2-letter versions) from the output, because the payments implementation
+    // explicitely displays it alongside this result.
+    //
+    // 3+ letters: If currency code is "ABCDEF" or "BTX", this code will
+    // transform
+    // "ABC55.00"/"BTX55.00" to "55.00".
+    // 2 letters: If currency code is "CAD", this code will transform "CA$55.00"
+    // to "$55.00" (en_US) or "55,00 $ CA" to "55,00 $" (fr_FR).
+    u_currency_code.truncate(3);
+    output.findAndReplace(u_currency_code, "");
+    u_currency_code.truncate(2);
+    output.findAndReplace(u_currency_code, "");
+    // Trims any unicode whitespace (including non-breaking space).

[please use gerrit instead, 2017/01/11 18:29:30]

base::TrimeWhitespace?

[Mathieu, 2017/01/11 22:22:26]

Does not handle unicode non-breaking space trimming. Ultimately this is why I'm
doing all the findAndReplace and manual trimming with a UnicodeString and only
converting to UTF8/UTF16 at the end. 

+    if (u_isUWhiteSpace(output[0])) {
+      output.remove(0, 1);
+    }
+    if (u_isUWhiteSpace(output[output.length() - 1])) {
+      output.remove(output.length() - 1, 1);
+    }
+  }
+
+  std::string output_str;
+  output.toUTF8String(output_str);
+  return output_str;
+}
+
+}  // namespace payments