Always initialize this year and month

chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java

 // Copyright 2014 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.
 
 package org.chromium.chrome.browser.autofill;
 
 import android.content.Context;
 import android.content.DialogInterface;
 import android.content.res.Resources;
 import android.graphics.Color;
@@ skipping 180 matching lines @@
                 .setTitle(title)
                 .setView(v)
                 .setNegativeButton(R.string.cancel, null)
                 .setPositiveButton(confirmButtonLabel, null)
                 .create();
         mDialog.setOnDismissListener(this);
 
         mShouldRequestExpirationDate = shouldRequestExpirationDate;
         mThisYear = -1;
         mThisMonth = -1;
-        if (mShouldRequestExpirationDate) new CalendarTask().execute();

[sebsg, 2017/03/29 18:58:21]

I would rather you do this logic in the onNewCardLinkClicked function.

Also there a few things I'm confused about. For example, in onNewCardLinkClicked
there is an "assert mShouldRequestExpirationDate". This would always trigger in
you case no? because if it's not, the CalendarTask would have executed in the
constructor?

[Shanfeng, 2017/03/29 22:16:58]

Actually prefer to keep it here after more thoughts:
1) The original code before
https://codereview.chromium.org/1964323002/patch/1/10001 initialize mThisYear
and mThisMonth here.
2) We should initialize a private parameter at the constructor. Because it may
be used anywhere in this class.
3) This task is light weight.
4) It's an async task. So will not block the constructor.

As for "assert mShouldRequestExpirationDate", I believe
mShouldRequestExpirationDate is false when we initialize the prompt. But it is
set to true later:
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...
(For more info, the controller will call update, and set
mShouldRequestExpirationDate to be true:
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...)

[sebsg, 2017/03/30 14:02:35]

Right, when you send the onNewCardLinkClicked to the native code, it will change
it's value of shoudlRequestExpiration and call update on the Java version

Then my proposition would be that in update() when assigning the value of
mShouldRequestExpirationDate, you check if it was false before and now set to
true, and then load the calendar.

I prefer this solution because loading the calendar in the constructor is a
waste the majority of the time, since the usual operation is to just enter the
CVC.

[Shanfeng, 2017/03/30 20:06:04]

Done.

+        new CalendarTask().execute();
 
         // Create the listeners to be notified when the user focuses out the input fields.
         mCardUnmaskInput.setOnFocusChangeListener(new OnFocusChangeListener() {
             @Override
             public void onFocusChange(View v, boolean hasFocus) {
                 mDidFocusOnCvc = true;
                 validate();
             }
         });
         mMonthInput.setOnFocusChangeListener(new OnFocusChangeListener() {
@@ skipping 551 matching lines @@
     @VisibleForTesting
     public AlertDialog getDialogForTest() {
         return mDialog;
     }
 
     @VisibleForTesting
     public String getErrorMessage() {
         return mErrorMessage.getText().toString();
     }
 }