Skip to content

Commit

Permalink
Reland "[AF] Add account indication footer to AutofillSaveCardInfoBar"
Browse files Browse the repository at this point in the history
This reverts commit 9e3dabc.

Reason for reland: This CL was not actually the cause of the test failures (https://crrev.com/c/2519333 fixed them).

Original change's description:
> Revert "[AF] Add account indication footer to AutofillSaveCardInfoBar"
>
> This reverts commit ba7d881.
>
> Reason for revert: SaveCardBubbleViewsFullFormBrowserTestWithAutofillUpstream.Upload_ClickingNoThanksClosesBubble and a bunch of other tests started failing in https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/21164
>
>
> Original change's description:
> > [AF] Add account indication footer to AutofillSaveCardInfoBar
> >
> > The footer is shown only to signed-in users have more than one account, and who do not sync.
> >
> > Screenshot with feature enabled in light mode:
> > https://screenshot.googleplex.com/ApVmkMVQakM6ai9
> >
> > Screenshot with feature enabled in dark mode:
> > https://screenshot.googleplex.com/BJB2jUuGoH3T5jM
> >
> > Bug: 1135847
> > Change-Id: Ia2e031a48920e44e66503fab62f26ed87b8e1d92
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2496184
> > Commit-Queue: Anne Lim <annelim@google.com>
> > Reviewed-by: Marc Treib <treib@chromium.org>
> > Reviewed-by: Matthew Jones <mdjones@chromium.org>
> > Reviewed-by: Jared Saul <jsaul@google.com>
> > Cr-Commit-Position: refs/heads/master@{#823710}
>
> TBR=treib@chromium.org,mdjones@chromium.org,jsaul@google.com,bsazonov@chromium.org,annelim@google.com
>
> Change-Id: I5f235bc5843acf4ea504a78762907a019576e656
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1135847
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518852
> Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> Commit-Queue: Christian Dullweber <dullweber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#823942}

TBR=treib@chromium.org,mdjones@chromium.org,jsaul@google.com,dullweber@chromium.org,bsazonov@chromium.org,annelim@google.com

# Not skipping CQ checks because this is a reland.

Bug: 1135847
Change-Id: I7d1a7ad2bc6b5c30d2b859c9f68bfee34992ff95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517530
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824006}
  • Loading branch information
Marc Treib authored and Commit Bot committed Nov 4, 2020
1 parent e7399f2 commit b6fcf13
Show file tree
Hide file tree
Showing 17 changed files with 215 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,26 @@

package org.chromium.chrome.browser.infobar;

import android.content.res.Resources;
import android.graphics.Bitmap;
import android.text.SpannableString;
import android.text.Spanned;
import android.text.TextUtils;
import android.text.style.ClickableSpan;
import android.view.LayoutInflater;
import android.view.View;
import android.widget.LinearLayout;
import android.widget.TextView;

import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.components.browser_ui.widget.RoundedCornerImageView;
import org.chromium.components.infobars.ConfirmInfoBar;
import org.chromium.components.infobars.InfoBarControlLayout;
import org.chromium.components.infobars.InfoBarLayout;
import org.chromium.components.signin.base.AccountInfo;
import org.chromium.ui.UiUtils;

import java.util.ArrayList;
Expand Down Expand Up @@ -84,6 +91,7 @@ public LegalMessageLine(String text) {
}
}

