Skip to content

Commit

Permalink
[SHv2] Make notification permission review service a SafetyHubService
Browse files Browse the repository at this point in the history 
This CL converts the existing service for the notification permission
review into a SafetyHubService, which is run automatically in the
background.

Bug: 1443466
Change-Id: I06ec56904ecf734f3ab73eb8898e3cdeaa21e4f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4890269
Auto-Submit: Tom Van Goethem <tov@chromium.org>
Reviewed-by: Joe DeBlasio <jdeblasio@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Side YILMAZ <sideyilmaz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1206512}
  • Loading branch information
@tomvangoethem
tomvangoethem authored and Chromium LUCI CQ committed Oct 6, 2023
1 parent 82ee078 commit 1df59ca
Show file tree
Hide file tree
Showing 10 changed files with 445 additions and 243 deletions.
251 changes: 154 additions & 97 deletions chrome/browser/ui/safety_hub/notification_permission_review_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "base/containers/contains.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/values.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/ui/safety_hub/safety_hub_service.h"
Expand Down Expand Up @@ -91,34 +92,6 @@ GetNotificationCountMapPerPatternPair(
return result;
}

bool ShouldAddToNotificationPermissionReviewList(
site_engagement::SiteEngagementService* service,
GURL url,
int notification_count) {
// The notification permission should be added to the list if one of the
// criteria below holds:
// - Site engagement level is NONE OR MINIMAL and average daily notification
// count is more than 0.
// - Site engamment level is LOW and average daily notification count is
// more than 3. Otherwise, the notification permission should not be added
// to review list.
double score = service->GetScore(url);
int low_engagement_notification_limit =
features::kSafetyCheckNotificationPermissionsLowEnagementLimit.Get();
bool is_low_engagement =
!site_engagement::SiteEngagementService::IsEngagementAtLeast(
score, blink::mojom::EngagementLevel::MEDIUM) &&
notification_count > low_engagement_notification_limit;
int min_engagement_notification_limit =
features::kSafetyCheckNotificationPermissionsMinEnagementLimit.Get();
bool is_minimal_engagement =
!site_engagement::SiteEngagementService::IsEngagementAtLeast(
score, blink::mojom::EngagementLevel::LOW) &&
notification_count > min_engagement_notification_limit;

return is_minimal_engagement || is_low_engagement;
}

} // namespace

NotificationPermissions::NotificationPermissions(
Expand Down Expand Up @@ -157,6 +130,33 @@ void NotificationPermissionsReviewService::NotificationPermissionsResult::
notification_permissions_.emplace_back(origin, notification_count);
}

base::Value::List NotificationPermissionsReviewService::
NotificationPermissionsResult::GetSortedListValueForUI() {
base::Value::List result;

// Sort notification permissions by their priority for surfacing to the user.
auto notification_permission_ordering = [](const auto& left,
const auto& right) {
return left.second > right.second;
};
std::sort(notification_permissions_.begin(), notification_permissions_.end(),
notification_permission_ordering);

// Each entry is a dictionary with origin as key and notification count as
// value.
for (const auto& notification_permission : notification_permissions_) {
base::Value::Dict permission;
permission.Set(kSafetyHubOriginKey,
notification_permission.first.ToString());
std::string notification_info_string = l10n_util::GetPluralStringFUTF8(
IDS_SETTINGS_SAFETY_CHECK_REVIEW_NOTIFICATION_PERMISSIONS_COUNT_LABEL,
notification_permission.second);
permission.Set(kSafetyHubNotificationInfoString, notification_info_string);
result.Append(std::move(permission));
}
return result;
}

std::vector<std::pair<ContentSettingsPattern, int>>
NotificationPermissionsReviewService::NotificationPermissionsResult::
GetNotificationPermissions() const {
Expand Down Expand Up @@ -234,9 +234,22 @@ int NotificationPermissionsReviewService::NotificationPermissionsResult::
}

NotificationPermissionsReviewService::NotificationPermissionsReviewService(
HostContentSettingsMap* hcsm)
: hcsm_(hcsm) {
HostContentSettingsMap* hcsm,
site_engagement::SiteEngagementService* engagement_service)
: engagement_service_(engagement_service), hcsm_(hcsm) {
content_settings_observation_.Observe(hcsm);

if (!base::FeatureList::IsEnabled(features::kSafetyHub)) {
return;
}

// TODO(crbug.com/1443466): Because there is only an UI thread for this
// service, calling both |StartRepeatedUpdates()| and
// |InitializeLatestResult()| will result in the result being calculated twice
// when the service starts. When redesigning SafetyHubService, that should be
// avoided.
StartRepeatedUpdates();
InitializeLatestResult();
}

