[Update notification service]: Refactor to adopt new scheduler features.

- Use built-in last shown notification timestamp to check whether to
show next one.
- Use built-in stats like number of throttle events to extend the
throttle interval.
- Remove all bridge/SharePref getter/setter methods that no longer needed.
- Add a max_interval as a ceiling to the throttle interval to avoid
unlimitted extension.

Change-Id: I9eaca883dac245d953351652d6782a4c13404fce
Bug: 1013685, 1043250
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2300632
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792441}

chrome/android/java/src/org/chromium/chrome/browser/omaha/notification/UpdateNotificationServiceBridge.java

@@ -8,10 +8,8 @@ import static org.chromium.chrome.browser.omaha.UpdateConfigs.getUpdateNotificat
 import static org.chromium.chrome.browser.omaha.UpdateConfigs.getUpdateNotificationTitle;
 import static org.chromium.chrome.browser.omaha.UpdateStatusProvider.UpdateState.INLINE_UPDATE_AVAILABLE;
 import static org.chromium.chrome.browser.omaha.UpdateStatusProvider.UpdateState.UPDATE_AVAILABLE;
-import static org.chromium.chrome.browser.omaha.notification.UpdateNotificationControllerImpl.PREF_LAST_TIME_UPDATE_NOTIFICATION_KEY;
 
 import android.content.Intent;
-import android.content.SharedPreferences;
 
 import androidx.annotation.Nullable;
 
@@ -25,7 +23,6 @@ import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.lifecycle.Destroyable;
-import org.chromium.chrome.browser.omaha.OmahaBase;
 import org.chromium.chrome.browser.omaha.UpdateStatusProvider;
 import org.chromium.chrome.browser.omaha.UpdateStatusProvider.UpdateState;
 
@@ -36,11 +33,6 @@ import org.chromium.chrome.browser.omaha.UpdateStatusProvider.UpdateState;
  */
 @JNINamespace("updates")
 public class UpdateNotificationServiceBridge implements UpdateNotificationController, Destroyable {
-    private static final String PREF_UPDATE_NOTIFICATION_THROTTLE_INTERVAL_KEY =
-            "pref_update_notification_throttle_interval_key";
-    private static final String PREF_UPDATE_NOTIFICATION_USER_DISMISS_COUNT_KEY =
-            "pref_update_notification_user_dismiss_count_key";
-
     private final Callback<UpdateStatusProvider.UpdateStatus> mObserver = status -> {
         mUpdateStatus = status;
         processUpdateStatus();
@@ -104,66 +96,6 @@ public class UpdateNotificationServiceBridge implements UpdateNotificationContro
                 boolean shouldShowImmediately);
     }
 
