[Autofill] Credit Card Assist Infobar

components/autofill/core/browser/autofill_assist_infobar_delegate_mobile.cc

+// Copyright 2016 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/autofill/core/browser/autofill_assist_infobar_delegate_mobile.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "components/autofill/core/browser/credit_card.h"
+#include "components/autofill/core/common/autofill_constants.h"
+#include "components/infobars/core/infobar.h"
+#include "components/infobars/core/infobar_manager.h"
+#include "grit/components_scaled_resources.h"
+#include "grit/components_strings.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/window_open_disposition.h"

[Peter Kasting, 2016/07/29 21:26:34]

You don't use this #include.  Check to see if there are other #includes you can
omit as well.

[Mathieu, 2016/08/01 22:16:57]

Done. Refined the includes, thanks.

+
+namespace autofill {
+
+AutofillAssistInfoBarDelegateMobile::AutofillAssistInfoBarDelegateMobile(
+    const CreditCard& card,
+    const base::Closure& assist_callback)
+    : ConfirmInfoBarDelegate(),
+      assist_callback_(assist_callback),
+      issuer_icon_id_(CreditCard::IconResourceId(card.type())),
+      card_label_(base::string16(kMidlineEllipsis) + card.LastFourDigits()),
+      card_sub_label_(card.AbbreviatedExpirationDateForDisplay()) {
+  AutofillMetrics::LogCreditCardAssistInfoBarMetric(
+      AutofillMetrics::INFOBAR_SHOWN);

[Peter Kasting, 2016/07/29 21:26:34]

This is a call that should probably be in the Create() function and be prefaced
on whether the AddInfoBar() call succeeds, since otherwise the infobar wasn't
actually shown.  Note that if the infobar was never shown, you probably don't
want to log INFOBAR_IGNORED either.

(This is one reason for Create() functions; as-is, you can't protect against,
say, someone just constructing an instance of this delegate and throwing it away
from corrupting your stats.)

[Mathieu, 2016/08/01 22:16:57]

Good catch! Now using a boolean to track whether the infobar was shown and only
logging if AddInfoBar was successful).

+}
+
+AutofillAssistInfoBarDelegateMobile::~AutofillAssistInfoBarDelegateMobile() {
+  if (!had_user_interaction_)
+    LogUserAction(AutofillMetrics::INFOBAR_IGNORED);
+}
+
+int AutofillAssistInfoBarDelegateMobile::GetIconId() const {
+  return IDR_INFOBAR_AUTOFILL_CC;
+}
+
+base::string16 AutofillAssistInfoBarDelegateMobile::GetMessageText() const {
+  return l10n_util::GetStringUTF16(
+      IDS_AUTOFILL_CREDIT_CARD_ASSIST_INFOBAR_TITLE);
+}
+
+infobars::InfoBarDelegate::Type
+AutofillAssistInfoBarDelegateMobile::GetInfoBarType() const {
+  return PAGE_ACTION_TYPE;
+}
+
+infobars::InfoBarDelegate::InfoBarIdentifier
+AutofillAssistInfoBarDelegateMobile::GetIdentifier() const {
+  return AUTOFILL_CC_ASSIST_INFOBAR_DELEGATE;
+}
+
+bool AutofillAssistInfoBarDelegateMobile::ShouldExpire(
+    const NavigationDetails& details) const {
+  // InfoBar should disappear when the page is navigated.
+  return true;

[Peter Kasting, 2016/07/29 21:26:34]

It's very unlikely that you want to override the base class implementation of
this method.  For example, this will mean the infobar hides if the user is
navigated to a different anchor within the page.

The base class already hides the infobar in navigations that feel like they're
"replacing the page content".  You probably just want that.

[Mathieu, 2016/08/01 22:16:57]

Base class implementation SGTM, thanks.

+}
+
+void AutofillAssistInfoBarDelegateMobile::InfoBarDismissed() {
+  LogUserAction(AutofillMetrics::INFOBAR_DENIED);
+}
+
+base::string16 AutofillAssistInfoBarDelegateMobile::GetButtonLabel(
+    InfoBarButton button) const {
+  return l10n_util::GetStringUTF16(
+      button == BUTTON_OK ? IDS_AUTOFILL_CREDIT_CARD_ASSIST_INFOBAR_ACCEPT
+                          : IDS_AUTOFILL_CREDIT_CARD_ASSIST_INFOBAR_DENY);
+}
+
+bool AutofillAssistInfoBarDelegateMobile::Accept() {
+  assist_callback_.Run();
+  assist_callback_.Reset();

[Peter Kasting, 2016/07/29 21:26:34]

Why is this call needed?

[Mathieu, 2016/08/01 22:16:57]

Removed.

+  LogUserAction(AutofillMetrics::INFOBAR_ACCEPTED);
+  return true;
+}
+
+bool AutofillAssistInfoBarDelegateMobile::Cancel() {
+  LogUserAction(AutofillMetrics::INFOBAR_DENIED);
+  return true;
+}
+
+void AutofillAssistInfoBarDelegateMobile::LogUserAction(
+    AutofillMetrics::InfoBarMetric user_action) {
+  DCHECK(!had_user_interaction_);
+
+  AutofillMetrics::LogCreditCardAssistInfoBarMetric(user_action);
+  had_user_interaction_ = true;
+}
+
+}  // namespace autofill