private final AccountInfo mAccountInfo;
private final long mNativeAutofillSaveCardInfoBar;
private final List<CardDetail> mCardDetails = new ArrayList<>();
private int mIconDrawableId = -1;
Expand All @@ -103,10 +111,11 @@ public LegalMessageLine(String text) {
* @param linkText Link text to display in addition to the message.
* @param buttonOk String to display on the OK button.
* @param buttonCancel String to display on the Cancel button.
* @param accountInfo AccountInfo which includes user's email address and profile picture.
*/
private AutofillSaveCardInfoBar(long nativeAutofillSaveCardInfoBar, int iconId,
Bitmap iconBitmap, String message, String linkText, String buttonOk,
String buttonCancel, boolean isGooglePayBrandingEnabled) {
String buttonCancel, boolean isGooglePayBrandingEnabled, AccountInfo accountInfo) {
// If Google Pay branding is enabled, no icon is specified here; it is rather added in
// |createContent|. This hides the ImageView that normally shows the icon and gets rid of
// the left padding of the infobar content.
Expand All @@ -117,6 +126,7 @@ private AutofillSaveCardInfoBar(long nativeAutofillSaveCardInfoBar, int iconId,
mTitleText = message;
mIsGooglePayBrandingEnabled = isGooglePayBrandingEnabled;
mNativeAutofillSaveCardInfoBar = nativeAutofillSaveCardInfoBar;
mAccountInfo = accountInfo;
}

/**
Expand All @@ -129,14 +139,15 @@ private AutofillSaveCardInfoBar(long nativeAutofillSaveCardInfoBar, int iconId,
* @param linkText Link text to display in addition to the message.
* @param buttonOk String to display on the OK button.
* @param buttonCancel String to display on the Cancel button.
* @param accountInfo AccountInfo which includes user's email address and profile picture.
* @return A new instance of the infobar.
*/
@CalledByNative
private static AutofillSaveCardInfoBar create(long nativeAutofillSaveCardInfoBar, int iconId,
Bitmap iconBitmap, String message, String linkText, String buttonOk,
String buttonCancel, boolean isGooglePayBrandingEnabled) {
String buttonCancel, boolean isGooglePayBrandingEnabled, AccountInfo accountInfo) {
return new AutofillSaveCardInfoBar(nativeAutofillSaveCardInfoBar, iconId, iconBitmap,
message, linkText, buttonOk, buttonCancel, isGooglePayBrandingEnabled);
message, linkText, buttonOk, buttonCancel, isGooglePayBrandingEnabled, accountInfo);
}

/**
Expand Down Expand Up @@ -219,6 +230,31 @@ public void onClick(View view) {
}
control.addDescription(text);
}

if (ChromeFeatureList.isEnabled(
ChromeFeatureList.AUTOFILL_ENABLE_SAVE_CARD_INFO_BAR_ACCOUNT_INDICATION_FOOTER)
&& !TextUtils.isEmpty(mAccountInfo.getEmail())
&& mAccountInfo.getAccountImage() != null) {
Resources res = layout.getResources();
int smallIconSize = res.getDimensionPixelSize(R.dimen.infobar_small_icon_size);
int padding = res.getDimensionPixelOffset(R.dimen.infobar_padding);

LinearLayout footer = (LinearLayout) LayoutInflater.from(layout.getContext())
.inflate(R.layout.infobar_footer, null, false);

TextView emailView = (TextView) footer.findViewById(R.id.infobar_footer_email);
emailView.setText(mAccountInfo.getEmail());

RoundedCornerImageView profilePicView =
(RoundedCornerImageView) footer.findViewById(R.id.infobar_footer_profile_pic);
Bitmap resizedProfilePic = Bitmap.createScaledBitmap(
mAccountInfo.getAccountImage(), smallIconSize, smallIconSize, false);
profilePicView.setRoundedCorners(
smallIconSize / 2, smallIconSize / 2, smallIconSize / 2, smallIconSize / 2);
profilePicView.setImageBitmap(resizedProfilePic);

layout.addFooterView(footer);
}
}

@NativeMethods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ private void assertPrimaryButtonLabel(String buttonLabel) {
Assert.assertEquals(buttonLabel, primaryButton.getText().toString());
}

private boolean isFooterShown() {
InfoBarLayout view = (InfoBarLayout) getAutofillSaveCardInfoBar().getView();
return view.hasFooter();
}

private void waitForSaveCardInfoBar() {
CriteriaHelper.pollUiThread(
()
Expand Down Expand Up @@ -222,4 +227,68 @@ public void testSaveCardInfoBarContinueButton_EmptyName_launchesNameFixFlow()
// Verify that dialog is not null
Assert.assertNotNull(fixflowPromptPropertyModel);
}

@Test
@MediumTest
@Restriction(Restriction.RESTRICTION_TYPE_INTERNET)
public void testSaveCardInfoBar_syncedUser_noAccountIndicationFooter() throws TimeoutException {
mActivityTestRule.startMainActivityWithURL(mServer.getURL(TEST_FORM_URL));
final WebContents webContents = mActivityTestRule.getActivity().getCurrentWebContents();
mActivityTestRule.setUpAccountAndEnableSyncForTesting();

DOMUtils.clickNode(webContents, "fill_form");
DOMUtils.clickNode(webContents, "submit");
waitForSaveCardInfoBar();

Assert.assertFalse(isFooterShown());
}

@Test
@MediumTest
@Restriction(Restriction.RESTRICTION_TYPE_INTERNET)
public void testSaveCardInfoBar_loggedInUnsyncedUser_showsAccountIndicationFooter()
throws TimeoutException {
mActivityTestRule.startMainActivityWithURL(mServer.getURL(TEST_FORM_URL));
final WebContents webContents = mActivityTestRule.getActivity().getCurrentWebContents();
mActivityTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();

DOMUtils.clickNode(webContents, "fill_form");
DOMUtils.clickNode(webContents, "submit");
waitForSaveCardInfoBar();

Assert.assertTrue(isFooterShown());
}

@Test
@MediumTest
@Restriction(Restriction.RESTRICTION_TYPE_INTERNET)
public void testSaveCardInfoBar_singleAccountUser_noAccountIndicationFooter()
throws TimeoutException {
mActivityTestRule.startMainActivityWithURL(mServer.getURL(TEST_FORM_URL));
final WebContents webContents = mActivityTestRule.getActivity().getCurrentWebContents();
mActivityTestRule.addAccount("account1");

DOMUtils.clickNode(webContents, "fill_form");
DOMUtils.clickNode(webContents, "submit");
waitForSaveCardInfoBar();

Assert.assertFalse(isFooterShown());
}

@Test
@MediumTest
@Restriction(Restriction.RESTRICTION_TYPE_INTERNET)
public void testSaveCardInfoBar_multipleAccountUser_showsAccountIndicationFooter()
throws TimeoutException {
mActivityTestRule.startMainActivityWithURL(mServer.getURL(TEST_FORM_URL));
final WebContents webContents = mActivityTestRule.getActivity().getCurrentWebContents();
mActivityTestRule.addAccount("account1");
mActivityTestRule.addAccount("account2");

DOMUtils.clickNode(webContents, "fill_form");
DOMUtils.clickNode(webContents, "submit");
waitForSaveCardInfoBar();

Assert.assertTrue(isFooterShown());
}
}
11 changes: 11 additions & 0 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6666,6 +6666,17 @@ const FeatureEntry kFeatureEntries[] = {
{"check-offline-capability", flag_descriptions::kCheckOfflineCapabilityName,
flag_descriptions::kCheckOfflineCapabilityDescription, kOsAll,
FEATURE_VALUE_TYPE(blink::features::kCheckOfflineCapability)},
#if defined(OS_ANDROID)
{"enable-autofill-save-card-info-bar-account-indication-footer",
flag_descriptions::
kEnableAutofillSaveCardInfoBarAccountIndicationFooterName,
flag_descriptions::
kEnableAutofillSaveCardInfoBarAccountIndicationFooterDescription,
kOsAndroid,
FEATURE_VALUE_TYPE(
autofill::features::
kAutofillEnableSaveCardInfoBarAccountIndicationFooter)},
#endif

// NOTE: Adding a new flag requires adding a corresponding entry to enum
// "LoginCustomFlags" in tools/metrics/histograms/enums.xml. See "Flag
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,11 @@
"owners": [ "tmartino@chromium.org" ],
"expiry_milestone": 78
},
{
"name": "enable-autofill-save-card-info-bar-account-indication-footer",
"owners": [ "annelim@google.com, mdjones@chromium.org"],
"expiry_milestone": 95
},
{
"name": "enable-autofill-upi-vpa",
"owners": [ "cfroussios" ],
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,12 @@ const char kEnableAutofillPasswordInfoBarAccountIndicationFooterDescription[] =
"When enabled, a footer indicating user's e-mail address will appear at "
"the bottom of corresponding password InfoBars.";

const char kEnableAutofillSaveCardInfoBarAccountIndicationFooterName[] =
"Display SaveCardInfoBar footer with account indication information";
const char kEnableAutofillSaveCardInfoBarAccountIndicationFooterDescription[] =
"When enabled, a footer indicating user's e-mail address will appear at "
"the bottom of SaveCardInfoBar.";

const char kEnableAutofillCreditCardCvcPromptGoogleLogoName[] =
"Enable Google Pay branding on CVC prompt on Android";
const char kEnableAutofillCreditCardCvcPromptGoogleLogoDescription[] =
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,10 @@ extern const char kEnableAutofillPasswordInfoBarAccountIndicationFooterName[];
extern const char
kEnableAutofillPasswordInfoBarAccountIndicationFooterDescription[];

extern const char kEnableAutofillSaveCardInfoBarAccountIndicationFooterName[];
extern const char
kEnableAutofillSaveCardInfoBarAccountIndicationFooterDescription[];

extern const char kEnableExperimentalCookieFeaturesName[];
extern const char kEnableExperimentalCookieFeaturesDescription[];

Expand Down
1 change: 1 addition & 0 deletions chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
&autofill::features::kAutofillCreditCardAuthentication,
&autofill::features::kAutofillDownstreamCvcPromptUseGooglePayLogo,
&autofill::features::kAutofillEnablePasswordInfoBarAccountIndicationFooter,
&autofill::features::kAutofillEnableSaveCardInfoBarAccountIndicationFooter,
&autofill::features::kAutofillKeyboardAccessory,
&autofill::features::kAutofillManualFallbackAndroid,
&autofill::features::kAutofillRefreshStyleAndroid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
"AutofillEnableGoogleIssuedCard";
public static final String AUTOFILL_ENABLE_PASSWORD_INFO_BAR_ACCOUNT_INDICATION_FOOTER =
"AutofillEnablePasswordInfoBarAccountIndicationFooter";
public static final String AUTOFILL_ENABLE_SAVE_CARD_INFO_BAR_ACCOUNT_INDICATION_FOOTER =
"AutofillEnableSaveCardInfoBarAccountIndicationFooter";
public static final String ADJUST_WEBAPK_INSTALLATION_SPACE = "AdjustWebApkInstallationSpace";
public static final String ANDROID_DEFAULT_BROWSER_PROMO = "AndroidDefaultBrowserPromo";
public static final String ANDROID_MANAGED_BY_MENU_ITEM = "AndroidManagedByMenuItem";
Expand Down
18 changes: 13 additions & 5 deletions chrome/browser/ui/android/infobars/autofill_save_card_infobar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,20 @@ using base::android::ScopedJavaLocalRef;
namespace autofill {

std::unique_ptr<infobars::InfoBar> CreateSaveCardInfoBarMobile(
std::unique_ptr<AutofillSaveCardInfoBarDelegateMobile> delegate) {
return std::make_unique<AutofillSaveCardInfoBar>(std::move(delegate));
std::unique_ptr<AutofillSaveCardInfoBarDelegateMobile> delegate,
base::Optional<AccountInfo> account_info) {
return std::make_unique<AutofillSaveCardInfoBar>(std::move(delegate),
account_info);
}

} // namespace autofill

AutofillSaveCardInfoBar::AutofillSaveCardInfoBar(
std::unique_ptr<autofill::AutofillSaveCardInfoBarDelegateMobile> delegate)
: ChromeConfirmInfoBar(std::move(delegate)) {}
std::unique_ptr<autofill::AutofillSaveCardInfoBarDelegateMobile> delegate,
base::Optional<AccountInfo> account_info)
: ChromeConfirmInfoBar(std::move(delegate)) {
account_info_ = account_info;
}

AutofillSaveCardInfoBar::~AutofillSaveCardInfoBar() {}

Expand All @@ -59,7 +64,10 @@ AutofillSaveCardInfoBar::CreateRenderInfoBar(JNIEnv* env) {
env, GetTextFor(ConfirmInfoBarDelegate::BUTTON_OK)),
base::android::ConvertUTF16ToJavaString(
env, GetTextFor(ConfirmInfoBarDelegate::BUTTON_CANCEL)),
delegate->IsGooglePayBrandingEnabled());
delegate->IsGooglePayBrandingEnabled(),
account_info_.has_value()
? ConvertToJavaAccountInfo(env, account_info_.value())
: nullptr);

Java_AutofillSaveCardInfoBar_setDescriptionText(
env, java_delegate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/macros.h"
#include "chrome/browser/ui/android/infobars/chrome_confirm_infobar.h"
#include "components/signin/public/identity_manager/account_info.h"

namespace autofill {
class AutofillSaveCardInfoBarDelegateMobile;
Expand All @@ -20,8 +21,9 @@ class AutofillSaveCardInfoBarDelegateMobile;
class AutofillSaveCardInfoBar : public ChromeConfirmInfoBar {
public:
explicit AutofillSaveCardInfoBar(
std::unique_ptr<autofill::AutofillSaveCardInfoBarDelegateMobile>
delegate);
std::unique_ptr<autofill::AutofillSaveCardInfoBarDelegateMobile> delegate,
base::Optional<AccountInfo> account_info);

~AutofillSaveCardInfoBar() override;

// Called when a link in the legal message text was clicked.
Expand All @@ -40,6 +42,8 @@ class AutofillSaveCardInfoBar : public ChromeConfirmInfoBar {
// are stored in /chrome and /components cannot depend on /chrome.
int GetGooglePayBrandingIconId();

base::Optional<AccountInfo> account_info_;

DISALLOW_COPY_AND_ASSIGN(AutofillSaveCardInfoBar);
};

Expand Down
31 changes: 29 additions & 2 deletions chrome/browser/ui/autofill/chrome_autofill_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,8 @@ void ChromeAutofillClient::ConfirmSaveCreditCardLocally(
/*upload=*/false, options, card, LegalMessageLines(),
/*upload_save_card_callback=*/
AutofillClient::UploadSaveCardPromptCallback(),
/*local_save_card_callback=*/std::move(callback), GetPrefs())));
/*local_save_card_callback=*/std::move(callback), GetPrefs()),
base::nullopt));
#else
// Do lazy initialization of SaveCardBubbleControllerImpl.
SaveCardBubbleControllerImpl::CreateForWebContents(web_contents());
Expand All @@ -428,9 +429,19 @@ void ChromeAutofillClient::ConfirmSaveCreditCardToCloud(
/*upload_save_card_callback=*/std::move(callback),
/*local_save_card_callback=*/
AutofillClient::LocalSaveCardPromptCallback(), GetPrefs());
bool sync_disabled_wallet_transport_enabled =
GetPersonalDataManager()->GetSyncSigninState() ==
autofill::AutofillSyncSigninState::kSignedInAndWalletSyncTransportEnabled;

