[payment] display mixed currency in item list and shipping optiopns. When there is more than one cu…

chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
index 954e4c925a3f57b49783aea735349a2e87c5736d..cdd65578c13b3cbcca9b3a2a2eb3a93974602734 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
@@ -54,8 +54,10 @@ import org.chromium.content_public.browser.RenderFrameHost;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.WebContentsStatics;
 import org.chromium.mojo.system.MojoException;
+import org.chromium.mojo.system.Pair;
 import org.chromium.payments.mojom.CanMakePaymentQueryResult;
 import org.chromium.payments.mojom.PaymentComplete;
+import org.chromium.payments.mojom.PaymentCurrencyAmount;
 import org.chromium.payments.mojom.PaymentDetails;
 import org.chromium.payments.mojom.PaymentDetailsModifier;
 import org.chromium.payments.mojom.PaymentErrorReason;
@@ -319,7 +321,7 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     private boolean mMerchantSupportsAutofillPaymentInstruments;
     private ContactEditor mContactEditor;
     private boolean mHasRecordedAbortReason;
-    private CurrencyFormatter mCurrencyFormatter;
+    private Map<Pair<String, String>, CurrencyFormatter> mCurrencyFormatterMap;
     private TabModelSelector mObservedTabModelSelector;
     private TabModel mObservedTabModel;
 
@@ -388,15 +390,19 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
 
         if (sCanMakePaymentQueries == null) sCanMakePaymentQueries = new ArrayMap<>();
 
+        mCurrencyFormatterMap = new HashMap<>();
+
         recordSuccessFunnelHistograms("Initiated");
     }
 
     @Override
     protected void finalize() throws Throwable {
         super.finalize();
-        if (mCurrencyFormatter != null) {
-            // Ensures the native implementation of currency formatter does not leak.
-            mCurrencyFormatter.destroy();
+        for (CurrencyFormatter formatter : mCurrencyFormatterMap.values()) {
+            if (formatter != null) {

[gogerald1, 2017/05/02 18:57:32]

'formatter' should never be null, instead of 'if' statement, please do 'assert
formatter != null'

[wuandy, 2017/05/05 15:04:37]

Done.

+                // Ensures the native implementation of currency formatter does not leak.
+                formatter.destroy();
+            }
         }
     }
 
@@ -775,28 +781,24 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             return false;
         }
 
-        if (mCurrencyFormatter == null) {
-            mCurrencyFormatter = new CurrencyFormatter(details.total.amount.currency,
-                    details.total.amount.currencySystem, Locale.getDefault());
-        }
-
         if (details.total != null) {
             mRawTotal = details.total;
         }
 
+        preloadRequiredCurrencyFormatters(details);

[gogerald1, 2017/05/02 18:57:32]

'preload' usually means you no need it right now and this function is also
called by updateWith(). So may be just 'LoadCurrencyFormatterForPaymentDetails'?

[wuandy, 2017/05/05 15:04:37]

Done.

         if (mRawTotal != null) {
             // Total is never pending.
-            LineItem uiTotal = new LineItem(mRawTotal.label,
-                    mCurrencyFormatter.getFormattedCurrencyCode(),
-                    mCurrencyFormatter.format(mRawTotal.amount.value), /* isPending */ false);
+            CurrencyFormatter formatter = getOrCreateCurrencyFormatter(mRawTotal.amount);
+            LineItem uiTotal = new LineItem(mRawTotal.label, formatter.getFormattedCurrencyCode(),
+                    formatter.format(mRawTotal.amount.value), /* isPending */ false);
 
-            List<LineItem> uiLineItems = getLineItems(details.displayItems, mCurrencyFormatter);
+            List<LineItem> uiLineItems = getLineItems(details.displayItems);
 
             mUiShoppingCart = new ShoppingCart(uiTotal, uiLineItems);
         }
         mRawLineItems = Collections.unmodifiableList(Arrays.asList(details.displayItems));
 
