notification scheduler: Custom throttle layer implementation.

Enable client to customize config related to throttling - mainly
suppression duration and number of negative action count threshold
from client side API -
 std::unique_ptr<ThrottleConfig> GetThrottleConfig() {...}

This API will give client the option to override the fields related to
throttling in global config.

Internally a Delegate of ImpressionHistoryTracker will be created, and
the glue layer NotificationScheduler delegates this request.

Cleaned up throttled related parameters from schedule_params, and
impression struct and proto.

Some task need to do:
- Check consecutive negative actions instead of dismiss actions only.
- Need to write more detailed unit test to go through whole flow.
- May consider adding other way (positive impression - release throttle)
- Apply this layer to current existing clients including a more
functional web_ui for better test and play purpose.


Bug: 1053659
Change-Id: I3c767bd2be9259e74926a83ef5231ab1940b3972
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063253
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744409}

chrome/browser/notifications/proto/impression.proto

@@ -13,7 +13,7 @@ import "notification_data.proto";
 // Contains data to determine when a notification should be shown to the user
 // and the user impression towards this notification. Should match Impression in
 // impression_types.h.
-// Next tag: 9
+// Next tag: 8
 message Impression {
   // The type of user feedback from a displayed notification. Should match
   // UserFeedback in notification_scheduler_types.h.
@@ -63,7 +63,4 @@ message Impression {
 
   // Custom data associated with a notification.
   repeated CustomData custom_data = 7;
-
-  // Custom suppresion duration in ms.
-  optional int64 custom_suppression_duration_ms = 8;
 }

chrome/browser/notifications/proto/notification_entry.proto

@@ -13,7 +13,7 @@ import "impression.proto";
 import "notification_data.proto";
 
 // Defines scheduling and throttling details.
-// Next tag: 6
+// Next tag: 5
 message ScheduleParams {
   enum Priority {
     LOW = 0;
@@ -28,9 +28,6 @@ message ScheduleParams {
 
   // The end time of deliver time window for the notification.
   optional int64 deliver_time_end = 4;
-
-  // The custom suppression duration in ms.
-  optional int64 custom_suppression_duration_ms = 5;
 }
 
 // The notification entry that contains all data for a scheduled notification.

chrome/browser/notifications/scheduler/internal/impression_history_tracker.cc

@@ -47,11 +47,15 @@ ImpressionHistoryTrackerImpl::ImpressionHistoryTrackerImpl(
       config_(config),
       registered_clients_(std::move(registered_clients)),
       initialized_(false),
-      clock_(clock) {}
+      clock_(clock),
+      delegate_(nullptr) {}
 
 ImpressionHistoryTrackerImpl::~ImpressionHistoryTrackerImpl() = default;
 
-void ImpressionHistoryTrackerImpl::Init(InitCallback callback) {
+void ImpressionHistoryTrackerImpl::Init(Delegate* delegate,
+                                        InitCallback callback) {
+  DCHECK(delegate && !delegate_);
+  delegate_ = delegate;
   store_->InitAndLoad(
       base::BindOnce(&ImpressionHistoryTrackerImpl::OnStoreInitialized,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
@@ -61,8 +65,7 @@ void ImpressionHistoryTrackerImpl::AddImpression(
     SchedulerClientType type,
     const std::string& guid,
     const Impression::ImpressionResultMap& impression_mapping,
-    const Impression::CustomData& custom_data,
-    const base::Optional<base::TimeDelta>& custom_suppression_duration) {
+    const Impression::CustomData& custom_data) {
   DCHECK(initialized_);
   auto it = client_states_.find(type);
   if (it == client_states_.end())
@@ -71,7 +74,6 @@ void ImpressionHistoryTrackerImpl::AddImpression(
   Impression impression(type, guid, clock_->Now());
   impression.impression_mapping = impression_mapping;
   impression.custom_data = custom_data;
-  impression.custom_suppression_duration = custom_suppression_duration;
   it->second->impressions.emplace_back(std::move(impression));
 
   impression_map_.emplace(guid, &it->second->impressions.back());
@@ -219,10 +221,7 @@ void ImpressionHistoryTrackerImpl::AnalyzeImpressionHistory(
         dismisses.emplace_back(impression);
         PruneImpressionByCreateTime(
             &dismisses, impression->create_time - config_.dismiss_duration);
-
-        // Three consecutive dismisses will result in suppression.
-        CheckConsecutiveDismiss(client_state, &dismisses,
-                                config_.dismiss_count);
+        CheckConsecutiveDismiss(client_state, &dismisses);
         break;
       case UserFeedback::kClick:
         OnClickInternal(impression->guid, false /*update_db*/);
@@ -316,8 +315,13 @@ void ImpressionHistoryTrackerImpl::UpdateThrottling(ClientState* client_state,
 
 void ImpressionHistoryTrackerImpl::CheckConsecutiveDismiss(
     ClientState* client_state,
-    base::circular_deque<Impression*>* impressions,
-    size_t num_actions) {
+    base::circular_deque<Impression*>* impressions) {
+  auto custom_throttle_config =
+      delegate_->GetThrottleConfig(client_state->type);
+  size_t num_actions =
+      custom_throttle_config
+          ? custom_throttle_config->negative_action_count_threshold
+          : config_.dismiss_count;
   if (impressions->size() < num_actions)
     return;
 
@@ -371,12 +375,15 @@ void ImpressionHistoryTrackerImpl::ApplyNegativeImpression(
   SetNeedsUpdate(client_state->type, true);
   impression->integrated = true;
 
+  auto custom_throttle_config =
+      delegate_->GetThrottleConfig(client_state->type);
+
+  auto suppression_duration = custom_throttle_config
+                                  ? custom_throttle_config->suppression_duration
+                                  : config_.suppression_duration;
   // Suppress the notification, the user will not see this type of notification
   // for a while.
-  SuppressionInfo supression_info(
-      clock_->Now(), impression->custom_suppression_duration.has_value()
-                         ? impression->custom_suppression_duration.value()
-                         : config_.suppression_duration);
+  SuppressionInfo supression_info(clock_->Now(), suppression_duration);
   client_state->suppression_info = std::move(supression_info);
   client_state->current_max_daily_show = 0;
   stats::LogImpressionEvent(stats::ImpressionEvent::kNewSuppression);

chrome/browser/notifications/scheduler/internal/impression_history_tracker.h

@@ -20,6 +20,7 @@
 #include "chrome/browser/notifications/scheduler/internal/impression_types.h"
 #include "chrome/browser/notifications/scheduler/internal/scheduler_config.h"
 #include "chrome/browser/notifications/scheduler/public/impression_detail.h"
+#include "chrome/browser/notifications/scheduler/public/throttle_config.h"
 #include "chrome/browser/notifications/scheduler/public/user_action_handler.h"
 
 namespace notifications {
@@ -32,16 +33,28 @@ class ImpressionHistoryTracker : public UserActionHandler {
       std::map<SchedulerClientType, std::unique_ptr<ClientState>>;
   using InitCallback = base::OnceCallback<void(bool)>;
 
+  class Delegate {
+   public:
+    Delegate() = default;
+    virtual ~Delegate() = default;
+
+    // Get ThrottleConfig.
+    virtual std::unique_ptr<ThrottleConfig> GetThrottleConfig(
+        SchedulerClientType type) = 0;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Delegate);
+  };
+
   // Initializes the impression tracker.
-  virtual void Init(InitCallback callback) = 0;
+  virtual void Init(Delegate* delegate, InitCallback callback) = 0;
 
   // Add a new impression, called after the notification is shown.
   virtual void AddImpression(
       SchedulerClientType type,
       const std::string& guid,
       const Impression::ImpressionResultMap& impression_map,
-      const Impression::CustomData& custom_data,
-      const base::Optional<base::TimeDelta>& custom_suppression_duration) = 0;
+      const Impression::CustomData& custom_data) = 0;
 
   // Analyzes the impression history for all notification clients, and adjusts
   // the |current_max_daily_show|.
@@ -82,13 +95,11 @@ class ImpressionHistoryTrackerImpl : public ImpressionHistoryTracker {
 
  private:
   // ImpressionHistoryTracker implementation.
-  void Init(InitCallback callback) override;
+  void Init(Delegate* delegate, InitCallback callback) override;
   void AddImpression(SchedulerClientType type,
                      const std::string& guid,
                      const Impression::ImpressionResultMap& impression_mapping,
-                     const Impression::CustomData& custom_data,
-                     const base::Optional<base::TimeDelta>&
-                         custom_suppression_duration) override;
+                     const Impression::CustomData& custom_data) override;
   void AnalyzeImpressionHistory() override;
   void GetClientStates(std::map<SchedulerClientType, const ClientState*>*
                            client_states) const override;
@@ -119,8 +130,7 @@ class ImpressionHistoryTrackerImpl : public ImpressionHistoryTracker {
   // Check consecutive user actions, and generate impression result if no less
   // than |num_actions| count of user actions.
   void CheckConsecutiveDismiss(ClientState* client_state,
-                               base::circular_deque<Impression*>* impressions,
-                               size_t num_actions);
+                               base::circular_deque<Impression*>* impressions);
 
   // Generates user impression result.
   void GenerateImpressionResult(Impression* impression);
@@ -183,6 +193,9 @@ class ImpressionHistoryTrackerImpl : public ImpressionHistoryTracker {
   // The clock to provide the current timestamp.
   base::Clock* clock_;
 
+  // Delegate object.
+  Delegate* delegate_;
+
   base::WeakPtrFactory<ImpressionHistoryTrackerImpl> weak_ptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(ImpressionHistoryTrackerImpl);
 };

chrome/browser/notifications/scheduler/internal/impression_history_tracker_unittest.cc

@@ -84,10 +84,21 @@ class MockImpressionStore : public CollectionStore<ClientState> {
   DISALLOW_COPY_AND_ASSIGN(MockImpressionStore);
 };
 
+class MockDelegate : public ImpressionHistoryTracker::Delegate {
+ public:
+  MockDelegate() = default;
+  ~MockDelegate() final = default;
+  MOCK_METHOD1(GetThrottleConfig,
+               std::unique_ptr<ThrottleConfig>(SchedulerClientType));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockDelegate);
+};
+
 // TODO(xingliu): Add more test cases following the test doc.
 class ImpressionHistoryTrackerTest : public ::testing::Test {
  public:
-  ImpressionHistoryTrackerTest() : store_(nullptr) {}
+  ImpressionHistoryTrackerTest() : store_(nullptr), delegate_(nullptr) {}
   ~ImpressionHistoryTrackerTest() override = default;
 
   void SetUp() override {
@@ -101,7 +112,7 @@ class ImpressionHistoryTrackerTest : public ::testing::Test {
   void CreateTracker(const TestCase& test_case) {
     auto store = std::make_unique<MockImpressionStore>();
     store_ = store.get();
-
+    delegate_ = std::make_unique<MockDelegate>();
     impression_trakcer_ = std::make_unique<ImpressionHistoryTrackerImpl>(
         config_, test_case.registered_clients, std::move(store), &clock_);
   }
@@ -117,12 +128,13 @@ class ImpressionHistoryTrackerTest : public ::testing::Test {
               std::move(cb).Run(true, std::move(entries));
             }));
     base::RunLoop loop;
-    impression_trakcer_->Init(base::BindOnce(
-        [](base::RepeatingClosure closure, bool success) {
-          EXPECT_TRUE(success);
-          std::move(closure).Run();
-        },
-        loop.QuitClosure()));
+    impression_trakcer_->Init(
+        delegate_.get(), base::BindOnce(
+                             [](base::RepeatingClosure closure, bool success) {
+                               EXPECT_TRUE(success);
+                               std::move(closure).Run();
+                             },
+                             loop.QuitClosure()));
     loop.Run();
   }
 
@@ -149,6 +161,8 @@ class ImpressionHistoryTrackerTest : public ::testing::Test {
 
   const SchedulerConfig& config() const { return config_; }
   MockImpressionStore* store() { return store_; }
+  MockDelegate* delegate() { return delegate_.get(); }
+
   ImpressionHistoryTracker* tracker() { return impression_trakcer_.get(); }
   test::FakeClock* clock() { return &clock_; }
 
@@ -158,6 +172,7 @@ class ImpressionHistoryTrackerTest : public ::testing::Test {
   SchedulerConfig config_;
   std::unique_ptr<ImpressionHistoryTracker> impression_trakcer_;
   MockImpressionStore* store_;
+  std::unique_ptr<MockDelegate> delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(ImpressionHistoryTrackerTest);
 };
@@ -215,9 +230,9 @@ TEST_F(ImpressionHistoryTrackerTest, AddImpression) {
   InitTrackerWithData(test_case);
 
   // No-op for unregistered client.
-  tracker()->AddImpression(
-      SchedulerClientType::kTest2, kGuid2, Impression::ImpressionResultMap(),
-      Impression::CustomData(), base::nullopt /*custom_suppression_duration*/);
+  tracker()->AddImpression(SchedulerClientType::kTest2, kGuid2,
+                           Impression::ImpressionResultMap(),
+                           Impression::CustomData());
   VerifyClientStates(test_case);
 
   clock()->SetNow(kTimeStr);
@@ -225,16 +240,13 @@ TEST_F(ImpressionHistoryTrackerTest, AddImpression) {
   Impression::ImpressionResultMap impression_mapping = {
       {UserFeedback::kDismiss, ImpressionResult::kNegative}};
   Impression::CustomData custom_data = {{"url", "https://www.example.com"}};
-  auto custom_suppression_duration = base::TimeDelta::FromDays(56);
   EXPECT_CALL(*store(), Update(_, _, _));
   tracker()->AddImpression(SchedulerClientType::kTest1, kGuid1,
-                           impression_mapping, custom_data,
-                           custom_suppression_duration);
+                           impression_mapping, custom_data);
   Impression expected_impression(SchedulerClientType::kTest1, kGuid1,
                                  clock()->Now());
   expected_impression.impression_mapping = impression_mapping;
   expected_impression.custom_data = custom_data;
-  expected_impression.custom_suppression_duration = custom_suppression_duration;
   test_case.expected.back().impressions.emplace_back(expected_impression);
   VerifyClientStates(test_case);
   EXPECT_EQ(*tracker()->GetImpression(kGuid1), expected_impression);
@@ -303,7 +315,6 @@ struct UserActionTestParam {
   base::Optional<ActionButtonType> button_type;
   bool integrated = false;
   bool has_suppression = false;
-  base::Optional<base::TimeDelta> custom_suppression_duration;
   std::map<UserFeedback, ImpressionResult> impression_mapping;
 };
 
@@ -344,7 +355,6 @@ const UserActionTestParam kUserActionTestParams[] = {
      base::nullopt,
      true /*integrated*/,
      true /*has_suppression*/,
-     base::nullopt /*custom_suppression_duration*/,
      {{UserFeedback::kClick,
        ImpressionResult::kNegative}} /*impression_mapping*/},
 
@@ -355,10 +365,10 @@ const UserActionTestParam kUserActionTestParams[] = {
      base::nullopt,
      true /*integrated*/,
      true /*has_suppression*/,
-     base::TimeDelta::FromDays(2) /*custom_suppression_duration*/,
      {{UserFeedback::kClick,
        ImpressionResult::kNegative}} /*impression_mapping*/}};
 
+// TODO(hesen): Add test for custom suppression duration from client.
 // User actions like clicks should update the ClientState data accordingly.
 TEST_P(ImpressionHistoryTrackerUserActionTest, UserAction) {
   clock()->SetNow(base::Time::UnixEpoch());
@@ -366,8 +376,6 @@ TEST_P(ImpressionHistoryTrackerUserActionTest, UserAction) {
   Impression impression = CreateImpression(base::Time::Now(), kGuid1);
   DCHECK(!test_case.input.empty());
   impression.impression_mapping = GetParam().impression_mapping;
-  impression.custom_suppression_duration =
-      GetParam().custom_suppression_duration;
   test_case.input.front().impressions.emplace_back(impression);
 
   impression.impression = GetParam().impression_result;
@@ -378,10 +386,7 @@ TEST_P(ImpressionHistoryTrackerUserActionTest, UserAction) {
   test_case.expected.front().impressions.emplace_back(impression);
   if (GetParam().has_suppression) {
     test_case.expected.front().suppression_info =
-        SuppressionInfo(base::Time::UnixEpoch(),
-                        GetParam().custom_suppression_duration.has_value()
-                            ? GetParam().custom_suppression_duration.value()
-                            : config().suppression_duration);
+        SuppressionInfo(base::Time::UnixEpoch(), config().suppression_duration);
   }
 
   CreateTracker(test_case);

chrome/browser/notifications/scheduler/internal/impression_types.h

@@ -66,10 +66,6 @@ struct Impression {
   // Custom data associated with a notification. Send back to the client when
   // the user interacts with the notification.
   CustomData custom_data;
-
-  // Custom suppresion duration in days. It will override |suppression_duration|
-  // in config.
-  base::Optional<base::TimeDelta> custom_suppression_duration;
 };
 
 // Contains details about supression and recovery after suppression expired.

chrome/browser/notifications/scheduler/internal/notification_scheduler.cc

@@ -46,7 +46,9 @@ class InitHelper {
   // Initializes subsystems in notification scheduler, |callback| will be
   // invoked if all initializations finished or anyone of them failed. The
   // object should be destroyed along with the |callback|.
-  void Init(NotificationSchedulerContext* context, InitCallback callback) {
+  void Init(NotificationSchedulerContext* context,
+            ImpressionHistoryTracker::Delegate* delegate,
+            InitCallback callback) {
     // TODO(xingliu): Initialize the databases in parallel, we currently
     // initialize one by one to work around a shared db issue. See
     // https://crbug.com/978680.
@@ -54,8 +56,8 @@ class InitHelper {
     callback_ = std::move(callback);
 
     context_->impression_tracker()->Init(
-        base::BindOnce(&InitHelper::OnImpressionTrackerInitialized,
-                       weak_ptr_factory_.GetWeakPtr()));
+        delegate, base::BindOnce(&InitHelper::OnImpressionTrackerInitialized,
+                                 weak_ptr_factory_.GetWeakPtr()));
   }
 
  private:
@@ -154,8 +156,7 @@ class DisplayHelper {
     // Tracks user impression on the notification to be shown.
     context_->impression_tracker()->AddImpression(
         entry->type, entry->guid, entry->schedule_params.impression_mapping,
-        updated_notification_data->custom_data,
-        entry->schedule_params.custom_suppression_duration);
+        updated_notification_data->custom_data);
 
     stats::LogNotificationShow(*updated_notification_data, entry->type);
 
@@ -191,9 +192,10 @@ class DisplayHelper {
 };
 
 // Implementation of NotificationScheduler.
-class NotificationSchedulerImpl : public NotificationScheduler {
+class NotificationSchedulerImpl : public NotificationScheduler,
+                                  public ImpressionHistoryTracker::Delegate {
  public:
-  NotificationSchedulerImpl(
+  explicit NotificationSchedulerImpl(
       std::unique_ptr<NotificationSchedulerContext> context)
       : context_(std::move(context)) {}
 
@@ -203,7 +205,7 @@ class NotificationSchedulerImpl : public NotificationScheduler {
   // NotificationScheduler implementation.
   void Init(InitCallback init_callback) override {
     init_helper_ = std::make_unique<InitHelper>();
-    init_helper_->Init(context_.get(),
+    init_helper_->Init(context_.get(), this,
                        base::BindOnce(&NotificationSchedulerImpl::OnInitialized,
                                       weak_ptr_factory_.GetWeakPtr(),
                                       std::move(init_callback)));
@@ -363,6 +365,12 @@ class NotificationSchedulerImpl : public NotificationScheduler {
     client->OnUserAction(client_action_data);
   }
 
+  std::unique_ptr<ThrottleConfig> GetThrottleConfig(
+      SchedulerClientType type) override {
+    auto* client = context_->client_registrar()->GetClient(type);
+    return client ? client->GetThrottleConfig() : nullptr;
+  }
+
   std::unique_ptr<NotificationSchedulerContext> context_;
   std::unique_ptr<InitHelper> init_helper_;
   std::unique_ptr<DisplayHelper> display_helper_;

chrome/browser/notifications/scheduler/internal/notification_scheduler_unittest.cc

@@ -86,8 +86,9 @@ class NotificationSchedulerTest : public testing::Test {
 
  protected:
   void Init() {
-    EXPECT_CALL(*impression_tracker(), Init(_))
-        .WillOnce(Invoke([&](ImpressionHistoryTracker::InitCallback callback) {
+    EXPECT_CALL(*impression_tracker(), Init(_, _))
+        .WillOnce(Invoke([&](ImpressionHistoryTracker::Delegate* delegate,
+                             ImpressionHistoryTracker::InitCallback callback) {
           std::move(callback).Run(true);
         }));
 
@@ -159,8 +160,9 @@ TEST_F(NotificationSchedulerTest, InitSuccess) {
 
 // Tests the case when impression tracker failed to initialize.
 TEST_F(NotificationSchedulerTest, InitImpressionTrackerFailed) {
-  EXPECT_CALL(*impression_tracker(), Init(_))
-      .WillOnce(Invoke([](ImpressionHistoryTracker::InitCallback callback) {
+  EXPECT_CALL(*impression_tracker(), Init(_, _))
+      .WillOnce(Invoke([](ImpressionHistoryTracker::Delegate* delegate,
+                          ImpressionHistoryTracker::InitCallback callback) {
         // Impression tracker failed to load.
         std::move(callback).Run(false);
       }));
@@ -179,8 +181,9 @@ TEST_F(NotificationSchedulerTest, InitImpressionTrackerFailed) {
 
 // Tests the case when scheduled notification manager failed to initialize.
 TEST_F(NotificationSchedulerTest, InitScheduledNotificationManagerFailed) {
-  EXPECT_CALL(*impression_tracker(), Init(_))
-      .WillOnce(Invoke([](ImpressionHistoryTracker::InitCallback callback) {
+  EXPECT_CALL(*impression_tracker(), Init(_, _))
+      .WillOnce(Invoke([](ImpressionHistoryTracker::Delegate* delegate,
+                          ImpressionHistoryTracker::InitCallback callback) {
         std::move(callback).Run(true);
       }));
 
@@ -315,7 +318,7 @@ TEST_F(NotificationSchedulerTest, BackgroundTaskStartShowNotification) {
       .WillOnce(SetArgPointee<2>(result));
 
   EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
-  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _, _));
+  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _));
   EXPECT_CALL(*notification_manager(), DisplayNotification(_, _))
       .WillOnce(
           Invoke([&](const std::string& guid,
@@ -345,7 +348,7 @@ TEST_F(NotificationSchedulerTest, BackgroundTaskStartNoEntry) {
       .WillOnce(SetArgPointee<2>(result));
 
   EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
-  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _, _)).Times(0);
+  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _)).Times(0);
   EXPECT_CALL(*display_agent(), ShowNotification(_, _)).Times(0);
   EXPECT_CALL(*client(), BeforeShowNotification(_, _)).Times(0);
   EXPECT_CALL(*notification_manager(), DisplayNotification(_, _))
@@ -372,7 +375,7 @@ TEST_F(NotificationSchedulerTest, BackgroundTaskStartNoClient) {
       .WillOnce(SetArgPointee<2>(result));
 
   EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
-  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _, _)).Times(0);
+  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _)).Times(0);
   EXPECT_CALL(*display_agent(), ShowNotification(_, _)).Times(0);
   EXPECT_CALL(*client(), BeforeShowNotification(_, _)).Times(0);
   EXPECT_CALL(*notification_manager(), DisplayNotification(_, _))
@@ -411,7 +414,7 @@ TEST_F(NotificationSchedulerTest, ClientDropNotification) {
           }));
 
   EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
-  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _, _)).Times(0);
+  EXPECT_CALL(*impression_tracker(), AddImpression(_, _, _, _)).Times(0);
   EXPECT_CALL(*display_agent(), ShowNotification(_, _)).Times(0);
 
   OnStartTask();

chrome/browser/notifications/scheduler/internal/proto_conversion.cc

@@ -282,11 +282,6 @@ void ScheduleParamsToProto(ScheduleParams* params,
     proto->set_deliver_time_end(
         TimeToMilliseconds(params->deliver_time_end.value()));
   }
-
-  if (params->custom_suppression_duration.has_value()) {
-    proto->set_custom_suppression_duration_ms(
-        TimeDeltaToMilliseconds(params->custom_suppression_duration.value()));
-  }
 }
 
 // Converts ScheduleParams from proto buffer type.
@@ -309,10 +304,6 @@ void ScheduleParamsFromProto(proto::ScheduleParams* proto,
   if (proto->has_deliver_time_end()) {
     params->deliver_time_end = MillisecondsToTime(proto->deliver_time_end());
   }
-  if (proto->has_custom_suppression_duration_ms()) {
-    params->custom_suppression_duration =
-        MillisecondsToTimeDelta(proto->custom_suppression_duration_ms());
-  }
 }
 
 }  // namespace
@@ -352,12 +343,6 @@ void ClientStateToProto(ClientState* client_state,
       data->set_key(pair.first);
       data->set_value(pair.second);
     }
-
-    if (impression.custom_suppression_duration.has_value()) {
-      impression_ptr->set_custom_suppression_duration_ms(
-          TimeDeltaToMilliseconds(
-              impression.custom_suppression_duration.value()));
-    }
   }
 
   if (client_state->suppression_info.has_value()) {
@@ -403,11 +388,6 @@ void ClientStateFromProto(proto::ClientState* proto,
       impression.custom_data.emplace(pair.key(), pair.value());
     }
 
-    if (proto_impression.has_custom_suppression_duration_ms()) {
-      impression.custom_suppression_duration = MillisecondsToTimeDelta(
-          proto_impression.custom_suppression_duration_ms());
-    }
-
     client_state->impressions.emplace_back(std::move(impression));
   }
 

chrome/browser/notifications/scheduler/internal/proto_conversion_unittest.cc

@@ -143,10 +143,6 @@ TEST(ProtoConversionTest, ImpressionProtoConversion) {
   // Verify custom data.
   first_impression.custom_data = {{"url", "https://www.example.com"}};
   TestClientStateConversion(&client_state);
-
-  // Verify custom suppression duration.
-  first_impression.custom_suppression_duration = base::TimeDelta::FromDays(3);
-  TestClientStateConversion(&client_state);
 }
 
 // Verifies multiple impressions are serialized correctly.
@@ -199,8 +195,6 @@ TEST(ProtoConversionTest, NotificationEntryConversion) {
   entry.schedule_params.deliver_time_start = entry.create_time;
   entry.schedule_params.deliver_time_end =
       entry.create_time + base::TimeDelta::FromMinutes(10);
-  entry.schedule_params.custom_suppression_duration =
-      base::TimeDelta::FromDays(3);
   TestNotificationEntryConversion(&entry);
 }
 

chrome/browser/notifications/scheduler/internal/webui_client.cc

@@ -29,4 +29,8 @@ void WebUIClient::OnUserAction(const UserActionData& action_data) {
   NOTIMPLEMENTED();
 }
 
+std::unique_ptr<ThrottleConfig> WebUIClient::GetThrottleConfig() {
+  return nullptr;
+}
+
 }  // namespace notifications

chrome/browser/notifications/scheduler/internal/webui_client.h

@@ -27,6 +27,7 @@ class WebUIClient : public NotificationSchedulerClient {
   void OnSchedulerInitialized(bool success,
                               std::set<std::string> guids) override;
   void OnUserAction(const UserActionData& action_data) override;
+  std::unique_ptr<ThrottleConfig> GetThrottleConfig() override;
 
   DISALLOW_COPY_AND_ASSIGN(WebUIClient);
 };

chrome/browser/notifications/scheduler/notification_schedule_service_browsertest.cc

@@ -61,6 +61,10 @@ class TestClient : public NotificationSchedulerClient {
 
   void OnUserAction(const UserActionData& action_data) override {}
 
+  std::unique_ptr<ThrottleConfig> GetThrottleConfig() override {
+    return nullptr;
+  }
+
   // Any NotificationData received before showing the notification.
   std::vector<NotificationData> shown_notification_data_;
 

chrome/browser/notifications/scheduler/public/BUILD.gn

@@ -34,6 +34,8 @@ source_set("public") {
     "schedule_params.h",
     "schedule_service_utils.cc",
     "schedule_service_utils.h",
+    "throttle_config.cc",
+    "throttle_config.h",
     "user_action_handler.h",
   ]
 

chrome/browser/notifications/scheduler/public/notification_scheduler_client.h

@@ -15,6 +15,7 @@
 #include "base/optional.h"
 #include "chrome/browser/notifications/scheduler/public/notification_data.h"
 #include "chrome/browser/notifications/scheduler/public/notification_scheduler_types.h"
+#include "chrome/browser/notifications/scheduler/public/throttle_config.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 namespace notifications {
@@ -45,6 +46,10 @@ class NotificationSchedulerClient {
   // Called when the user interacts with the notification.
   virtual void OnUserAction(const UserActionData& action_data) = 0;
 
+  // Used to pull customized throttle config from client and may override global
+  // config in the framework. Return |nullptr| if no customization is needed.
+  virtual std::unique_ptr<ThrottleConfig> GetThrottleConfig() = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(NotificationSchedulerClient);
 };

chrome/browser/notifications/scheduler/public/schedule_params.cc

@@ -14,8 +14,7 @@ bool ScheduleParams::operator==(const ScheduleParams& other) const {
   return priority == other.priority &&
          impression_mapping == other.impression_mapping &&
          deliver_time_start == other.deliver_time_start &&
-         deliver_time_end == other.deliver_time_end &&
-         custom_suppression_duration == other.custom_suppression_duration;
+         deliver_time_end == other.deliver_time_end;
 }
 
 ScheduleParams::~ScheduleParams() = default;

chrome/browser/notifications/scheduler/public/schedule_params.h

@@ -45,11 +45,6 @@ struct ScheduleParams {
   // The end time of the deliver time window of the notification. Use in pair
   // with |deliver_time_start|.
   base::Optional<base::Time> deliver_time_end;
-
-  // Support a custom suppression duration(in days) for the notification.
-  // If client sets this field, it will override |suppression_duration| in
-  // config.
-  base::Optional<base::TimeDelta> custom_suppression_duration;
 };
 
 }  // namespace notifications

chrome/browser/notifications/scheduler/public/throttle_config.cc

+// Copyright 2020 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 "chrome/browser/notifications/scheduler/public/throttle_config.h"
+
+namespace notifications {
+
+ThrottleConfig::ThrottleConfig() : negative_action_count_threshold(0) {}
+
+ThrottleConfig::ThrottleConfig(const ThrottleConfig& other) = default;
+
+bool ThrottleConfig::operator==(const ThrottleConfig& other) const {
+  return suppression_duration == other.suppression_duration &&
+         negative_action_count_threshold ==
+             other.negative_action_count_threshold;
+}
+
+ThrottleConfig::~ThrottleConfig() = default;
+
+}  // namespace notifications

chrome/browser/notifications/scheduler/public/throttle_config.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_THROTTLE_CONFIG_H_
+#define CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_THROTTLE_CONFIG_H_
+
+#include "base/time/time.h"
+
+namespace notifications {
+
+// Specifies the throttling related configuration for each client.
+struct ThrottleConfig {
+  ThrottleConfig();
+  ThrottleConfig(const ThrottleConfig& other);
+  bool operator==(const ThrottleConfig& other) const;
+  ~ThrottleConfig();
+
+  // Support a custom suppression duration(in days) for the notification.
+  // If client sets this field, it will override |suppression_duration| in
+  // global config.
+  base::TimeDelta suppression_duration;
+
+  // Maxmium number of consecutive negative actions to trigger negative
+  // impression event.
+  // If client sets this field, it will override |dismiss_count| in global
+  // config.
+  int negative_action_count_threshold;
+};
+
+}  // namespace notifications
+
+#endif  // CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_THROTTLE_CONFIG_H_

chrome/browser/notifications/scheduler/test/mock_impression_history_tracker.h

@@ -19,13 +19,12 @@ class MockImpressionHistoryTracker : public ImpressionHistoryTracker {
   MockImpressionHistoryTracker();
   ~MockImpressionHistoryTracker() override;
 
-  MOCK_METHOD1(Init, void(ImpressionHistoryTracker::InitCallback));
-  MOCK_METHOD5(AddImpression,
+  MOCK_METHOD2(Init, void(Delegate*, ImpressionHistoryTracker::InitCallback));
+  MOCK_METHOD4(AddImpression,
                void(SchedulerClientType,
                     const std::string&,
                     const Impression::ImpressionResultMap&,
-                    const Impression::CustomData&,
-                    const base::Optional<base::TimeDelta>&));
+                    const Impression::CustomData&));
   MOCK_METHOD0(AnalyzeImpressionHistory, void());
   MOCK_CONST_METHOD1(GetClientStates,
                      void(std::map<SchedulerClientType, const ClientState*>*));

chrome/browser/notifications/scheduler/test/mock_notification_scheduler_client.h

@@ -24,6 +24,7 @@ class MockNotificationSchedulerClient : public NotificationSchedulerClient {
                     NotificationDataCallback));
   MOCK_METHOD2(OnSchedulerInitialized, void(bool, std::set<std::string>));
   MOCK_METHOD1(OnUserAction, void(const UserActionData&));
+  MOCK_METHOD0(GetThrottleConfig, std::unique_ptr<ThrottleConfig>());
 };
 
 }  // namespace test

chrome/browser/notifications/scheduler/test/test_utils.cc

@@ -110,9 +110,6 @@ std::string DebugString(const NotificationEntry* entry) {
     stream << " \n large_icons_id:"
            << entry->icons_uuid.at(IconType::kLargeIcon);
 
-  if (entry->schedule_params.custom_suppression_duration.has_value())
-    stream << " \n custom_suppression_duration:"
-           << entry->schedule_params.custom_suppression_duration.value();
   return stream.str();
 }
 
@@ -149,10 +146,6 @@ std::string DebugString(const ClientState* client_state) {
              << " value: " << pair.second;
     }
 
-    if (impression.custom_suppression_duration.has_value()) {
-      stream << " \n custom suppression duration  "
-             << impression.custom_suppression_duration.value();
-    }
     log += stream.str();
   }
 

chrome/browser/offline_pages/prefetch/notifications/prefetch_notification_client.cc

@@ -33,4 +33,9 @@ void PrefetchNotificationClient::OnUserAction(
   NOTIMPLEMENTED();
 }
 
+std::unique_ptr<notifications::ThrottleConfig>
+PrefetchNotificationClient::GetThrottleConfig() {
+  return nullptr;
+}
+
 }  // namespace offline_pages

chrome/browser/offline_pages/prefetch/notifications/prefetch_notification_client.h

@@ -22,6 +22,7 @@ class PrefetchNotificationClient
  public:
   using NotificationData = notifications::NotificationData;
   using UserActionData = notifications::UserActionData;
+  using ThrottleConfig = notifications::ThrottleConfig;
   using GetServiceCallback =
       base::RepeatingCallback<PrefetchNotificationService*()>;
 
@@ -36,6 +37,7 @@ class PrefetchNotificationClient
   void OnSchedulerInitialized(bool success,
                               std::set<std::string> guids) override;
   void OnUserAction(const UserActionData& action_data) override;
+  std::unique_ptr<ThrottleConfig> GetThrottleConfig() override;
 
   GetServiceCallback get_service_callback_;
 

chrome/browser/updates/update_notification_client.cc

@@ -59,4 +59,9 @@ void UpdateNotificationClient::OnUserAction(const UserActionData& action_data) {
   }
 }
 
+std::unique_ptr<notifications::ThrottleConfig>
+UpdateNotificationClient::GetThrottleConfig() {
+  return nullptr;
+}
+
 }  // namespace updates

chrome/browser/updates/update_notification_client.h

@@ -23,6 +23,7 @@ class UpdateNotificationClient
  public:
   using NotificationData = notifications::NotificationData;
   using UserActionData = notifications::UserActionData;
+  using ThrottleConfig = notifications::ThrottleConfig;
   using GetServiceCallback =
       base::RepeatingCallback<UpdateNotificationService*()>;
 
@@ -37,6 +38,7 @@ class UpdateNotificationClient
   void OnSchedulerInitialized(bool success,
                               std::set<std::string> guids) override;
   void OnUserAction(const UserActionData& action_data) override;
+  std::unique_ptr<ThrottleConfig> GetThrottleConfig() override;
 
   GetServiceCallback get_service_callback_;