base::Optional<AccountInfo> account_info = base::nullopt;
// AccountInfo data should be passed down only if sync is off and user has
// multiple accounts.
if (sync_disabled_wallet_transport_enabled && IsMultipleAccountUser()) {
account_info = GetAccountInfo();
}
InfoBarService::FromWebContents(web_contents())
->AddInfoBar(CreateSaveCardInfoBarMobile(
std::move(save_card_info_bar_delegate_mobile)));
std::move(save_card_info_bar_delegate_mobile), account_info));
#else
// Do lazy initialization of SaveCardBubbleControllerImpl.
SaveCardBubbleControllerImpl::CreateForWebContents(web_contents());
Expand Down Expand Up @@ -718,6 +729,22 @@ Profile* ChromeAutofillClient::GetProfile() const {
return Profile::FromBrowserContext(web_contents()->GetBrowserContext());
}

base::Optional<AccountInfo> ChromeAutofillClient::GetAccountInfo() {
signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(GetProfile());
CoreAccountId account_id =
identity_manager->GetPrimaryAccountId(signin::ConsentLevel::kNotRequired);
return identity_manager
->FindExtendedAccountInfoForAccountWithRefreshTokenByAccountId(
account_id);
}

bool ChromeAutofillClient::IsMultipleAccountUser() {
signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(GetProfile());
return identity_manager->GetAccountsWithRefreshTokens().size() > 1;
}

base::string16 ChromeAutofillClient::GetAccountHolderName() {
Profile* profile = GetProfile();
if (!profile)
Expand Down
Loading

0 comments on commit b6fcf13

Please sign in to comment.