-        mUiShippingOptions = getShippingOptions(details.shippingOptions, mCurrencyFormatter);
+        mUiShippingOptions = getShippingOptions(details.shippingOptions);
 
         for (int i = 0; i < details.modifiers.length; i++) {
             PaymentDetailsModifier modifier = details.modifiers[i];
@@ -823,7 +825,8 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             PaymentDetailsModifier modifier = getModifier(instrument);
             instrument.setModifiedTotal(modifier == null || modifier.total == null
                             ? null
-                            : mCurrencyFormatter.format(modifier.total.amount.value));
+                            : getOrCreateCurrencyFormatter(modifier.total.amount)
+                                      .format(modifier.total.amount.value));
         }
 
         updateOrderSummary((PaymentInstrument) mPaymentMethodsSection.getSelectedItem());
@@ -837,12 +840,11 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         PaymentItem total = modifier == null ? null : modifier.total;
         if (total == null) total = mRawTotal;
 
-        mUiShoppingCart.setTotal(
-                new LineItem(total.label, mCurrencyFormatter.getFormattedCurrencyCode(),
-                        mCurrencyFormatter.format(total.amount.value), false /* isPending */));
-        mUiShoppingCart.setAdditionalContents(modifier == null
-                        ? null
-                        : getLineItems(modifier.additionalDisplayItems, mCurrencyFormatter));
+        CurrencyFormatter formatter = getOrCreateCurrencyFormatter(total.amount);
+        mUiShoppingCart.setTotal(new LineItem(total.label, formatter.getFormattedCurrencyCode(),
+                formatter.format(total.amount.value), false /* isPending */));
+        mUiShoppingCart.setAdditionalContents(
+                modifier == null ? null : getLineItems(modifier.additionalDisplayItems));
         mUI.updateOrderSummarySection(mUiShoppingCart);
     }
 
@@ -858,20 +860,19 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
      * Converts a list of payment items and returns their parsed representation.
      *
      * @param items     The payment items to parse. Can be null.
-     * @param formatter A formatter for the currency amount value.
      * @return A list of valid line items.
      */
