Revert "[Coupon] Add rate control for FLC bubble"

This reverts commit 5640d8c. Reason for revert: Tests failing in linux MSAN bots Original change's description: > [Coupon] Add rate control for FLC bubble > > For FreeListing coupons, we don't want the infobar bubble to show at > the same frequency as GPay CLO offers. Instead, we want to limit > it to show once per day per domain, and only show the omnibox icon for > other times. This CL implements this rate control mechanism. More > details can be found in the doc in the bug link. > > Bug: 1256989 > Change-Id: I734a672a6c514ead79bc1545a54dacb5d281de6c > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212647 > Commit-Queue: Yue Zhang <yuezhanggg@chromium.org> > Reviewed-by: Evan Stade <estade@chromium.org> > Reviewed-by: Jared Saul <jsaul@google.com> > Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org> > Cr-Commit-Position: refs/heads/main@{#931743} Bug: 1256989, 1260238 Change-Id: I24f91601b0623056a0b829b190ef73cf5a281438 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3225340 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Owners-Override: Rakina Zata Amni <rakina@google.com> Cr-Commit-Position: refs/heads/main@{#931854}
marcosholgado · Oct 15, 2021 · a0cb006 · a0cb006
1 parent e3b84fb
commit a0cb006
Show file tree

Hide file tree

Showing 12 changed files with 4 additions and 165 deletions.
diff --git 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<base::TimeDelta> 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
@@ -40,6 +40,7 @@ CouponService::CouponService(std::unique_ptr<CouponDB> 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
@@ -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<CouponDB> 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
@@ -6,10 +6,6 @@
 
 #include <string>
 
-#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
@@ -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<GURL> 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
@@ -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*>(
-        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
@@ -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
@@ -206,11 +206,6 @@ void OfferNotificationBubbleViewsTestBase::ResetEventWaiterForSequence(
       std::make_unique<EventWaiter<DialogEvent>>(std::move(event_sequence));
 }
 
-void OfferNotificationBubbleViewsTestBase::UpdateFreeListingCouponDisplayTime(
-    std::unique_ptr<AutofillOfferData> 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
@@ -80,9 +80,6 @@ class OfferNotificationBubbleViewsTestBase
 
   void ResetEventWaiterForSequence(std::list<DialogEvent> event_sequence);
 
-  void UpdateFreeListingCouponDisplayTime(
-      std::unique_ptr<AutofillOfferData> 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
@@ -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
@@ -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
@@ -1987,21 +1987,6 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <token key="ShowType" variants="Autofill.PaymentBubble.Show"/>
 </histogram>
 
-<histogram name="Autofill.OfferNotificationBubbleSuppressed.{BubbleType}"
-    enum="BooleanSuppressed" expires_after="2022-04-01">
-  <owner>jsaul@google.com</owner>
-  <owner>yuezhanggg@chromium.org</owner>
-  <owner>payments-autofill-team@google.com</owner>
-  <owner>chrome-shopping@google.com</owner>
-  <summary>
-    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.
-  </summary>
-  <token key="BubbleType" variants="Autofill.OfferNotification.Type"/>
-</histogram>
-
 <histogram name="Autofill.OfferNotificationInfoBarOffer.{OfferType}"
     enum="BooleanShown" expires_after="2022-04-01">
   <owner>siashah@chromium.org</owner>