-    @CalledByNative
-    public static long getLastShownTimeStamp() {
-        SharedPreferences preferences = OmahaBase.getSharedPreferences();
-        return preferences.getLong(PREF_LAST_TIME_UPDATE_NOTIFICATION_KEY, 0);
-    }
-
-    @CalledByNative
-    private static void updateLastShownTimeStamp(long timestamp) {
-        SharedPreferences preferences = OmahaBase.getSharedPreferences();
-        SharedPreferences.Editor editor = preferences.edit();
-        editor.putLong(PREF_LAST_TIME_UPDATE_NOTIFICATION_KEY, timestamp);
-        editor.apply();
-    }
-
-    /**
-     * Gets the throttle interval in milliseconds from {@link SharedPreferences}.
-     * return 0 if not exists.
-     */
-    @CalledByNative
-    public static long getThrottleInterval() {
-        SharedPreferences preferences = OmahaBase.getSharedPreferences();
-        return preferences.getLong(PREF_UPDATE_NOTIFICATION_THROTTLE_INTERVAL_KEY, 0);
-    }
-
-    /**
-     * Updates the throttle interval to show Chrome update notification in {@link
-     * SharedPreferences}.
-     * @param interval Throttle interval in milliseconds.
-     */
-    @CalledByNative
-    private static void updateThrottleInterval(long interval) {
-        SharedPreferences preferences = OmahaBase.getSharedPreferences();
-        SharedPreferences.Editor editor = preferences.edit();
-        editor.putLong(PREF_UPDATE_NOTIFICATION_THROTTLE_INTERVAL_KEY, interval);
-        editor.apply();
-    }
-
-    /**
-     * Gets the number of users consecutive dismiss or negative button action on update notification
-     * from {@link SharedPreferences}.
-     */
-    @CalledByNative
-    public static int getNegativeActionCount() {
-        SharedPreferences preferences = OmahaBase.getSharedPreferences();
-        return preferences.getInt(PREF_UPDATE_NOTIFICATION_USER_DISMISS_COUNT_KEY, 0);
-    }
-
-    /**
-     * Updates the number to record users consecutive dismiss or negative button click action on
-     * update notification in {@link SharedPreferences}.
-     * @param count A number of users consecutive dimiss action.
-     */
-    @CalledByNative
-    private static void updateNegativeActionCount(int count) {
-        SharedPreferences preferences = OmahaBase.getSharedPreferences();
-        SharedPreferences.Editor editor = preferences.edit();
-        editor.putInt(PREF_UPDATE_NOTIFICATION_USER_DISMISS_COUNT_KEY, count);
-        editor.apply();
-    }
-
     /**
      * Launches Chrome activity depends on {@link UpdateState}.
      * @param state An enum value of {@link UpdateState} stored in native side schedule service.

chrome/browser/updates/internal/update_notification_service_impl.h

@@ -10,11 +10,11 @@
 #include <memory>
 
 #include "base/memory/weak_ptr.h"
+#include "base/time/clock.h"
 
 namespace notifications {
 struct ClientOverview;
 class NotificationScheduleService;
-struct ScheduleParams;
 }  // namespace notifications
 
 namespace updates {
@@ -28,35 +28,43 @@ class UpdateNotificationServiceImpl : public UpdateNotificationService {
   UpdateNotificationServiceImpl(
       notifications::NotificationScheduleService* schedule_service,
       std::unique_ptr<UpdateNotificationConfig> config,
-      std::unique_ptr<UpdateNotificationServiceBridge> bridge);
+      std::unique_ptr<UpdateNotificationServiceBridge> bridge,
+      base::Clock* clock);
   ~UpdateNotificationServiceImpl() override;
 
  private:
   // UpdateNotificationService implementation.
   void Schedule(UpdateNotificationInfo data) override;
-  bool IsReadyToDisplay() const override;
-  void OnUserDismiss() override;
   void OnUserClick(const ExtraData& extra) override;
-  void OnUserClickButton(bool is_positive_button) override;
-
-  // Called after querying the |ClientOverview| struct from scheduler system
-  // completed.
-  void OnClientOverviewQueried(UpdateNotificationInfo data,
-                               notifications::ClientOverview overview);
-
-  // Build notification ScheduleParams for update notification.
-  notifications::ScheduleParams BuildScheduleParams(
-      bool should_show_immediately);
-
-  // Return throttle interval from Android shared preference if exists,
-  // otherwise return the default interval from config.
-  base::TimeDelta GetThrottleInterval() const;
-
-  // Apply linear throttle logic.
-  void ApplyLinearThrottle();
-
-  // Apply negative action including dismiss and unhelpful button.
-  void ApplyNegativeAction();
+  void GetThrottleConfig(ThrottleConfigCallback callback) override;
+  void BeforeShowNotification(
+      std::unique_ptr<notifications::NotificationData> notification_data,
+      NotificationDataCallback callback) override;
+
+  // Calculates the params of throttle config depends on |client_overview|, and
+  // reply to the |callback|. suppresion duration length starts with
+  // |init_interval| read from config, and the increase is proportional to
+  // the number of suppression events, until reach the |max_interva|l in config.
+  void DetermineThrottleConfig(ThrottleConfigCallback callback,
+                               notifications::ClientOverview client_overview);
+
+  // Called before displaying the notification, and will reply nullptr to
+  // callback if it's not the right time to show this upcoming notification.
+  void MaybeShowNotification(
+      std::unique_ptr<notifications::NotificationData> notification_data,
+      NotificationDataCallback callback,
+      notifications::ClientOverview client_overview);
+
+  // Return true if |current timestamp - last notification shown timestamp| is
+  // smailler than interval, which is based on the config and number of throttle
+  // events read from |client_overview|.
+  bool TooSoonForNextNotification(
+      const notifications::ClientOverview& client_overview);
+
+  // Called before using notification schedule service to actually schedule.
+  // Will not schedule if already has too many notifications cached.
+  void ScheduleInternal(UpdateNotificationInfo data,
+                        notifications::ClientOverview client_overview);
 
   // Used to schedule notification to show in the future. Must outlive this
   // class.
@@ -66,9 +74,9 @@ class UpdateNotificationServiceImpl : public UpdateNotificationService {
 
   std::unique_ptr<UpdateNotificationServiceBridge> bridge_;
 
-  base::WeakPtrFactory<UpdateNotificationServiceImpl> weak_ptr_factory_{this};
+  base::Clock* clock_;
 
-  DISALLOW_COPY_AND_ASSIGN(UpdateNotificationServiceImpl);
+  base::WeakPtrFactory<UpdateNotificationServiceImpl> weak_ptr_factory_{this};
 };
 
 }  // namespace updates

chrome/browser/updates/internal/update_notification_service_impl_unittest.cc

@@ -7,14 +7,28 @@
 #include <memory>
 #include <utility>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/simple_test_clock.h"
 #include "base/test/task_environment.h"
+#include "chrome/browser/notifications/scheduler/public/schedule_service_utils.h"
+#include "chrome/browser/notifications/scheduler/public/throttle_config.h"
 #include "chrome/browser/notifications/scheduler/test/mock_notification_schedule_service.h"
 #include "chrome/browser/updates/test/mock_update_notification_service_bridge.h"
 #include "chrome/browser/updates/update_notification_config.h"
+#include "chrome/browser/updates/update_notification_info.h"
 
 namespace updates {
 namespace {
 
+using testing::_;
+using ::testing::Invoke;
+
+const auto kTestTitle = base::UTF8ToUTF16("hello");
+const auto kTestMessage = base::UTF8ToUTF16("world");
+
 class UpdateNotificationServiceImplTest : public testing::Test {
  public:
   UpdateNotificationServiceImplTest() : bridge_(nullptr), config_(nullptr) {}
@@ -31,21 +45,22 @@ class UpdateNotificationServiceImplTest : public testing::Test {
     bridge_ = bridge.get();
     auto config = UpdateNotificationConfig::Create();
     config_ = config.get();
-
+    config_->is_enabled = true;
     service_ = std::make_unique<updates::UpdateNotificationServiceImpl>(
-        scheduler_.get(), std::move(config), std::move(bridge));
+        scheduler_.get(), std::move(config), std::move(bridge), &clock_);
   }
 
  protected:
   notifications::test::MockNotificationScheduleService* scheduler() {
     return scheduler_.get();
   }
-
   test::MockUpdateNotificationServiceBridge* bridge() { return bridge_; }
-
   UpdateNotificationService* service() { return service_.get(); }
+  UpdateNotificationConfig* config() { return config_; }
+  base::SimpleTestClock* clock() { return &clock_; }
 
  private:
+  base::SimpleTestClock clock_;
   base::test::TaskEnvironment task_environment_;
   test::MockUpdateNotificationServiceBridge* bridge_;
   std::unique_ptr<notifications::test::MockNotificationScheduleService>
@@ -55,5 +70,138 @@ class UpdateNotificationServiceImplTest : public testing::Test {
   std::unique_ptr<UpdateNotificationService> service_;
 };
 
+MATCHER_P(NotificationParamsEq,
+          expected,
+          "Compare the notification params except GUID") {
+  EXPECT_EQ(arg->schedule_params, expected->schedule_params);
+  EXPECT_EQ(arg->notification_data, expected->notification_data);
+  EXPECT_EQ(arg->enable_ihnr_buttons, expected->enable_ihnr_buttons);
+  EXPECT_EQ(arg->type, expected->type);
+  return true;
+}
+
+TEST_F(UpdateNotificationServiceImplTest, Schedule) {
+  base::Time fake_now;
+  EXPECT_TRUE(base::Time::FromString("05/18/20 01:00:00 AM", &fake_now));
+  clock()->SetNow(fake_now);
+
+  notifications::ScheduleParams expected_schedule_params;
+  expected_schedule_params.impression_mapping.emplace(
+      notifications::UserFeedback::kDismiss,
+      notifications::ImpressionResult::kNegative);
+  expected_schedule_params.impression_mapping.emplace(
+      notifications::UserFeedback::kNotHelpful,
+      notifications::ImpressionResult::kNegative);
+  notifications::TimePair deliver_window;
+  notifications::NextTimeWindow(clock(), config()->deliver_window_morning,
+                                config()->deliver_window_evening,
+                                &deliver_window);
+  expected_schedule_params.deliver_time_start =
+      base::make_optional(std::move(deliver_window.first));
+  expected_schedule_params.deliver_time_end =
+      base::make_optional(std::move(deliver_window.second));
+
+  notifications::NotificationData expected_notification_data;
+  expected_notification_data.title = kTestTitle;
+  expected_notification_data.message = kTestMessage;
+  expected_notification_data
+      .custom_data["extra_data_map_key_update_state_enum"] = "1";
+
+  notifications::NotificationParams expected_params(
+      notifications::SchedulerClientType::kChromeUpdate,
+      std::move(expected_notification_data),
+      std::move(expected_schedule_params));
+  expected_params.enable_ihnr_buttons = true;
+
+  EXPECT_CALL(
+      *scheduler(),
+      GetClientOverview(notifications::SchedulerClientType::kChromeUpdate, _))
+      .WillOnce(Invoke(
+          [](notifications::SchedulerClientType client,
+             base::OnceCallback<void(notifications::ClientOverview)> callback) {
+            notifications::ClientOverview client_overview;
+            client_overview.num_scheduled_notifications = 0;
+            std::move(callback).Run(std::move(client_overview));
+          }));
+  EXPECT_CALL(*scheduler(), Schedule(NotificationParamsEq(&expected_params)));
+
+  UpdateNotificationInfo data;
+  data.title = kTestTitle;
+  data.message = kTestMessage;
+  data.state = 1;
+  data.should_show_immediately = false;
+  service()->Schedule(std::move(data));
+}
+
+TEST_F(UpdateNotificationServiceImplTest, VerifyOnUserClick) {
+  std::vector<int> update_states = {1 /*UPDATE_AVAILABLE*/,
+                                    3 /*INLINE_UPDATE_AVAILABLE*/};
+  for (const auto& state : update_states) {
+    UpdateNotificationService::ExtraData extra = {
+        {"extra_data_map_key_update_state_enum", base::NumberToString(state)}};
+    EXPECT_CALL(*bridge(), LaunchChromeActivity(state));
+    service()->OnUserClick(extra);
+  }
+}
+
+TEST_F(UpdateNotificationServiceImplTest, VerifyGetThrottleConfig) {
+  for (int i = 0; i < 4; i++) {
+    EXPECT_CALL(
+        *scheduler(),
+        GetClientOverview(notifications::SchedulerClientType::kChromeUpdate, _))
+        .WillOnce(
+            Invoke([&i](notifications::SchedulerClientType client,
+                        base::OnceCallback<void(notifications::ClientOverview)>
+                            callback) {
+              notifications::ClientOverview client_overview;
+              client_overview.impression_detail.num_negative_events = i;
+              std::move(callback).Run(std::move(client_overview));
+            }));
+    service()->GetThrottleConfig(base::BindOnce(
+        [](int num_suppression,
+           std::unique_ptr<notifications::ThrottleConfig> throttle_config) {
+          EXPECT_EQ(throttle_config->suppression_duration.value(),
+                    base::TimeDelta::FromDays(
+                        std::min(21 * (1 + num_suppression), 90)));
+          EXPECT_EQ(throttle_config->negative_action_count_threshold.value(),
+                    2);
+        },
+        i));
+  }
+}
+
+TEST_F(UpdateNotificationServiceImplTest, BeforeShowNotification) {
+  std::vector<int> last_shown_timestamp_test_cases = {0, 20, 21, 22};
+  for (int test_case : last_shown_timestamp_test_cases) {
+    EXPECT_CALL(
+        *scheduler(),
+        GetClientOverview(notifications::SchedulerClientType::kChromeUpdate, _))
+        .WillOnce(
+            Invoke([&](notifications::SchedulerClientType client,
+                       base::OnceCallback<void(notifications::ClientOverview)>
+                           callback) {
+              base::Time fake_now;
+              EXPECT_TRUE(
+                  base::Time::FromString("05/18/20 01:00:00 AM", &fake_now));
+              clock()->SetNow(fake_now);
+              notifications::ClientOverview client_overview;
+              client_overview.impression_detail.last_shown_ts =
+                  this->clock()->Now() - base::TimeDelta::FromDays(test_case);
+              std::move(callback).Run(std::move(client_overview));
+            }));
+    service()->BeforeShowNotification(
+        std::make_unique<notifications::NotificationData>(),
+        base::BindOnce(
+            [](bool should_show,
+               std::unique_ptr<notifications::NotificationData> data) {
+              if (should_show)
+                EXPECT_NE(data, nullptr);
+              else
+                EXPECT_EQ(data, nullptr);
+            },
+            test_case > 21));
+  }
+}
+
 }  // namespace
 }  // namespace updates

