Skip to content

Commit

Permalink
[Autofill] Introduce concept of offers that are not card-linked
Browse files Browse the repository at this point in the history
This CL:
1) Moves away from calling offers "CreditCardOffers" in code in favor of
   "AutofillOffers", to make way for promo code offers
2) Creates new IsCardLinkedOffer() and IsPromoCodeOffer() functions
3) Ensures existing offer-based functionality only applies to
   card-linked (CLO) offers

Bug: 1190334
Change-Id: I4c2a8d0ebcbbef421e84da027ebc43e9d942d0b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2857525
Commit-Queue: Jared Saul <jsaul@google.com>
Reviewed-by: Siyu An <siyua@chromium.org>
Reviewed-by: Matthias Körber <koerber@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#879556}
  • Loading branch information
Jared Saul authored and Chromium LUCI CQ committed May 5, 2021
1 parent aca1024 commit ec58366
Show file tree
Hide file tree
Showing 35 changed files with 308 additions and 220 deletions.
Expand Up @@ -71,7 +71,7 @@ class SingleClientOfferSyncTest : public SyncTest {

void WaitForNumberOfOffers(size_t expected_count,
autofill::PersonalDataManager* pdm) {
while (pdm->GetCreditCardOffers().size() != expected_count ||
while (pdm->GetAutofillOffers().size() != expected_count ||
pdm->HasPendingQueriesForTesting()) {
WaitForOnPersonalDataChanged(pdm);
}
Expand Down Expand Up @@ -106,12 +106,12 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ClearOnDisableSync) {
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
// Make sure the offer data is in the DB.
ASSERT_EQ(1uL, pdm->GetCreditCardOffers().size());
ASSERT_EQ(1uL, pdm->GetAutofillOffers().size());

// Disable sync, the offer data should be gone.
GetSyncService(0)->StopAndClear();
WaitForNumberOfOffers(0, pdm);
EXPECT_EQ(0uL, pdm->GetCreditCardOffers().size());
EXPECT_EQ(0uL, pdm->GetAutofillOffers().size());

// Turn sync on again, the data should come back.
GetSyncService(0)->GetUserSettings()->SetSyncRequested(true);
Expand All @@ -121,7 +121,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ClearOnDisableSync) {
kSetSourceFromTest);
// Wait until Sync restores the card and it arrives at PDM.
WaitForNumberOfOffers(1, pdm);
EXPECT_EQ(1uL, pdm->GetCreditCardOffers().size());
EXPECT_EQ(1uL, pdm->GetAutofillOffers().size());
}

// Ensures that offer data should get cleared from the database when sync is
Expand All @@ -134,18 +134,18 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ClearOnStopSync) {
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
// Make sure the offer data is in the DB.
ASSERT_EQ(1uL, pdm->GetCreditCardOffers().size());
ASSERT_EQ(1uL, pdm->GetAutofillOffers().size());

// Stop sync, the offer data should be gone.
GetSyncService(0)->GetUserSettings()->SetSyncRequested(false);
WaitForNumberOfOffers(0, pdm);
EXPECT_EQ(0uL, pdm->GetCreditCardOffers().size());
EXPECT_EQ(0uL, pdm->GetAutofillOffers().size());

// Turn sync on again, the data should come back.
GetSyncService(0)->GetUserSettings()->SetSyncRequested(true);
// Wait until Sync restores the card and it arrives at PDM.
WaitForNumberOfOffers(1, pdm);
EXPECT_EQ(1uL, pdm->GetCreditCardOffers().size());
EXPECT_EQ(1uL, pdm->GetAutofillOffers().size());
}

// ChromeOS does not sign out, so the test below does not apply.
Expand All @@ -157,12 +157,12 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ClearOnSignOut) {
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
// Make sure the data & metadata is in the DB.
ASSERT_EQ(1uL, pdm->GetCreditCardOffers().size());
ASSERT_EQ(1uL, pdm->GetAutofillOffers().size());

// Signout, the data & metadata should be gone.
GetClient(0)->SignOutPrimaryAccount();
WaitForNumberOfOffers(0, pdm);
EXPECT_EQ(0uL, pdm->GetCreditCardOffers().size());
EXPECT_EQ(0uL, pdm->GetAutofillOffers().size());
}
#endif // !BUILDFLAG(IS_CHROMEOS_ASH)

