diff --git a/chrome/browser/commerce/commerce_feature_list.h b/chrome/browser/commerce/commerce_feature_list.h index 6f0f48536b45f5..8741f6fa93c9ad 100644 --- a/chrome/browser/commerce/commerce_feature_list.h +++ b/chrome/browser/commerce/commerce_feature_list.h @@ -17,10 +17,6 @@ extern const base::Feature kShoppingList; extern const base::Feature kRetailCoupons; extern const base::Feature kCommerceDeveloper; -// Interval that controls the frequency of showing coupons in infobar bubbles. -constexpr base::FeatureParam kCouponDisplayInterval{ - &commerce::kRetailCoupons, "coupon_display_interval", base::Hours(18)}; - // Check if a URL belongs to a partner merchant of coupon discount. bool IsCouponDiscountPartnerMerchant(const GURL& url); } // namespace commerce diff --git a/chrome/browser/commerce/coupons/coupon_service.cc b/chrome/browser/commerce/coupons/coupon_service.cc index 1e2c91718867ad..864e5700d17271 100644 --- a/chrome/browser/commerce/coupons/coupon_service.cc +++ b/chrome/browser/commerce/coupons/coupon_service.cc @@ -40,6 +40,7 @@ CouponService::CouponService(std::unique_ptr coupon_db) InitializeCouponsMap(); } CouponService::~CouponService() = default; +CouponService::CouponService() = default; void CouponService::UpdateFreeListingCoupons(const CouponsMap& coupon_map) { coupon_db_->DeleteAllCoupons(); @@ -123,8 +124,6 @@ bool CouponService::IsUrlEligible(const GURL& url) { return coupon_map_.find(url.DeprecatedGetOriginAsURL()) != coupon_map_.end(); } -CouponService::CouponService() = default; - CouponDB* CouponService::GetDB() { return coupon_db_.get(); } diff --git a/chrome/browser/commerce/coupons/coupon_service.h b/chrome/browser/commerce/coupons/coupon_service.h index ca322eaa38355f..2df786ecd2dfc1 100644 --- a/chrome/browser/commerce/coupons/coupon_service.h +++ b/chrome/browser/commerce/coupons/coupon_service.h @@ -43,7 +43,7 @@ class CouponService : public KeyedService, virtual void DeleteAllFreeListingCoupons(); // Get the last time that |offer| has shown in infobar bubble. - virtual base::Time GetCouponDisplayTimestamp( + base::Time GetCouponDisplayTimestamp( const autofill::AutofillOfferData& offer); // Record the last display timestamp of a coupon in the cache layer and @@ -59,15 +59,12 @@ class CouponService : public KeyedService, // Check if CouponService has eligible coupons for |url|. bool IsUrlEligible(const GURL& url) override; - protected: - // Default constructor for testing purposes only. - CouponService(); - private: friend class CouponServiceFactory; friend class CouponServiceTest; friend class CartServiceCouponTest; + CouponService(); // Use |CouponServiceFactory::GetForProfile(...)| to get an instance of this // service. explicit CouponService(std::unique_ptr coupon_db); diff --git a/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.cc b/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.cc index 479d3d92a9d0a7..9d53a5bcec8027 100644 --- a/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.cc +++ b/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.cc @@ -6,10 +6,6 @@ #include -#include "chrome/browser/commerce/commerce_feature_list.h" -#include "chrome/browser/commerce/coupons/coupon_service.h" -#include "chrome/browser/commerce/coupons/coupon_service_factory.h" -#include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/autofill/autofill_bubble_base.h" #include "chrome/browser/ui/autofill/autofill_bubble_handler.h" #include "chrome/browser/ui/browser.h" @@ -49,9 +45,7 @@ OfferNotificationBubbleController* OfferNotificationBubbleController::Get( OfferNotificationBubbleControllerImpl::OfferNotificationBubbleControllerImpl( content::WebContents* web_contents) - : AutofillBubbleControllerBase(web_contents), - coupon_service_(CouponServiceFactory::GetForProfile( - Profile::FromBrowserContext(web_contents->GetBrowserContext()))) {} + : AutofillBubbleControllerBase(web_contents) {} std::u16string OfferNotificationBubbleControllerImpl::GetWindowTitle() const { switch (offer_->GetOfferType()) { @@ -160,23 +154,6 @@ void OfferNotificationBubbleControllerImpl::ShowOfferNotificationIfApplicable( if (card) card_ = *card; - if (offer->GetOfferType() == - AutofillOfferData::OfferType::FREE_LISTING_COUPON_OFFER) { - base::Time last_display_time = - coupon_service_->GetCouponDisplayTimestamp(*offer); - if (!last_display_time.is_null() && - (base::Time::Now() - last_display_time) < - commerce::kCouponDisplayInterval.Get()) { - UpdatePageActionIcon(); - AutofillMetrics::LogOfferNotificationBubbleSuppressed( - AutofillOfferData::OfferType::FREE_LISTING_COUPON_OFFER); - return; - } - // This will update the offer's last shown time both in cache layer and - // storage. - coupon_service_->RecordCouponDisplayTimestamp(*offer); - } - is_user_gesture_ = false; Show(); } diff --git a/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.h b/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.h index ebff9f3c2e0417..81ab0ae61b71a7 100644 --- a/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.h +++ b/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.h @@ -12,8 +12,6 @@ #include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_user_data.h" -class CouponService; - namespace autofill { struct AutofillOfferData; @@ -72,7 +70,6 @@ class OfferNotificationBubbleControllerImpl private: friend class content::WebContentsUserData< OfferNotificationBubbleControllerImpl>; - friend class OfferNotificationBubbleControllerImplTest; friend class OfferNotificationBubbleViewsTestBase; // Returns whether the web content associated with this controller is active. @@ -104,9 +101,6 @@ class OfferNotificationBubbleControllerImpl // when navigating to a origins outside of this set. std::vector origins_to_display_bubble_; - // Used to update coupon last display timestamp. - CouponService* coupon_service_; - ObserverForTest* observer_for_testing_ = nullptr; WEB_CONTENTS_USER_DATA_KEY_DECL(); diff --git a/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl_unittest.cc b/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl_unittest.cc index 4ab33ffb7683ab..6a0e0d31249dbc 100644 --- a/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl_unittest.cc +++ b/chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl_unittest.cc @@ -4,10 +4,7 @@ #include "chrome/browser/ui/autofill/payments/offer_notification_bubble_controller_impl.h" -#include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" -#include "chrome/browser/commerce/commerce_feature_list.h" -#include "chrome/browser/commerce/coupons/coupon_service.h" #include "chrome/browser/ui/autofill/autofill_bubble_base.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -67,10 +64,6 @@ class OfferNotificationBubbleControllerImplTest BrowserWithTestWindowTest::SetUp(); AddTab(GURL("about:blank")); TestOfferNotificationBubbleControllerImpl::CreateForTesting(web_contents()); - static_cast( - TestOfferNotificationBubbleControllerImpl::FromWebContents( - web_contents())) - ->coupon_service_ = &mock_coupon_service_; } content::WebContents* web_contents() { @@ -82,14 +75,6 @@ class OfferNotificationBubbleControllerImplTest } protected: - class MockCouponService : public CouponService { - public: - MOCK_METHOD1(RecordCouponDisplayTimestamp, - void(const autofill::AutofillOfferData& offer)); - MOCK_METHOD1(GetCouponDisplayTimestamp, - base::Time(const autofill::AutofillOfferData& offer)); - }; - void ShowBubble(const AutofillOfferData* offer) { controller()->ShowOfferNotificationIfApplicable(offer, &card_); } @@ -119,14 +104,6 @@ class OfferNotificationBubbleControllerImplTest web_contents())); } - void SetCouponServiceForController( - TestOfferNotificationBubbleControllerImpl* controller, - CouponService* coupon_service) { - controller->coupon_service_ = coupon_service; - } - - MockCouponService mock_coupon_service_; - private: CreditCard card_ = test::GetCreditCard(); }; @@ -178,50 +155,6 @@ TEST_F(OfferNotificationBubbleControllerImplTest, EXPECT_EQ(&offer, controller()->GetOffer()); } -TEST_F(OfferNotificationBubbleControllerImplTest, - FreeListing_NotShownWithinTimeGap) { - base::HistogramTester histogram_tester; - AutofillOfferData offer = CreateTestOfferWithOrigins( - {GURL("https://www.example.com/first/").GetOrigin()}); - offer.promo_code = "FREEFALL1234"; - // Try to show a FreeListing coupon whose last shown timestamp is within time - // gap. - EXPECT_CALL(mock_coupon_service_, GetCouponDisplayTimestamp(offer)) - .Times(1) - .WillOnce(::testing::Return(base::Time::Now())); - EXPECT_CALL(mock_coupon_service_, RecordCouponDisplayTimestamp(offer)) - .Times(0); - - ShowBubble(&offer); - - histogram_tester.ExpectTotalCount( - "Autofill.OfferNotificationBubbleSuppressed.FreeListingCouponOffer", 1); - EXPECT_FALSE(controller()->GetOfferNotificationBubbleView()); -} - -TEST_F(OfferNotificationBubbleControllerImplTest, - FreeListing_ShownBeyondTimeGap) { - base::HistogramTester histogram_tester; - AutofillOfferData offer = CreateTestOfferWithOrigins( - {GURL("https://www.example.com/first/").GetOrigin()}); - offer.promo_code = "FREEFALL1234"; - // Try to show a FreeListing coupon whose last shown timestamp is beyond time - // gap. - EXPECT_CALL(mock_coupon_service_, GetCouponDisplayTimestamp(offer)) - .Times(1) - .WillOnce(::testing::Return(base::Time::Now() - - commerce::kCouponDisplayInterval.Get() - - base::Seconds(1))); - EXPECT_CALL(mock_coupon_service_, RecordCouponDisplayTimestamp(offer)) - .Times(1); - - ShowBubble(&offer); - - histogram_tester.ExpectTotalCount( - "Autofill.OfferNotificationBubbleSuppressed.FreeListingCouponOffer", 0); - EXPECT_TRUE(controller()->GetOfferNotificationBubbleView()); -} - class OfferNotificationBubbleControllerImplPrerenderTest : public OfferNotificationBubbleControllerImplTest { public: diff --git a/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_browsertest.cc b/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_browsertest.cc index 76cb1249bc7e5e..17d44c3c8df985 100644 --- a/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_browsertest.cc +++ b/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_browsertest.cc @@ -92,20 +92,6 @@ IN_PROC_BROWSER_TEST_F(OfferNotificationBubbleViewsBrowserTest, EXPECT_TRUE(GetOfferNotificationBubbleViews()); } -IN_PROC_BROWSER_TEST_F(OfferNotificationBubbleViewsBrowserTest, - PromoCodeOffer_FromCouponService_WithinTimeGap) { - const GURL orgin("https://www.example.com/"); - SetUpFreeListingCouponOfferDataForCouponService( - CreatePromoCodeOfferDataWithDomains({orgin})); - UpdateFreeListingCouponDisplayTime( - CreatePromoCodeOfferDataWithDomains({orgin})); - - NavigateTo("https://www.example.com/first/"); - - EXPECT_TRUE(IsIconVisible()); - EXPECT_FALSE(GetOfferNotificationBubbleViews()); -} - class OfferNotificationBubbleViewsBrowserTestWithoutPromoCodes : public OfferNotificationBubbleViewsTestBase { public: diff --git a/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.cc b/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.cc index 76e5222231449a..4020cdcea21a81 100644 --- a/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.cc +++ b/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.cc @@ -206,11 +206,6 @@ void OfferNotificationBubbleViewsTestBase::ResetEventWaiterForSequence( std::make_unique>(std::move(event_sequence)); } -void OfferNotificationBubbleViewsTestBase::UpdateFreeListingCouponDisplayTime( - std::unique_ptr offer) { - coupon_service_->RecordCouponDisplayTimestamp(*offer); -} - std::string OfferNotificationBubbleViewsTestBase::GetDefaultTestPromoCode() const { return kDefaultTestPromoCode; diff --git a/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.h b/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.h index eca0f2e60f0f23..0e02849fefa61c 100644 --- a/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.h +++ b/chrome/browser/ui/views/autofill/payments/offer_notification_bubble_views_test_base.h @@ -80,9 +80,6 @@ class OfferNotificationBubbleViewsTestBase void ResetEventWaiterForSequence(std::list event_sequence); - void UpdateFreeListingCouponDisplayTime( - std::unique_ptr offer); - void WaitForObservedEvent() { event_waiter_->Wait(); } PersonalDataManager* personal_data() { return personal_data_; } diff --git a/components/autofill/core/browser/autofill_metrics.cc b/components/autofill/core/browser/autofill_metrics.cc index 7c6acb069aba44..c69cac8fc38a6f 100644 --- a/components/autofill/core/browser/autofill_metrics.cc +++ b/components/autofill/core/browser/autofill_metrics.cc @@ -1235,24 +1235,6 @@ void AutofillMetrics::LogOfferNotificationBubblePromoCodeButtonClicked( base::UmaHistogramBoolean(histogram_name, true); } -// static -void AutofillMetrics::LogOfferNotificationBubbleSuppressed( - AutofillOfferData::OfferType offer_type) { - std::string histogram_name = "Autofill.OfferNotificationBubbleSuppressed."; - // Switch to different sub-histogram depending on offer type being suppressed. - // Card-linked offers will not be suppressed. - switch (offer_type) { - case AutofillOfferData::OfferType::FREE_LISTING_COUPON_OFFER: - histogram_name += "FreeListingCouponOffer"; - break; - case AutofillOfferData::OfferType::GPAY_CARD_LINKED_OFFER: - case AutofillOfferData::OfferType::UNKNOWN: - NOTREACHED(); - return; - } - base::UmaHistogramBoolean(histogram_name, true); -} - // static void AutofillMetrics::LogOfferNotificationInfoBarDeepLinkClicked() { base::RecordAction(base::UserMetricsAction( diff --git a/components/autofill/core/browser/autofill_metrics.h b/components/autofill/core/browser/autofill_metrics.h index abdb7564430e75..9603e9b12e3f0f 100644 --- a/components/autofill/core/browser/autofill_metrics.h +++ b/components/autofill/core/browser/autofill_metrics.h @@ -1224,8 +1224,6 @@ class AutofillMetrics { bool is_reshow); static void LogOfferNotificationBubblePromoCodeButtonClicked( AutofillOfferData::OfferType offer_type); - static void LogOfferNotificationBubbleSuppressed( - AutofillOfferData::OfferType offer_type); static void LogOfferNotificationInfoBarDeepLinkClicked(); static void LogOfferNotificationInfoBarResultMetric( OfferNotificationInfoBarResultMetric metric); diff --git a/tools/metrics/histograms/metadata/autofill/histograms.xml b/tools/metrics/histograms/metadata/autofill/histograms.xml index fe1ef5a9f5e6c3..17221640b31fb1 100644 --- a/tools/metrics/histograms/metadata/autofill/histograms.xml +++ b/tools/metrics/histograms/metadata/autofill/histograms.xml @@ -1987,21 +1987,6 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. - - jsaul@google.com - yuezhanggg@chromium.org - payments-autofill-team@google.com - chrome-shopping@google.com - - Records when the offer notification bubble should show but gets suppressed - due to reasons like show frequency rate control. The subhistogram is the - type of offer shown, such as GPay promo code offer or free-listing coupon - offer. - - - - siashah@chromium.org