chrome/browser/updates/update_notification_client.cc

@@ -11,20 +11,15 @@
 namespace updates {
 
 UpdateNotificationClient::UpdateNotificationClient(GetServiceCallback callback)
-    : get_service_callback_(std::move(callback)) {}
+    : service_getter_(std::move(callback)) {}
 
 UpdateNotificationClient::~UpdateNotificationClient() = default;
 
 void UpdateNotificationClient::BeforeShowNotification(
     std::unique_ptr<NotificationData> notification_data,
     NotificationDataCallback callback) {
-  auto* update_notification_service = get_service_callback_.Run();
-  DCHECK(update_notification_service);
-  if (!update_notification_service->IsReadyToDisplay()) {
-    std::move(callback).Run(nullptr);
-    return;
-  }
-  std::move(callback).Run(std::move(notification_data));
+  GetUpdateNotificationService()->BeforeShowNotification(
+      std::move(notification_data), std::move(callback));
 }
 
 void UpdateNotificationClient::OnSchedulerInitialized(
@@ -36,32 +31,19 @@ void UpdateNotificationClient::OnSchedulerInitialized(
 void UpdateNotificationClient::OnUserAction(const UserActionData& action_data) {
   DCHECK(action_data.client_type ==
          notifications::SchedulerClientType::kChromeUpdate);
-  auto* update_notification_service = get_service_callback_.Run();
-  DCHECK(update_notification_service);
-
-  switch (action_data.action_type) {
-    case notifications::UserActionType::kClick:
-      update_notification_service->OnUserClick(action_data.custom_data);
-      break;
-    case notifications::UserActionType::kButtonClick:
-      DCHECK(!action_data.button_click_info.has_value());
-      if (action_data.button_click_info.value().type ==
-          notifications::ActionButtonType::kUnhelpful) {
-        update_notification_service->OnUserClickButton(false);
-      }
-      break;
-    case notifications::UserActionType::kDismiss:
-      update_notification_service->OnUserDismiss();
-      break;
-    default:
-      NOTREACHED();
-      break;
+  if (action_data.action_type == notifications::UserActionType::kClick) {
+    GetUpdateNotificationService()->OnUserClick(action_data.custom_data);
   }
 }
 
 void UpdateNotificationClient::GetThrottleConfig(
     ThrottleConfigCallback callback) {
-  std::move(callback).Run(nullptr);
+  GetUpdateNotificationService()->GetThrottleConfig(std::move(callback));
+}
+
+UpdateNotificationService*
+UpdateNotificationClient::GetUpdateNotificationService() {
+  return service_getter_.Run();
 }
 
 }  // namespace updates

chrome/browser/updates/update_notification_client.h

@@ -40,9 +40,9 @@ class UpdateNotificationClient
   void OnUserAction(const UserActionData& action_data) override;
   void GetThrottleConfig(ThrottleConfigCallback callback) override;
 
-  GetServiceCallback get_service_callback_;
+  UpdateNotificationService* GetUpdateNotificationService();
 
-  DISALLOW_COPY_AND_ASSIGN(UpdateNotificationClient);
+  GetServiceCallback service_getter_;
 };
 
 }  // namespace updates

chrome/browser/updates/update_notification_config.cc

@@ -17,11 +17,6 @@ int GetFinchConfigInt(const std::string& name, int default_value) {
       chrome::android::kInlineUpdateFlow, name, default_value);
 }
 
