align shipping option name to left and shipping cost to right on bottom sheet

chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
index 128dd59c415dc9290f13ae57872c3435b927e9cd..f49e2199daf0e377729f25bee96db9cc3b3619dd 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
@@ -935,7 +935,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
                     ApiCompatibilityUtils.setTextAppearance(labelView, isEnabled
                             ? R.style.PaymentsUiSectionDefaultText
                             : R.style.PaymentsUiSectionDisabledText);
-                    labelView.setText(convertOptionToString(mOption,
+                    labelView.setText(convertOptionToString(mOption, false, /* excludeMainLabel */
                             mDelegate.isBoldLabelNeeded(OptionSection.this),
                             false /* singleLine */));
                     labelView.setEnabled(isEnabled);
@@ -1061,6 +1061,10 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
          */
         boolean mSetDisplaySummaryInSingleLineInNormalMode = true;
 
+        /** Indicates whether the summary should be split to display in left and right summary
+         * text views in {@link PaymentRequestSection.DISPLAY_MODE_NORMAL}. */

[gogerald1, 2017/06/02 15:05:10]

This is not a public interface or variable, so you can replace "{@link
PaymentRequestSection.DISPLAY_MODE_NORMAL}" with "DISPLAY_MODE_NORMAL".

In addition, if the comments have multiple lines, put '/**' and '*/' in a
separate line without any text in that line, like below.

/**
 * Indicates whether the summary should be split to display in the left and
right summary
 * text views in DISPLAY_MODE_NORMAL.
 */

[wuandy1, 2017/06/02 19:53:12]

Done.

+        private boolean mSplitSummaryInNormalMode;

[gogerald1, 2017/06/02 15:05:10]

looks 'mSplitSummaryInDisplayModeNormal' is better, 'DisplayModeNormal'
corresponds to 'DISPLAY_MODE_NORMAL'

[wuandy1, 2017/06/02 19:53:12]

Done.

+
         /** Indicates whether the summary is set to R.style.PaymentsUiSectionDescriptiveText. */
         private boolean mSummaryInDescriptiveText;
 
@@ -1173,6 +1177,16 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
             mSetDisplaySummaryInSingleLineInNormalMode = singleLine;
         }
 
+        /**
+         * @param splitSummary set to true to split the display of summary in left and right

[gogerald1, 2017/06/02 15:05:10]

Uppercase the first character to comment a param. In addition, the your comments
is for the param but not the function, so "set to true to...." looks a little
confusing. You can follow below pattern.

/**
 * Function comments.....
 *
 * @param splitSummary If true, ....... Otherwise...
 */

[wuandy1, 2017/06/02 19:53:12]

Done.

+         *                     text views in {@link DISPLAY_MODE_NORMAL}. Otherwise the entire
+         *                     summary is displayed in left text view. Note that summary is
+         *                     split into 'label' and the rest('sublabel', 'Tertiary label').
+         */
+        public void setSplitSummaryInNormalMode(boolean splitSummary) {

[gogerald1, 2017/06/02 15:05:10]

setSplitSummaryInDisplayNormalMode?

[wuandy1, 2017/06/02 19:53:12]

Done.

+            mSplitSummaryInNormalMode = splitSummary;
+        }
+
         /** Updates the View to account for the new {@link SectionInformation} being passed in. */
         public void update(SectionInformation information) {
             mSectionInformation = information;
@@ -1290,9 +1304,19 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
                             getSummaryLeftTextView(), R.style.PaymentsUiSectionDefaultText);
                     mSummaryInDescriptiveText = false;
                 }
-                setSummaryText(convertOptionToString(selectedItem, false /* useBoldLabel */,
-                                       mSummaryInSingleLine),
-                        null);
+                // Align main label to the left and sub label to the right if caller specified and
+                // section is in DISPLAY_MODE_NORMAL

[gogerald1, 2017/06/02 15:05:10]

// Split summary in DISPLAY_MODE_NORMAL if caller specified. The first part is
displayed on the left summary text view aligned to the left. The second part is
displayed on the right summary text view aligned to the right.

[wuandy1, 2017/06/02 19:53:12]

Done.

+                boolean splitSummary =
+                        mSplitSummaryInNormalMode && (mDisplayMode == DISPLAY_MODE_NORMAL);
+                if (splitSummary) {
+                    setSummaryText(selectedItem.getLabel(),
+                            convertOptionToString(selectedItem, true /* excludeMainLabel */,
+                                    false /* useBoldLabel */, mSummaryInSingleLine));
+                } else {
+                    setSummaryText(convertOptionToString(selectedItem, false /* excludeMainLabel */,
+                                           false /* useBoldLabel */, mSummaryInSingleLine),
+                            null);
+                }
             }
 
             updateControlLayout();
@@ -1347,17 +1371,19 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
             }
         }
 
-        private CharSequence convertOptionToString(
-                PaymentOption item, boolean useBoldLabel, boolean singleLine) {
-            SpannableStringBuilder builder = new SpannableStringBuilder(item.getLabel());
-            String labelSeparator = singleLine
-                    ? getContext().getString(R.string.autofill_address_summary_separator)
-                    : "\n";
+        private CharSequence convertOptionToString(PaymentOption item, boolean excludeMainLabel,
+                boolean useBoldLabel, boolean singleLine) {
+            SpannableStringBuilder builder = new SpannableStringBuilder();
+            if (!excludeMainLabel) builder.append(item.getLabel());
+
             if (useBoldLabel) {

[gogerald1, 2017/06/02 15:05:10]

useBoldLabel is used for main label only

[wuandy1, 2017/06/02 19:53:12]

I think this is something better left to the callers to this function to decide.
This 
function as a utility should only provide the options available, while the
caller can
set useBoldLabel=false while excludeMainLabel=true accordingly.

[gogerald1, 2017/06/06 16:55:19]

Yes, but previously useBoldLabel is only apply for item.getLabel, we should keep
this fact and let caller do it or not. However after your change, the span will
be applied on an empty string sometimes, that doesn't make sense, so you might
want to move this logic inside the if(!excludeMainLabel) check.

[wuandy1, 2017/06/07 01:58:04]

absolutely! good catch!

                 builder.setSpan(
                         new StyleSpan(android.graphics.Typeface.BOLD), 0, builder.length(), 0);
             }
 
+            String labelSeparator = singleLine
+                    ? getContext().getString(R.string.autofill_address_summary_separator)
+                    : "\n";
             if (!TextUtils.isEmpty(item.getSublabel())) {
                 if (builder.length() > 0) builder.append(labelSeparator);
                 builder.append(item.getSublabel());
@@ -1396,10 +1422,18 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
          * Returns the label of the section summary.
          */
         @VisibleForTesting
-        public TextView getSummaryLabelForTest() {
+        public TextView getLeftSummaryLabelForTest() {
             return getSummaryLeftTextView();
         }
 
+        /**
+         * Returns the label of the summary right text view for the OptionSection.

[gogerald1, 2017/06/02 15:05:10]

*the right summary text view* to be consistent

[wuandy1, 2017/06/02 19:53:12]

Done.

+         */
+        @VisibleForTesting
+        public TextView getRightSummaryLabelForTest() {
+            return getSummaryRightTextView();
+        }
+
         /** Returns the number of option labels. */
         @VisibleForTesting
         public int getNumberOfOptionLabelsForTest() {