Skip to content

Commit

Permalink
[SHv2] Add notification trigger logic for SB
Browse files Browse the repository at this point in the history 
This CL adds the special trigger logic for Safe Browsing. More
specifically, a notification should only be shown three times in the
entire lifetime unless the user change the Safe Browsing settings.
Additionally, the notification will only be shown after three days after
the user changed the setting.

Bug: 1443466
Change-Id: I32996d335dc0e8277f03f67fc466e9730d4d869d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974618
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Side YILMAZ <sideyilmaz@chromium.org>
Commit-Queue: Tom Van Goethem <tov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216249}
  • Loading branch information
@tomvangoethem
tomvangoethem authored and Chromium LUCI CQ committed Oct 27, 2023
1 parent 25a8dd8 commit d44d841
Show file tree
Hide file tree
Showing 8 changed files with 210 additions and 52 deletions.
87 changes: 67 additions & 20 deletions chrome/browser/ui/safety_hub/menu_notification.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <string>

#include "base/json/values_util.h"
#include "base/time/time.h"
#include "chrome/browser/ui/safety_hub/notification_permission_review_service.h"
#include "chrome/browser/ui/safety_hub/safe_browsing_result.h"
#include "chrome/browser/ui/safety_hub/safety_hub_constants.h"
Expand All @@ -18,41 +19,62 @@ SafetyHubMenuNotification::SafetyHubMenuNotification()
: is_currently_active_(false),
impression_count_(0),
first_impression_time_(absl::nullopt),
last_impression_time_(absl::nullopt) {}
last_impression_time_(absl::nullopt),
show_only_after_(absl::nullopt),
all_time_notification_count_(0) {}

SafetyHubMenuNotification::~SafetyHubMenuNotification() = default;

SafetyHubMenuNotification::SafetyHubMenuNotification(
const base::Value::Dict& dict,
safety_hub::SafetyHubModuleType type) {
is_currently_active_ =
dict.FindBool(kSafetyHubMenuNotificationActiveKey).value();
dict.FindBool(safety_hub::kSafetyHubMenuNotificationActiveKey).value();
impression_count_ =
dict.FindInt(kSafetyHubMenuNotificationImpressionCountKey).value();
dict.FindInt(safety_hub::kSafetyHubMenuNotificationImpressionCountKey)
.value_or(0);
all_time_notification_count_ =
dict.FindInt(safety_hub::kSafetyHubMenuNotificationAllTimeCountKey)
.value_or(0);
first_impression_time_ = base::ValueToTime(
dict.Find(kSafetyHubMenuNotificationFirstImpressionKey));
last_impression_time_ =
base::ValueToTime(dict.Find(kSafetyHubMenuNotificationLastImpressionKey));
if (dict.contains(kSafetyHubMenuNotificationResultKey)) {
dict.Find(safety_hub::kSafetyHubMenuNotificationFirstImpressionKey));
last_impression_time_ = base::ValueToTime(
dict.Find(safety_hub::kSafetyHubMenuNotificationLastImpressionKey));
show_only_after_ = base::ValueToTime(
dict.Find(safety_hub::kSafetyHubMenuNotificationShowAfterTimeKey));
if (dict.contains(safety_hub::kSafetyHubMenuNotificationResultKey)) {
result_ = GetResultFromDict(
*dict.FindDict(kSafetyHubMenuNotificationResultKey), type);
*dict.FindDict(safety_hub::kSafetyHubMenuNotificationResultKey), type);
}
}