-    private static List<LineItem> getLineItems(
-            @Nullable PaymentItem[] items, CurrencyFormatter formatter) {
+    private List<LineItem> getLineItems(@Nullable PaymentItem[] items) {
         // Line items are optional.
         if (items == null) return new ArrayList<>();
 
         List<LineItem> result = new ArrayList<>(items.length);
         for (int i = 0; i < items.length; i++) {
             PaymentItem item = items[i];
-
-            result.add(new LineItem(
-                    item.label, "", formatter.format(item.amount.value), item.pending));
+            CurrencyFormatter formatter = getOrCreateCurrencyFormatter(item.amount);
+            result.add(new LineItem(item.label,
+                    isMixedCurrency() ? formatter.getFormattedCurrencyCode() : "",
+                    formatter.format(item.amount.value), item.pending));
         }
 
         return Collections.unmodifiableList(result);
@@ -881,11 +882,9 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
      * Converts a list of shipping options and returns their parsed representation.
      *
      * @param options   The raw shipping options to parse. Can be null.
-     * @param formatter A formatter for the currency amount value.
      * @return The UI representation of the shipping options.
      */
-    private static SectionInformation getShippingOptions(
-            @Nullable PaymentShippingOption[] options, CurrencyFormatter formatter) {
+    private SectionInformation getShippingOptions(@Nullable PaymentShippingOption[] options) {
         // Shipping options are optional.
         if (options == null || options.length == 0) {
             return new SectionInformation(PaymentRequestUI.TYPE_SHIPPING_OPTIONS);
@@ -895,8 +894,11 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         int selectedItemIndex = SectionInformation.NO_SELECTION;
         for (int i = 0; i < options.length; i++) {
             PaymentShippingOption option = options[i];
+            CurrencyFormatter formatter = getOrCreateCurrencyFormatter(option.amount);
+            String currencyPrefix =
+                    isMixedCurrency() ? formatter.getFormattedCurrencyCode() + " " : "";
             result.add(new PaymentOption(option.id, option.label,
-                    formatter.format(option.amount.value), null));
+                    currencyPrefix + formatter.format(option.amount.value), null));
             if (option.selected) selectedItemIndex = i;
         }
 
@@ -905,6 +907,52 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     }
 
     /**
+     * Load all currency formatter required by given PaymentDetails, such that we know beforehand
+     * if we are dealing with mixed currency or not.
+     */
+    private void preloadRequiredCurrencyFormatters(PaymentDetails details) {
+        if (details.total != null) {
+            getOrCreateCurrencyFormatter(details.total.amount);
+        }

[gogerald1, 2017/05/02 18:57:32]

one space line

[wuandy, 2017/05/05 15:04:37]

Done.

+        if (details.displayItems != null) {
+            for (PaymentItem item : details.displayItems) {

[gogerald1, 2017/05/02 18:57:32]

use 'for(int i = 0; i <....)' loop if possible since it is more efficient on
Android than iterator

[wuandy, 2017/05/05 15:04:37]

Keeping the change as discussed offline.

+                getOrCreateCurrencyFormatter(item.amount);
+            }
+        }

[gogerald1, 2017/05/02 18:57:32]

one space line

[wuandy, 2017/05/05 15:04:37]

Done.

+        if (details.shippingOptions != null) {
+            for (PaymentShippingOption option : details.shippingOptions) {

[gogerald1, 2017/05/02 18:57:32]

use 'for(int i = 0; i <....)' loop if possible since it is more efficient on
Android than iterator

[wuandy, 2017/05/05 15:04:37]

Keeping the change as discussed offline.

+                getOrCreateCurrencyFormatter(option.amount);
+            }
+        }

[gogerald1, 2017/05/02 18:57:32]

one space line

[wuandy, 2017/05/05 15:04:37]

Done.

+        if (details.modifiers != null) {
+            for (PaymentDetailsModifier modifier : details.modifiers) {

[gogerald1, 2017/05/02 18:57:32]

use 'for(int i = 0; i <....)' loop if possible since it is more efficient on
Android than iterator

[wuandy, 2017/05/05 15:04:37]

Keeping the change as discussed offline.

+                getOrCreateCurrencyFormatter(modifier.total.amount);
+                for (PaymentItem item : modifier.additionalDisplayItems) {

[gogerald1, 2017/05/02 18:57:32]

use 'for(int i = 0; i <....)' loop if possible since it is more efficient on
Android than iterator

[wuandy, 2017/05/05 15:04:37]

Keeping the change as discussed offline.

+                    getOrCreateCurrencyFormatter(item.amount);
+                }
+            }
+        }
+    }
+
+    private boolean isMixedCurrency() {
+        return mCurrencyFormatterMap.size() > 1;
+    }
+
+    /**
+     * Look for a formatter for a given PaymentCurrencyAmount, create one if no existing instance
+     * is found.
+     */
+    private CurrencyFormatter getOrCreateCurrencyFormatter(PaymentCurrencyAmount amount) {
+        Pair<String, String> key = new Pair<>(amount.currency, amount.currencySystem);

[gogerald1, 2017/05/02 18:57:32]

We only need an unique identifier here, so "amount.currency +
amount.currencySystem" should be good enough,

[wuandy, 2017/05/05 15:04:37]

Done.

+        if (!mCurrencyFormatterMap.containsKey(key)) {
+            mCurrencyFormatterMap.put(key,

[gogerald1, 2017/05/02 18:57:32]

when this could happen? PaymentDetails only change during init and updateWith, 

+                    new CurrencyFormatter(
+                            amount.currency, amount.currencySystem, Locale.getDefault()));
+        }
+        return mCurrencyFormatterMap.get(key);
+    }
+
+    /**
      * Called to retrieve the data to show in the initial PaymentRequest UI.
      */
     @Override