NotificationPermissionsReviewService::~NotificationPermissionsReviewService() =
Expand All @@ -246,6 +259,19 @@ void NotificationPermissionsReviewService::OnContentSettingChanged(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern,
ContentSettingsTypeSet content_type_set) {
if (content_type_set.Contains(
ContentSettingsType::NOTIFICATION_PERMISSION_REVIEW)) {
// In order to keep the latest result updated with the actual list that
// should be reviewed, the latest result should be updated here. This is
// triggered whenever an update is made to the ignore list. For other
// updates on notification permissions,
// |RemovePatternFromNotificationPermissionReviewBlocklist| will also update
// the ignore list, and thus will eventually result in the latest result
// being updated.
SetLatestResult(
UpdateOnUIThread(std::make_unique<NotificationPermissionsResult>()));
return;
}
if (!content_type_set.Contains(ContentSettingsType::NOTIFICATIONS)) {
return;
}
Expand All @@ -269,8 +295,37 @@ NotificationPermissionsReviewService::GetResultFromDictValue(
return std::make_unique<NotificationPermissionsResult>(dict);
}

std::vector<NotificationPermissions>
NotificationPermissionsReviewService::GetNotificationSiteListForReview() {
void NotificationPermissionsReviewService::
AddPatternToNotificationPermissionReviewBlocklist(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern) {
base::Value::Dict permission_dict;
permission_dict.Set(kExcludedKey, base::Value(true));

hcsm_->SetWebsiteSettingCustomScope(
primary_pattern, secondary_pattern,
ContentSettingsType::NOTIFICATION_PERMISSION_REVIEW,
base::Value(std::move(permission_dict)));
}

void NotificationPermissionsReviewService::
RemovePatternFromNotificationPermissionReviewBlocklist(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern) {
hcsm_->SetWebsiteSettingCustomScope(
primary_pattern, secondary_pattern,
ContentSettingsType::NOTIFICATION_PERMISSION_REVIEW, {});
}

std::unique_ptr<SafetyHubService::Result>
NotificationPermissionsReviewService::UpdateOnUIThread(
std::unique_ptr<SafetyHubService::Result> interim_result) {
auto result = std::make_unique<NotificationPermissionsResult>();
if (!base::FeatureList::IsEnabled(
features::kSafetyCheckNotificationPermissions) &&
!base::FeatureList::IsEnabled(features::kSafetyHub)) {
return result;
}
// Get blocklisted pattern pairs that should not be shown in the review list.
std::set<std::pair<ContentSettingsPattern, ContentSettingsPattern>>
ignored_patterns_set = GetIgnoredPatternPairs(hcsm_);
Expand All @@ -280,8 +335,8 @@ NotificationPermissionsReviewService::GetNotificationSiteListForReview() {
notification_count_map = GetNotificationCountMapPerPatternPair(hcsm_);

// Get the permissions with notification counts that needs to be reviewed.
// This list will be filtered based on notification count and site engagement
// score in the PopulateNotificationPermissionReviewData function.
// This list is filtered based on notification count and site engagement
// score.
std::vector<NotificationPermissions> notification_permissions_list;
for (auto& item :
hcsm_->GetSettingsForOneType(ContentSettingsType::NOTIFICATIONS)) {
Expand All @@ -304,80 +359,82 @@ NotificationPermissionsReviewService::GetNotificationSiteListForReview() {
}

int notification_count = notification_count_map[pair];
notification_permissions_list.emplace_back(
item.primary_pattern, item.secondary_pattern, notification_count);
}

return notification_permissions_list;
}
// Converting primary pattern to GURL should always be valid, since
// Notification Permission Review list only contains single origins.
GURL url = GURL(item.primary_pattern.ToString());
DCHECK(url.is_valid());
if (!ShouldAddToNotificationPermissionReviewList(url, notification_count)) {
continue;
}

void NotificationPermissionsReviewService::
AddPatternToNotificationPermissionReviewBlocklist(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern) {
base::Value::Dict permission_dict;
permission_dict.Set(kExcludedKey, base::Value(true));
result->AddNotificationPermission(item.primary_pattern, notification_count);
}

hcsm_->SetWebsiteSettingCustomScope(
primary_pattern, secondary_pattern,
ContentSettingsType::NOTIFICATION_PERMISSION_REVIEW,
base::Value(std::move(permission_dict)));
return result;
}

void NotificationPermissionsReviewService::
RemovePatternFromNotificationPermissionReviewBlocklist(
const ContentSettingsPattern& primary_pattern,
const ContentSettingsPattern& secondary_pattern) {
hcsm_->SetWebsiteSettingCustomScope(
primary_pattern, secondary_pattern,
ContentSettingsType::NOTIFICATION_PERMISSION_REVIEW, {});
base::Value::List NotificationPermissionsReviewService::
PopulateNotificationPermissionReviewData() {
// Return the cached result, which is kept in sync with the values on disk
// (i.e. HCSM), when available. Otherwise, re-calculate the result.
std::unique_ptr<SafetyHubService::Result> cached_result =
GetCachedResult().value_or(
UpdateOnUIThread(std::make_unique<NotificationPermissionsResult>()));
return (static_cast<NotificationPermissionsResult*>(cached_result.get()))
->GetSortedListValueForUI();
}

base::Value::List
NotificationPermissionsReviewService::PopulateNotificationPermissionReviewData(
Profile* profile) {
base::Value::List result;
if (!base::FeatureList::IsEnabled(
features::kSafetyCheckNotificationPermissions)) {
return result;
}
base::TimeDelta
NotificationPermissionsReviewService::GetRepeatedUpdateInterval() {
return base::Days(1);
}

auto notification_permissions = GetNotificationSiteListForReview();
base::OnceCallback<std::unique_ptr<SafetyHubService::Result>()>
NotificationPermissionsReviewService::GetBackgroundTask() {
return base::BindOnce(&UpdateOnBackgroundThread);
}

site_engagement::SiteEngagementService* engagement_service =
site_engagement::SiteEngagementService::Get(profile);
// static
std::unique_ptr<SafetyHubService::Result>
NotificationPermissionsReviewService::UpdateOnBackgroundThread() {
// Return an empty result.
return std::make_unique<NotificationPermissionsResult>();
}

// Sort notification permissions by their priority for surfacing to the user.
auto notification_permission_ordering =
[](const NotificationPermissions& left,
const NotificationPermissions& right) {
return left.notification_count > right.notification_count;
};
std::sort(notification_permissions.begin(), notification_permissions.end(),
notification_permission_ordering);
std::unique_ptr<SafetyHubService::Result>
NotificationPermissionsReviewService::InitializeLatestResultImpl() {
return UpdateOnUIThread(std::make_unique<NotificationPermissionsResult>());
}

for (const auto& notification_permission : notification_permissions) {
// Converting primary pattern to GURL should always be valid, since
// Notification Permission Review list only contains single origins. Those
// are filtered in
// NotificationPermissionsReviewService::GetNotificationSiteListForReview.
GURL url = GURL(notification_permission.primary_pattern.ToString());
DCHECK(url.is_valid());
if (!ShouldAddToNotificationPermissionReviewList(
engagement_service, url,
notification_permission.notification_count)) {
continue;
}
base::WeakPtr<SafetyHubService>
NotificationPermissionsReviewService::GetAsWeakRef() {
return weak_factory_.GetWeakPtr();
}

base::Value::Dict permission;
permission.Set(kSafetyHubOriginKey,
notification_permission.primary_pattern.ToString());
std::string notification_info_string = l10n_util::GetPluralStringFUTF8(
IDS_SETTINGS_SAFETY_CHECK_REVIEW_NOTIFICATION_PERMISSIONS_COUNT_LABEL,
notification_permission.notification_count);
permission.Set(kSafetyHubNotificationInfoString, notification_info_string);
result.Append(std::move(permission));
}
bool NotificationPermissionsReviewService::
ShouldAddToNotificationPermissionReviewList(GURL url,
int notification_count) {
// The notification permission should be added to the list if one of the
// criteria below holds:
// - Site engagement level is NONE OR MINIMAL and average daily notification
// count is more than 0.
// - Site engamment level is LOW and average daily notification count is
// more than 3. Otherwise, the notification permission should not be added
// to review list.
double score = engagement_service_->GetScore(url);
int low_engagement_notification_limit =
features::kSafetyCheckNotificationPermissionsLowEnagementLimit.Get();
bool is_low_engagement =
!site_engagement::SiteEngagementService::IsEngagementAtLeast(
score, blink::mojom::EngagementLevel::MEDIUM) &&
notification_count > low_engagement_notification_limit;
int min_engagement_notification_limit =
features::kSafetyCheckNotificationPermissionsMinEnagementLimit.Get();
bool is_minimal_engagement =
!site_engagement::SiteEngagementService::IsEngagementAtLeast(
score, blink::mojom::EngagementLevel::LOW) &&
notification_count > min_engagement_notification_limit;

return result;
return is_minimal_engagement || is_low_engagement;
}
Loading

0 comments on commit 1df59ca

Please sign in to comment.