base::Value::Dict SafetyHubMenuNotification::ToDictValue() const {
base::Value::Dict result;
result.Set(kSafetyHubMenuNotificationActiveKey, is_currently_active_);
result.Set(kSafetyHubMenuNotificationImpressionCountKey, impression_count_);
result.Set(safety_hub::kSafetyHubMenuNotificationActiveKey,
is_currently_active_);
if (impression_count_ != 0) {
result.Set(safety_hub::kSafetyHubMenuNotificationImpressionCountKey,
impression_count_);
}
if (first_impression_time_.has_value()) {
result.Set(kSafetyHubMenuNotificationFirstImpressionKey,
result.Set(safety_hub::kSafetyHubMenuNotificationFirstImpressionKey,
base::TimeToValue(first_impression_time_.value()));
}
if (last_impression_time_.has_value()) {
result.Set(kSafetyHubMenuNotificationLastImpressionKey,
result.Set(safety_hub::kSafetyHubMenuNotificationLastImpressionKey,
base::TimeToValue(last_impression_time_.value()));
}
if (all_time_notification_count_ != 0) {
result.Set(safety_hub::kSafetyHubMenuNotificationAllTimeCountKey,
all_time_notification_count_);
}
if (show_only_after_.has_value()) {
result.Set(safety_hub::kSafetyHubMenuNotificationShowAfterTimeKey,
base::TimeToValue(show_only_after_.value()));
}
if (result_ != nullptr) {
result.Set(kSafetyHubMenuNotificationResultKey, result_->ToDictValue());
result.Set(safety_hub::kSafetyHubMenuNotificationResultKey,
result_->ToDictValue());
}
return result;
}
Expand All @@ -71,18 +93,35 @@ void SafetyHubMenuNotification::Dismiss() {
is_currently_active_ = false;
impression_count_ = 0;
first_impression_time_ = absl::nullopt;
// TODO(crbug.com/1443466): Capture lifetime count, and determine whether it
// should still be shown. E.g. SafeBrowsing notification should only be shown
// 3 times in total.
++all_time_notification_count_;
}

bool SafetyHubMenuNotification::ShouldBeShown(base::TimeDelta interval) const {
// The maximum all time impressions for a notification are passed on each call
// instead of determined in the constructor to make these easier
// Finch-configurable as the constructor will only called once initially for
// each Safety Hub module, after which it will always be read from disk.
bool SafetyHubMenuNotification::ShouldBeShown(
base::TimeDelta interval,
int max_all_time_impressions) const {
// The type of notification has already been shown the maximum number of times
// in the total lifetime.
if (max_all_time_impressions > 0 &&
all_time_notification_count_ >= max_all_time_impressions) {
return false;
}

// There is no associated result, or the result does not meet the bar for menu
// notifications.
if (!result_ || !result_->IsTriggerForMenuNotification()) {
return false;
}

// Do not show notification if it is too soon.
if (show_only_after_.has_value() &&
base::Time::Now() < show_only_after_.value()) {
return false;
}

// Notifications that have never been shown can be shown as long as the result
// is a trigger.
if (!HasAnyNotificationBeenShown()) {
Expand Down Expand Up @@ -112,11 +151,11 @@ bool SafetyHubMenuNotification::IsShownEnough() const {
}

bool isShownEnoughDays =
(base::Time::Now() - first_impression_time_.value()) >
(base::Time::Now() - first_impression_time_.value()) >=
kSafetyHubMenuNotificationMinNotificationDuration;

bool isShownEnoughTimes =
impression_count_ > kSafetyHubMenuNotificationMinImpressionCount;
impression_count_ >= kSafetyHubMenuNotificationMinImpressionCount;

return isShownEnoughDays && isShownEnoughTimes;
}
Expand All @@ -127,7 +166,7 @@ bool SafetyHubMenuNotification::HasIntervalPassed(
if (!HasAnyNotificationBeenShown()) {
return true;
}
return base::Time::Now() - last_impression_time_.value() > interval;
return base::Time::Now() - last_impression_time_.value() >= interval;
}

bool SafetyHubMenuNotification::HasAnyNotificationBeenShown() const {
Expand Down Expand Up @@ -175,3 +214,11 @@ SafetyHubMenuNotification::GetResultFromDict(
return std::make_unique<SafetyHubSafeBrowsingResult>(dict);
}
}

void SafetyHubMenuNotification::SetOnlyShowAfter(base::Time time) {
show_only_after_ = time;
}

void SafetyHubMenuNotification::ResetAllTimeNotificationCount() {
all_time_notification_count_ = 0;
}
29 changes: 17 additions & 12 deletions chrome/browser/ui/safety_hub/menu_notification.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,6 @@
#include "chrome/browser/ui/safety_hub/safety_hub_service.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

constexpr char kSafetyHubMenuNotificationActiveKey[] = "isCurrentlyActive";
constexpr char kSafetyHubMenuNotificationImpressionCountKey[] =
"impressionCount";
constexpr char kSafetyHubMenuNotificationFirstImpressionKey[] =
"firstImpressionTime";
constexpr char kSafetyHubMenuNotificationLastImpressionKey[] =
"lastImpressionTime";
constexpr char kSafetyHubMenuNotificationResultKey[] = "result";

constexpr base::TimeDelta kSafetyHubMenuNotificationMinNotificationDuration =
base::Days(3);
constexpr int kSafetyHubMenuNotificationMinImpressionCount = 5;
Expand Down Expand Up @@ -52,9 +43,12 @@ class SafetyHubMenuNotification {
void Dismiss();

// Determines whether the notification should be shown given the maximum
// interval at which this type of notification should be shown. This does not
// take into account whether the notification is currently active.
bool ShouldBeShown(base::TimeDelta interval) const;
// interval at which this type of notification should be shown. If the
// provided maximum number of all-time impressions is not 0, this will also be
// taken into consideration. This method does not take into account whether
// the notification is currently active.
bool ShouldBeShown(base::TimeDelta interval,
int max_all_time_impressions = 0) const;

// Returns whether the notification is actively being shown.
bool IsCurrentlyActive() const;
Expand All @@ -64,6 +58,13 @@ class SafetyHubMenuNotification {
// are made. Otherwise, the menu notification will be considered as a new one.
void UpdateResult(std::unique_ptr<SafetyHubService::Result> result);

// Sets the time at which a notification can start to be shown.
void SetOnlyShowAfter(base::Time time);

// Resets the all-time counter for the number of notifications that have ever
// been shown.
void ResetAllTimeNotificationCount();

// Returns the notification string that will be shown in the three-dot menu.
std::u16string GetNotificationString() const;

Expand Down Expand Up @@ -106,6 +107,10 @@ class SafetyHubMenuNotification {
absl::optional<base::Time> last_impression_time_;
// The result for which the notification may be shown.
std::unique_ptr<SafetyHubService::Result> result_ = nullptr;
// Menu notifications should only be shown after this time.
absl::optional<base::Time> show_only_after_;
// The total number of time in total that a notification has been shown.
int all_time_notification_count_ = 0;
};

#endif // CHROME_BROWSER_UI_SAFETY_HUB_MENU_NOTIFICATION_H_
34 changes: 31 additions & 3 deletions chrome/browser/ui/safety_hub/menu_notification_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@

#include "base/functional/bind.h"
#include "base/functional/callback_forward.h"
#include "base/time/time.h"
#include "base/values.h"
#include "chrome/browser/ui/safety_hub/menu_notification.h"
#include "chrome/browser/ui/safety_hub/notification_permission_review_service.h"
#include "chrome/browser/ui/safety_hub/safe_browsing_result.h"
#include "chrome/browser/ui/safety_hub/safety_hub_constants.h"
#include "chrome/browser/ui/safety_hub/safety_hub_prefs.h"
#include "chrome/browser/ui/safety_hub/safety_hub_service.h"
#include "chrome/browser/ui/safety_hub/unused_site_permissions_service.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"

namespace {
SafetyHubModuleInfoElement::SafetyHubModuleInfoElement() = default;
Expand Down Expand Up @@ -64,9 +67,19 @@ SafetyHubMenuNotificationService::SafetyHubMenuNotificationService(
base::BindRepeating(&SafetyHubSafeBrowsingResult::GetResult,
base::Unretained(pref_service)),
stored_notifications);
// Listen for changes to the Safe Browsing pref to accommodate the trigger
// logic.
registrar_.Init(pref_service);
registrar_.Add(
prefs::kSafeBrowsingEnabled,
base::BindRepeating(
&SafetyHubMenuNotificationService::OnSafeBrowsingPrefUpdate,
base::Unretained(this)));
}

SafetyHubMenuNotificationService::~SafetyHubMenuNotificationService() = default;
SafetyHubMenuNotificationService::~SafetyHubMenuNotificationService() {
registrar_.RemoveAll();
}

absl::optional<MenuNotificationEntry>
SafetyHubMenuNotificationService::GetNotificationToShow() {
Expand All @@ -81,7 +94,10 @@ SafetyHubMenuNotificationService::GetNotificationToShow() {
module_info_map_[item.first].get();
SafetyHubMenuNotification* notification = info_element->notification.get();
notification->UpdateResult(std::move(result_map.value()[item.first]));
if (notification->ShouldBeShown(info_element->interval)) {
int max_all_time_impressions =
item.first == safety_hub::SafetyHubModuleType::SAFE_BROWSING ? 3 : 0;
if (notification->ShouldBeShown(info_element->interval,
max_all_time_impressions)) {
// Notifications are first sorted by priority, and then by being currently
// active.
if (info_element->priority > cur_highest_priority ||
Expand Down Expand Up @@ -160,7 +176,11 @@ SafetyHubMenuNotificationService::GetNotificationFromDict(
const base::Value::Dict* notification_dict =
dict.FindDict(pref_dict_key_map_.find(type)->second);
if (!notification_dict) {
return std::make_unique<SafetyHubMenuNotification>();
auto new_notification = std::make_unique<SafetyHubMenuNotification>();
if (type == safety_hub::SafetyHubModuleType::SAFE_BROWSING) {
new_notification->SetOnlyShowAfter(base::Time::Now() + base::Days(1));
}
return new_notification;
}
return std::make_unique<SafetyHubMenuNotification>(*notification_dict, type);
}
Expand All @@ -177,3 +197,11 @@ void SafetyHubMenuNotificationService::SetInfoElement(
priority, interval, result_getter,
GetNotificationFromDict(stored_notifications, type));
}

void SafetyHubMenuNotificationService::OnSafeBrowsingPrefUpdate() {
module_info_map_[safety_hub::SafetyHubModuleType::SAFE_BROWSING]
->notification->SetOnlyShowAfter(base::Time::Now() + base::Days(1));
module_info_map_[safety_hub::SafetyHubModuleType::SAFE_BROWSING]
->notification->ResetAllTimeNotificationCount();
SaveNotificationsToPrefs();
}
6 changes: 6 additions & 0 deletions chrome/browser/ui/safety_hub/menu_notification_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ class SafetyHubMenuNotificationService : public KeyedService {
result_getter,
const base::Value::Dict& stored_notifications);

// Called when the pref for Safe Browsing has been updated.
void OnSafeBrowsingPrefUpdate();

const std::map<safety_hub::SafetyHubModuleType, const char*>
pref_dict_key_map_ = {
{safety_hub::SafetyHubModuleType::UNUSED_SITE_PERMISSIONS,
Expand All @@ -127,6 +130,9 @@ class SafetyHubMenuNotificationService : public KeyedService {
std::map<safety_hub::SafetyHubModuleType,
std::unique_ptr<SafetyHubModuleInfoElement>>
module_info_map_;

// Registrar to record the pref changes to Safe Browsing.
PrefChangeRegistrar registrar_;
};

#endif // CHROME_BROWSER_UI_SAFETY_HUB_MENU_NOTIFICATION_SERVICE_H_

0 comments on commit d44d841

Please sign in to comment.