-double GetFinchConfigDouble(const std::string& name, double default_value) {
-  return base::GetFieldTrialParamByFeatureAsDouble(
-      chrome::android::kInlineUpdateFlow, name, default_value);
-}
-
 bool GetFinchConfigBool(const std::string& name, bool default_value) {
   return base::GetFieldTrialParamByFeatureAsBool(
       chrome::android::kInlineUpdateFlow, name, default_value);
@@ -29,8 +24,11 @@ bool GetFinchConfigBool(const std::string& name, bool default_value) {
 
 }  // namespace
 
-// Default update notification schedule interval in days.
-constexpr int kDefaultUpdateNotificationInterval = 21;
+// Default update notification schedule initial interval in days.
+constexpr int kDefaultUpdateNotificationInitInterval = 21;
+
+// Default update notification schedule maximum interval in days.
+constexpr int kDefaultUpdateNotificationMaxInterval = 90;
 
 // Default start clock of deliver window in the morning.
 constexpr int kDefaultDeliverWindowMorningStart = 5;
@@ -44,12 +42,6 @@ constexpr int kDefaultDeliverWindowEveningStart = 18;
 // Default end clock of deliver window in the evening.
 constexpr int kDefaultDeliverWindowEveningEnd = 20;
 
-// Default scale coefficient of custom throttle linear function.
-constexpr double kDefaultThrottleIntervalScale = 1.0;
-
-// Default offset coefficient of custom throttle linear function.
-constexpr double kDefaultThrottleIntervalOffset = 0.0;
-
 // Default update notification state.
 constexpr bool kDefaultUpdateNotificationState = false;
 
@@ -65,9 +57,13 @@ UpdateNotificationConfig::CreateFromFinch() {
   config->is_enabled = GetFinchConfigBool(kUpdateNotificationStateParamName,
                                           kDefaultUpdateNotificationState);
 
-  config->default_interval = base::TimeDelta::FromDays(
-      GetFinchConfigInt(kUpdateNotificationIntervalParamName,
-                        kDefaultUpdateNotificationInterval));
+  config->init_interval = base::TimeDelta::FromDays(
+      GetFinchConfigInt(kUpdateNotificationInitIntervalParamName,
+                        kDefaultUpdateNotificationInitInterval));
+
+  config->max_interval = base::TimeDelta::FromDays(
+      GetFinchConfigInt(kUpdateNotificationMaxIntervalParamName,
+                        kDefaultUpdateNotificationMaxInterval));
 
   int morning_window_start =
       GetFinchConfigInt(kUpdateNotificationDeliverWindowMorningStartParamName,
@@ -89,22 +85,15 @@ UpdateNotificationConfig::CreateFromFinch() {
       base::TimeDelta::FromHours(evening_window_start),
       base::TimeDelta::FromHours(evening_window_end)};
 
-  config->throttle_interval_linear_co_scale =
-      GetFinchConfigDouble(kUpdateNotificationDeliverScaleCoefficientParamName,
-                           kDefaultThrottleIntervalScale);
-  config->throttle_interval_linear_co_offset = GetFinchConfigDouble(
-      kUpdateNotificationThrottleOffsetCoefficientParamName,
-      kDefaultThrottleIntervalOffset);
-
   return config;
 }
 
 UpdateNotificationConfig::UpdateNotificationConfig()
     : is_enabled(true),
-      default_interval(
-          base::TimeDelta::FromDays(kDefaultUpdateNotificationInterval)),
-      throttle_interval_linear_co_scale(kDefaultThrottleIntervalScale),
-      throttle_interval_linear_co_offset(kDefaultThrottleIntervalOffset),
+      init_interval(
+          base::TimeDelta::FromDays(kDefaultUpdateNotificationInitInterval)),
+      max_interval(
+          base::TimeDelta::FromDays(kDefaultUpdateNotificationMaxInterval)),
       deliver_window_morning(
           base::TimeDelta::FromHours(kDefaultDeliverWindowMorningStart),
           base::TimeDelta::FromHours(kDefaultDeliverWindowMorningEnd)),

chrome/browser/updates/update_notification_config.h

@@ -22,9 +22,13 @@ namespace updates {
 constexpr char kUpdateNotificationStateParamName[] =
     "update_notification_state";
 
-// Configure the update notification schedule interval in days.
-constexpr char kUpdateNotificationIntervalParamName[] =
-    "update_notification_interval_days";
+// Configure the initial schedule interval update notification in days.
+constexpr char kUpdateNotificationInitIntervalParamName[] =
+    "update_notification_init_interval_days";
+
+// Configure the maximum schedule interval update notification in days.
+constexpr char kUpdateNotificationMaxIntervalParamName[] =
+    "update_notification_max_interval_days";
 
 // Configure the start of deliver window in the morning.
 constexpr char kUpdateNotificationDeliverWindowMorningStartParamName[] =
@@ -42,14 +46,6 @@ constexpr char kUpdateNotificationDeliverWindowEveningStartParamName[] =
 constexpr char kUpdateNotificationDeliverWindowEveningEndParamName[] =
     "update_notification_deliver_window_evening_end";
 
-// Configure the custom throttle linear function scale coefficient.
-constexpr char kUpdateNotificationDeliverScaleCoefficientParamName[] =
-    "update_notification_throttle_co_scale";
-
-// Configure the custom throttle linear function offset coefficient.
-constexpr char kUpdateNotificationThrottleOffsetCoefficientParamName[] =
-    "update_notification_throttle_co_offset";
-
 struct UpdateNotificationConfig {
   // Create a default update notification config.
   static std::unique_ptr<UpdateNotificationConfig> Create();
@@ -64,32 +60,16 @@ struct UpdateNotificationConfig {
   bool is_enabled;
 
   // Default interval to schedule next update notification.
-  base::TimeDelta default_interval;
+  base::TimeDelta init_interval;
 
-  // Throttle logic could be conducted by adjusting schedule interval.
-  // A linear function X' = a * X + b would be applied.
-  // X represents the current interval(init value is default_interval),
-  // a represents scale coefficient,
-  // b represents offset coefficient,
-  // X' represents new schedule interval after applying throttling logic.
-  // For example, default_interval X is 3 weeks(21 days), a = 2, b = 2,
-  // then the schedule interval X' would be 8 weeks if it is throttled,
-  // and next time should be 18 weeks etc.
-
-  // Scale coefficient a in the linear function X' = a * X + b;
-  double throttle_interval_linear_co_scale;
-
-  // Offset coefficient b in the linear function X' = a * X + b;
-  double throttle_interval_linear_co_offset;
+  // Maximum interval to schedule next update notification.
+  base::TimeDelta max_interval;
 
   // Deliver window pair [start, end] in the morning.
   std::pair<base::TimeDelta, base::TimeDelta> deliver_window_morning;
 
   // Deliver window pair [start, end] in the evening.
   std::pair<base::TimeDelta, base::TimeDelta> deliver_window_evening;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(UpdateNotificationConfig);
 };
 
 }  // namespace updates

chrome/browser/updates/update_notification_config_unittest.cc

@@ -20,7 +20,8 @@ TEST(UpdateNotificationConfigTest, FinchConfigTest) {
 
   std::map<std::string, std::string> parameters = {
       {kUpdateNotificationStateParamName, "true"},
-      {kUpdateNotificationIntervalParamName, base::NumberToString(7)},
+      {kUpdateNotificationInitIntervalParamName, base::NumberToString(7)},
+      {kUpdateNotificationMaxIntervalParamName, base::NumberToString(123)},
       {kUpdateNotificationDeliverWindowMorningStartParamName,
        base::NumberToString(5)},
       {kUpdateNotificationDeliverWindowMorningEndParamName,
@@ -29,23 +30,18 @@ TEST(UpdateNotificationConfigTest, FinchConfigTest) {
        base::NumberToString(21)},
       {kUpdateNotificationDeliverWindowEveningEndParamName,
        base::NumberToString(23)},
-      {kUpdateNotificationThrottleOffsetCoefficientParamName,
-       base::NumberToString(2)},
-      {kUpdateNotificationDeliverScaleCoefficientParamName,
-       base::NumberToString(1)},
   };
   scoped_feature_list.InitAndEnableFeatureWithParameters(
       chrome::android::kInlineUpdateFlow, parameters);
   std::unique_ptr<UpdateNotificationConfig> config =
       UpdateNotificationConfig::CreateFromFinch();
   EXPECT_TRUE(config->is_enabled);
-  EXPECT_EQ(config->default_interval.InDays(), 7);
+  EXPECT_EQ(config->init_interval.InDays(), 7);
+  EXPECT_EQ(config->max_interval.InDays(), 123);
   EXPECT_EQ(config->deliver_window_morning.first.InHours(), 5);
   EXPECT_EQ(config->deliver_window_morning.second.InHours(), 6);
   EXPECT_EQ(config->deliver_window_evening.first.InHours(), 21);
   EXPECT_EQ(config->deliver_window_evening.second.InHours(), 23);
-  EXPECT_EQ(config->throttle_interval_linear_co_offset, 2);
-  EXPECT_EQ(config->throttle_interval_linear_co_scale, 1);
 }
 
 }  // namespace

chrome/browser/updates/update_notification_service.h

@@ -9,9 +9,15 @@
 #include <memory>
 #include <string>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "components/keyed_service/core/keyed_service.h"
 
+namespace notifications {
+struct ThrottleConfig;
+struct NotificationData;
+}  // namespace notifications
+
 namespace updates {
 
 struct UpdateNotificationInfo;
@@ -21,30 +27,30 @@ struct UpdateNotificationInfo;
 class UpdateNotificationService : public KeyedService {
  public:
   using ExtraData = std::map<std::string, std::string>;
+  using ThrottleConfigCallback =
+      base::OnceCallback<void(std::unique_ptr<notifications::ThrottleConfig>)>;
+  using NotificationDataCallback = base::OnceCallback<void(
+      std::unique_ptr<notifications::NotificationData>)>;
 
-  // Try yo schedule an update notification.
+  // Schedule an update notification.
   virtual void Schedule(UpdateNotificationInfo data) = 0;
 
-  // Validate the notification is ready to show.
-  virtual bool IsReadyToDisplay() const = 0;
-
-  // Called when the notification is dismissed by user.
-  virtual void OnUserDismiss() = 0;
-
   // Called when the notification is clicked by user. Passing |extra| for
   // processing custom data.
   virtual void OnUserClick(const ExtraData& extra) = 0;
 
-  // Called when the helpful/unhelpful buttons are clicked.
-  virtual void OnUserClickButton(bool is_positive_button) = 0;
+  // Replies customized throttle config.
+  virtual void GetThrottleConfig(ThrottleConfigCallback callback) = 0;
+
+  // Confirm whether the upcoming notification is ready to display.
+  virtual void BeforeShowNotification(
+      std::unique_ptr<notifications::NotificationData> notification_data,
+      NotificationDataCallback callback) = 0;
 
   ~UpdateNotificationService() override = default;
 
  protected:
   UpdateNotificationService() = default;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(UpdateNotificationService);
 };
 
 }  // namespace updates

chrome/browser/updates/update_notification_service_bridge.h

@@ -18,37 +18,19 @@ class UpdateNotificationServiceBridge {
   // Create the default UpdateNotificationServiceBridge.
   static std::unique_ptr<UpdateNotificationServiceBridge> Create();
 
-  // Updates and persists |timestamp| in Android SharedPreferences.
-  virtual void UpdateLastShownTimeStamp(base::Time timestamp) = 0;
-
-  // Returns persisted timestamp of last shown notification from Android
-  // SharedPreferences. Return nullopt if there is no data.
-  virtual base::Optional<base::Time> GetLastShownTimeStamp() = 0;
-
-  // Updates and persists |interval| in Android SharedPreferences.
-  virtual void UpdateThrottleInterval(base::TimeDelta interval) = 0;
-
-  // Returns persisted interval that might be throttled from Android
-  // SharedPreferences. Return nullopt if there is no data.
-  virtual base::Optional<base::TimeDelta> GetThrottleInterval() = 0;
-
-  // Updates and persists |count| in Android SharedPreferences.
-  virtual void UpdateNegativeActionCount(int count) = 0;
-
-  // Returns persisted count from Android SharedPreferences.
-  virtual int GetNegativeActionCount() = 0;
-
   // Launches Chrome activity after user clicked the notification. Launching
   // behavior may be different which depends on |state|.
   virtual void LaunchChromeActivity(int state) = 0;
 
   virtual ~UpdateNotificationServiceBridge() = default;
 
+  UpdateNotificationServiceBridge(
+      const UpdateNotificationServiceBridge& other) = delete;
+  UpdateNotificationServiceBridge& operator=(
+      const UpdateNotificationServiceBridge& other) = delete;
+
  protected:
   UpdateNotificationServiceBridge() = default;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(UpdateNotificationServiceBridge);
 };
 
 }  // namespace updates

chrome/browser/updates/update_notification_service_bridge_android.cc

@@ -21,9 +21,6 @@ using base::android::ScopedJavaLocalRef;
 
 namespace updates {
 
-//
-// Java -> C++
-//
 void JNI_UpdateNotificationServiceBridge_Schedule(
     JNIEnv* env,
     const JavaParamRef<jstring>& j_title,
@@ -41,53 +38,6 @@ void JNI_UpdateNotificationServiceBridge_Schedule(
   update_notification_service->Schedule(std::move(data));
 }
 
-//
-// C++ -> Java
-//
-void UpdateNotificationServiceBridgeAndroid::UpdateLastShownTimeStamp(
-    base::Time timestamp) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_UpdateNotificationServiceBridge_updateLastShownTimeStamp(
-      env, timestamp.ToJavaTime());
-}
-
-base::Optional<base::Time>
-UpdateNotificationServiceBridgeAndroid::GetLastShownTimeStamp() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  auto timestamp =
-      Java_UpdateNotificationServiceBridge_getLastShownTimeStamp(env);
-  return timestamp == 0
-             ? base::nullopt
-             : (base::make_optional(base::Time::FromJavaTime(timestamp)));
-}
-
-void UpdateNotificationServiceBridgeAndroid::UpdateThrottleInterval(
-    base::TimeDelta interval) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_UpdateNotificationServiceBridge_updateThrottleInterval(
-      env, interval.InMilliseconds());
-}
-
-base::Optional<base::TimeDelta>
-UpdateNotificationServiceBridgeAndroid::GetThrottleInterval() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  auto interval = Java_UpdateNotificationServiceBridge_getThrottleInterval(env);
-  return interval == 0 ? base::nullopt
-                       : (base::make_optional(
-                             base::TimeDelta::FromMilliseconds(interval)));
-}
-
-void UpdateNotificationServiceBridgeAndroid::UpdateNegativeActionCount(
-    int count) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_UpdateNotificationServiceBridge_updateNegativeActionCount(env, count);
-}
-
-int UpdateNotificationServiceBridgeAndroid::GetNegativeActionCount() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  return Java_UpdateNotificationServiceBridge_getNegativeActionCount(env);
-}
-
 void UpdateNotificationServiceBridgeAndroid::LaunchChromeActivity(int state) {
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_UpdateNotificationServiceBridge_launchChromeActivity(

chrome/browser/updates/update_notification_service_bridge_android.h

@@ -17,15 +17,7 @@ class UpdateNotificationServiceBridgeAndroid
 
  private:
   // UpdateNotificationServiceBridge implementation.
-  void UpdateLastShownTimeStamp(base::Time timestamp) override;
-  base::Optional<base::Time> GetLastShownTimeStamp() override;
-  void UpdateThrottleInterval(base::TimeDelta interval) override;
-  base::Optional<base::TimeDelta> GetThrottleInterval() override;
-  void UpdateNegativeActionCount(int count) override;
-  int GetNegativeActionCount() override;
   void LaunchChromeActivity(int state) override;
-
-  DISALLOW_COPY_AND_ASSIGN(UpdateNotificationServiceBridgeAndroid);
 };
 
 }  // namespace updates

chrome/browser/updates/update_notification_service_factory.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/memory/singleton.h"
+#include "base/time/default_clock.h"
 #include "chrome/browser/notifications/scheduler/notification_schedule_service_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile_key.h"
@@ -49,7 +50,8 @@ UpdateNotificationServiceFactory::BuildServiceInstanceFor(
   auto bridge =
       std::make_unique<updates::UpdateNotificationServiceBridgeAndroid>();
   return std::make_unique<updates::UpdateNotificationServiceImpl>(
-      schedule_service, std::move(config), std::move(bridge));
+      schedule_service, std::move(config), std::move(bridge),
+      base::DefaultClock::GetInstance());
 }
 
 SimpleFactoryKey* UpdateNotificationServiceFactory::GetKeyToUse(