Expand All @@ -178,7 +178,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest,
// Make sure the data is in the DB.
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
std::vector<AutofillOfferData*> offers = pdm->GetCreditCardOffers();
std::vector<AutofillOfferData*> offers = pdm->GetAutofillOffers();
ASSERT_EQ(1uL, offers.size());
EXPECT_EQ(999, offers[0]->offer_id);

Expand All @@ -189,7 +189,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest,
WaitForOnPersonalDataChanged(pdm);

// Make sure only the new data is present.
offers = pdm->GetCreditCardOffers();
offers = pdm->GetAutofillOffers();
ASSERT_EQ(1uL, offers.size());
EXPECT_EQ(888, offers[0]->offer_id);
}
Expand All @@ -206,7 +206,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, EmptyUpdatesAreIgnored) {
// Make sure the card is in the DB.
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
std::vector<AutofillOfferData*> offers = pdm->GetCreditCardOffers();
std::vector<AutofillOfferData*> offers = pdm->GetAutofillOffers();
ASSERT_EQ(1uL, offers.size());
EXPECT_EQ(999, offers[0]->offer_id);

Expand All @@ -231,7 +231,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, EmptyUpdatesAreIgnored) {
}

// Make sure the same data is present on the client.
offers = pdm->GetCreditCardOffers();
offers = pdm->GetAutofillOffers();
ASSERT_EQ(1uL, offers.size());
EXPECT_EQ(999, offers[0]->offer_id);
}
Expand All @@ -249,7 +249,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ChangedEntityGetsUpdated) {
// Make sure the card is in the DB.
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
std::vector<AutofillOfferData*> offers = pdm->GetCreditCardOffers();
std::vector<AutofillOfferData*> offers = pdm->GetAutofillOffers();
ASSERT_EQ(1uL, offers.size());
EXPECT_EQ(999, offers[0]->offer_id);
EXPECT_EQ(1U, offers[0]->eligible_instrument_id.size());
Expand All @@ -262,7 +262,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ChangedEntityGetsUpdated) {
// Make sure the data is present on the client.
pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
offers = pdm->GetCreditCardOffers();
offers = pdm->GetAutofillOffers();
ASSERT_EQ(1uL, offers.size());
EXPECT_EQ(999, offers[0]->offer_id);
EXPECT_EQ(2U, offers[0]->eligible_instrument_id.size());
Expand All @@ -277,11 +277,11 @@ IN_PROC_BROWSER_TEST_F(SingleClientOfferSyncTest, ClearOnDisableWalletSync) {
autofill::PersonalDataManager* pdm = GetPersonalDataManager(0);
ASSERT_NE(nullptr, pdm);
// Make sure the data is in the DB.
ASSERT_EQ(1uL, pdm->GetCreditCardOffers().size());
ASSERT_EQ(1uL, pdm->GetAutofillOffers().size());

// Turn off autofill sync, the data should be gone.
ASSERT_TRUE(
GetClient(0)->DisableSyncForType(syncer::UserSelectableType::kAutofill));
WaitForNumberOfOffers(0, pdm);
EXPECT_EQ(0uL, pdm->GetCreditCardOffers().size());
EXPECT_EQ(0uL, pdm->GetAutofillOffers().size());
}
27 changes: 20 additions & 7 deletions chrome/browser/ui/autofill/chrome_autofill_client.cc
Expand Up @@ -650,20 +650,33 @@ void ChromeAutofillClient::HideAutofillPopup(PopupHidingReason reason) {
}

void ChromeAutofillClient::ShowOfferNotificationIfApplicable(
const std::vector<GURL>& domains_to_display_bubble,
const GURL& offer_details_url,
const CreditCard* card) {
const AutofillOfferData* offer) {
if (!offer)
return;

// Ensure the card for a card-linked offer is successfully on the device.
CreditCard* card =
offer->eligible_instrument_id.empty()
? nullptr
: GetPersonalDataManager()->GetCreditCardByInstrumentId(
offer->eligible_instrument_id[0]);
if (offer->IsCardLinkedOffer() && !card)
return;

// TODO(crbug.com/1203811): Promo code offers should eventually show offer
// details in their own format as well.
if (!offer->IsCardLinkedOffer())
return;

#if defined(OS_ANDROID)
std::unique_ptr<OfferNotificationInfoBarControllerImpl> controller =
std::make_unique<OfferNotificationInfoBarControllerImpl>(web_contents());
controller->ShowIfNecessary(domains_to_display_bubble, offer_details_url,
card);
controller->ShowIfNecessary(offer, card);
#else
OfferNotificationBubbleControllerImpl::CreateForWebContents(web_contents());
OfferNotificationBubbleControllerImpl* controller =
OfferNotificationBubbleControllerImpl::FromWebContents(web_contents());
controller->ShowOfferNotificationIfApplicable(domains_to_display_bubble,
card);
controller->ShowOfferNotificationIfApplicable(offer, card);
#endif
}

Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/ui/autofill/chrome_autofill_client.h
Expand Up @@ -148,9 +148,7 @@ class ChromeAutofillClient
PopupType popup_type) override;
void HideAutofillPopup(PopupHidingReason reason) override;
void ShowOfferNotificationIfApplicable(
const std::vector<GURL>& domains_to_display_bubble,
const GURL& offer_details_url,
const CreditCard* card) override;
const AutofillOfferData* offer) override;
bool IsAutofillAssistantShowing() override;
bool IsAutocompleteEnabled() override;
void PropagateAutofillPredictions(
Expand Down
Expand Up @@ -13,6 +13,7 @@
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/page_action/page_action_icon_type.h"
#include "components/autofill/core/browser/autofill_metrics.h"
#include "components/autofill/core/browser/data_model/autofill_offer_data.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/navigation_handle.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -104,18 +105,20 @@ void OfferNotificationBubbleControllerImpl::OnBubbleClosed(
}

void OfferNotificationBubbleControllerImpl::ShowOfferNotificationIfApplicable(
const std::vector<GURL>& origins_to_display_bubble,
const AutofillOfferData* offer,
const CreditCard* card) {
DCHECK(offer);
// If icon/bubble is already visible, that means we have already shown a
// notification for this page.
if (IsIconVisible() || bubble_view())
return;

origins_to_display_bubble_.clear();
for (auto origin : origins_to_display_bubble)
for (auto origin : offer->merchant_domain)
origins_to_display_bubble_.emplace_back(origin);

card_ = *card;
if (card)
card_ = *card;

is_user_gesture_ = false;
Show();
Expand Down
Expand Up @@ -14,6 +14,8 @@

namespace autofill {

struct AutofillOfferData;

// Implementation of per-tab class to control the offer notification bubble and
// Omnibox icon.
class OfferNotificationBubbleControllerImpl
Expand Down Expand Up @@ -44,13 +46,11 @@ class OfferNotificationBubbleControllerImpl
bool IsIconVisible() const override;
void OnBubbleClosed(PaymentsBubbleClosedReason closed_reason) override;

// Displays an offer notification on current page. Populates the value for
// |origins_to_display_bubble_|, since the bubble and icon are sticky over a
// given set of origins. For a card linked offer, The information of the
// |card| will be displayed in the bubble.
void ShowOfferNotificationIfApplicable(
const std::vector<GURL>& origins_to_display_bubble,
const CreditCard* card);
// Displays an offer notification for the given |offer| on the current page.
// The information of the |card|, if present, will be displayed in the bubble
// for a card-linked offer.
void ShowOfferNotificationIfApplicable(const AutofillOfferData* offer,
const CreditCard* card);

// Called when user clicks on omnibox icon.
void ReshowBubble();
Expand Down
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/data_model/autofill_offer_data.h"
#include "content/public/test/mock_navigation_handle.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -71,9 +72,8 @@ class OfferNotificationBubbleControllerImplTest
}

protected:
void ShowBubble(const std::vector<GURL> origins_to_display_bubble) {
controller()->ShowOfferNotificationIfApplicable(origins_to_display_bubble,
&card_);
void ShowBubble(const AutofillOfferData* offer) {
controller()->ShowOfferNotificationIfApplicable(offer, &card_);
}

void CloseBubble(PaymentsBubbleClosedReason closed_reason =
Expand All @@ -86,6 +86,15 @@ class OfferNotificationBubbleControllerImplTest
controller()->ReshowBubble();
}

AutofillOfferData CreateTestOfferWithOrigins(
const std::vector<GURL>& merchant_origins) {
// Only adding what the tests need to pass. Feel free to add more populated
// fields as necessary.
AutofillOfferData offer;
offer.merchant_domain = merchant_origins;
return offer;
}

TestOfferNotificationBubbleControllerImpl* controller() {
return static_cast<TestOfferNotificationBubbleControllerImpl*>(
TestOfferNotificationBubbleControllerImpl::FromWebContents(
Expand All @@ -98,7 +107,9 @@ class OfferNotificationBubbleControllerImplTest

TEST_F(OfferNotificationBubbleControllerImplTest, BubbleShown) {
// Check that bubble is visible.
ShowBubble({GURL("https://www.example.com/first/").GetOrigin()});
AutofillOfferData offer = CreateTestOfferWithOrigins(
{GURL("https://www.example.com/first/").GetOrigin()});
ShowBubble(&offer);
EXPECT_TRUE(controller()->GetOfferNotificationBubbleView());
}

Expand All @@ -117,8 +128,10 @@ TEST_F(OfferNotificationBubbleControllerImplTest, OriginSticky) {
for (const auto& test_case : test_cases) {
// Set the initial origin that the bubble will be displayed on.
NavigateAndCommitActiveTab(GURL("https://www.example.com/first/"));
ShowBubble({GURL("https://www.example.com/first/").GetOrigin(),
GURL("https://www.test.com/first/").GetOrigin()});
AutofillOfferData offer = CreateTestOfferWithOrigins(
{GURL("https://www.example.com/first/").GetOrigin(),
GURL("https://www.test.com/first/").GetOrigin()});
ShowBubble(&offer);

// Bubble should be visible.
EXPECT_TRUE(controller()->GetOfferNotificationBubbleView());
Expand Down
Expand Up @@ -6,6 +6,7 @@

#include "chrome/browser/ui/android/infobars/autofill_offer_notification_infobar.h"
#include "chrome/browser/ui/autofill/payments/offer_notification_helper.h"
#include "components/autofill/core/browser/data_model/autofill_offer_data.h"
#include "components/autofill/core/browser/payments/autofill_offer_notification_infobar_delegate_mobile.h"
#include "components/infobars/content/content_infobar_manager.h"
#include "components/infobars/core/infobar.h"
Expand All @@ -18,19 +19,22 @@ OfferNotificationInfoBarControllerImpl::OfferNotificationInfoBarControllerImpl(
: web_contents_(contents) {}

void OfferNotificationInfoBarControllerImpl::ShowIfNecessary(
const std::vector<GURL>& origins_to_display_infobar,
const GURL& offer_details_url,
const AutofillOfferData* offer,
const CreditCard* card) {
DCHECK(offer);

OfferNotificationHelper::CreateForWebContents(web_contents_);
OfferNotificationHelper* offer_notification_helper =
OfferNotificationHelper::FromWebContents(web_contents_);
if (offer_notification_helper->OfferNotificationHasAlreadyBeenShown())
return;

std::vector<GURL> origins_to_display_infobar = offer->merchant_domain;
if (card) {
infobars::ContentInfoBarManager::FromWebContents(web_contents_)
->AddInfoBar(std::make_unique<AutofillOfferNotificationInfoBar>(
std::make_unique<AutofillOfferNotificationInfoBarDelegateMobile>(
offer_details_url, *card)));
offer->offer_details_url, *card)));
offer_notification_helper->OnDisplayOfferNotification(
origins_to_display_infobar);
}
Expand Down
Expand Up @@ -12,6 +12,8 @@

namespace autofill {

struct AutofillOfferData;

// Per-tab controller to control the offer notification infobar displayed on
// mobile.
class OfferNotificationInfoBarControllerImpl {
Expand All @@ -27,9 +29,7 @@ class OfferNotificationInfoBarControllerImpl {

// Show the infobar unless it was already shown in the same tab with the same
// origin.
void ShowIfNecessary(const std::vector<GURL>& origins_to_display_infobar,
const GURL& offer_details_url,
const CreditCard* card);
void ShowIfNecessary(const AutofillOfferData* offer, const CreditCard* card);

private:
content::WebContents* web_contents_;
Expand Down

0 comments on commit ec58366

Please sign in to comment.