Create RegisterForContinuousDisplay() & RegisterForSingleDisplay().
The PromosManager needs to expose two public methods enabling promos to
be registered for single or continuous display.
Although the active promos are stored in a list Pref, the order promo
entries are populated in the list is irrelevant, as active promos are
stored in an in-memory set during PromosManager evaluation.
A given promo can be registered both for single & continuous display.
The manager will handle this use-case gracefully.
Change-Id: I8e62d2995f776a8390ba4390912c92259a566d57
Bug: 1359359
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3864316
Reviewed-by: Robbie Gibson <rkgibson@google.com>
Commit-Queue: Benjamin Williams <bwwilliams@google.com>
Cr-Commit-Position: refs/heads/main@{#1042716}
diff --git a/ios/chrome/browser/promos_manager/promos_manager.h b/ios/chrome/browser/promos_manager/promos_manager.h
index c5789d8..cabedd2 100644
--- a/ios/chrome/browser/promos_manager/promos_manager.h
+++ b/ios/chrome/browser/promos_manager/promos_manager.h
@@ -28,6 +28,14 @@
// Returns the next promo for display, if any.
absl::optional<promos_manager::Promo> NextPromoForDisplay() const;
+ // Registers `promo` for continuous display, and persists registration status
+ // across app launches.
+ void RegisterPromoForContinuousDisplay(promos_manager::Promo promo);
+
+ // Registers `promo` for single (one-time) display, and persists registration
+ // status across app launches.
+ void RegisterPromoForSingleDisplay(promos_manager::Promo promo);
+
// Initialize the Promos Manager by restoring state from Prefs. Must be called
// after creation and before any other operation.
void Init();
@@ -182,6 +190,24 @@
ReturnsBlankActivePromosForBlankPrefs);
FRIEND_TEST_ALL_PREFIXES(PromosManagerTest,
ReturnsActivePromosAndSkipsMalformedData);
+ FRIEND_TEST_ALL_PREFIXES(PromosManagerTest,
+ RegistersPromoForContinuousDisplay);
+ FRIEND_TEST_ALL_PREFIXES(
+ PromosManagerTest,
+ RegistersPromoForContinuousDisplayForEmptyActivePromos);
+ FRIEND_TEST_ALL_PREFIXES(PromosManagerTest,
+ RegistersAlreadyRegisteredPromoForContinuousDisplay);
+ FRIEND_TEST_ALL_PREFIXES(
+ PromosManagerTest,
+ RegistersAlreadyRegisteredPromoForContinuousDisplayForEmptyActivePromos);
+ FRIEND_TEST_ALL_PREFIXES(PromosManagerTest, RegistersPromoForSingleDisplay);
+ FRIEND_TEST_ALL_PREFIXES(PromosManagerTest,
+ RegistersPromoForSingleDisplayForEmptyActivePromos);
+ FRIEND_TEST_ALL_PREFIXES(PromosManagerTest,
+ RegistersAlreadyRegisteredPromoForSingleDisplay);
+ FRIEND_TEST_ALL_PREFIXES(
+ PromosManagerTest,
+ RegistersAlreadyRegisteredPromoForSingleDisplayForEmptyActivePromos);
};
#endif // IOS_CHROME_BROWSER_PROMOS_MANAGER_PROMOS_MANAGER_H_
diff --git a/ios/chrome/browser/promos_manager/promos_manager.mm b/ios/chrome/browser/promos_manager/promos_manager.mm
index df7dcb7..9eb0b3e6 100644
--- a/ios/chrome/browser/promos_manager/promos_manager.mm
+++ b/ios/chrome/browser/promos_manager/promos_manager.mm
@@ -16,6 +16,7 @@
#import "base/time/time.h"
#import "base/values.h"
#import "components/prefs/pref_service.h"
+#import "components/prefs/scoped_user_pref_update.h"
#import "ios/chrome/browser/prefs/pref_names.h"
#import "ios/chrome/browser/promos_manager/constants.h"
#import "ios/chrome/browser/promos_manager/features.h"
@@ -34,6 +35,25 @@
return (base::Time::Now() - base::Time::UnixEpoch()).InDays();
}
+// Conditionally appends `promo` to the list pref `pref_path`. If `promo`
+// already exists in the list pref `pref_path`, does nothing. If `promo` doesn't
+// exist in the list pref `pref_path`, appends `promo` to the list.
+void ConditionallyAppendPromoToPrefList(promos_manager::Promo promo,
+ const std::string& pref_path,
+ PrefService* local_state) {
+ DCHECK(local_state);
+
+ ListPrefUpdate update(local_state, pref_path);
+ base::Value::List& active_promos = update->GetList();
+ std::string promo_name = promos_manager::NameForPromo(promo);
+
+ // Erase `promo_name` if it already exists in `active_promos`; avoid polluting
+ // `active_promos` with duplicate `promo_name` entries.
+ active_promos.EraseValue(base::Value(promo_name));
+
+ active_promos.Append(promo_name);
+}
+
} // namespace
#pragma mark - PromosManager
@@ -63,6 +83,17 @@
local_state_->GetValueList(prefs::kIosPromosManagerImpressions));
}
+void PromosManager::RegisterPromoForContinuousDisplay(
+ promos_manager::Promo promo) {
+ ConditionallyAppendPromoToPrefList(
+ promo, prefs::kIosPromosManagerActivePromos, local_state_);
+}
+
+void PromosManager::RegisterPromoForSingleDisplay(promos_manager::Promo promo) {
+ ConditionallyAppendPromoToPrefList(
+ promo, prefs::kIosPromosManagerSingleDisplayActivePromos, local_state_);
+}
+
absl::optional<promos_manager::Promo> PromosManager::NextPromoForDisplay()
const {
// Construct a superset including active (1) single-display and
diff --git a/ios/chrome/browser/promos_manager/promos_manager_unittest.mm b/ios/chrome/browser/promos_manager/promos_manager_unittest.mm
index 4d159fc..5869ecf 100644
--- a/ios/chrome/browser/promos_manager/promos_manager_unittest.mm
+++ b/ios/chrome/browser/promos_manager/promos_manager_unittest.mm
@@ -636,3 +636,260 @@
EXPECT_EQ(expected, promos_manager_->ActivePromos(promos));
}
+
+// Tests PromosManager::RegisterPromoForContinuousDisplay() correctly registers
+// a promo for continuous display by writing the promo's name to the pref
+// `kIosPromosManagerActivePromos`.
+TEST_F(PromosManagerTest, RegistersPromoForContinuousDisplay) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).empty());
+
+ // Initial active promos state.
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::AppStoreRating);
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).size(),
+ (size_t)2);
+
+ // Register new promo.
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::DefaultBrowser);
+
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).size(),
+ (size_t)3);
+ EXPECT_EQ(local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[0],
+ promos_manager::NameForPromo(
+ promos_manager::Promo::CredentialProviderExtension));
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[1],
+ promos_manager::NameForPromo(promos_manager::Promo::AppStoreRating));
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[2],
+ promos_manager::NameForPromo(promos_manager::Promo::DefaultBrowser));
+}
+
+// Tests PromosManager::RegisterPromoForContinuousDisplay() correctly registers
+// a promo—for the very first time—for continuous display by writing the
+// promo's name to the pref `kIosPromosManagerActivePromos`.
+TEST_F(PromosManagerTest,
+ RegistersPromoForContinuousDisplayForEmptyActivePromos) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).empty());
+
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::DefaultBrowser);
+
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).size(),
+ (size_t)1);
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[0],
+ promos_manager::NameForPromo(promos_manager::Promo::DefaultBrowser));
+}
+
+// Tests PromosManager::RegisterPromoForContinuousDisplay() correctly registers
+// an already-registered promo for continuous display by first erasing, and then
+// re-writing, the promo's name to the pref `kIosPromosManagerActivePromos`;
+// tests no duplicate entries are created.
+TEST_F(PromosManagerTest, RegistersAlreadyRegisteredPromoForContinuousDisplay) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).empty());
+
+ // Initial active promos state.
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::AppStoreRating);
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).size(),
+ (size_t)2);
+
+ // Register existing promo.
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).size(),
+ (size_t)2);
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[0],
+ promos_manager::NameForPromo(promos_manager::Promo::AppStoreRating));
+ EXPECT_EQ(local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[1],
+ promos_manager::NameForPromo(
+ promos_manager::Promo::CredentialProviderExtension));
+}
+
+// Tests PromosManager::RegisterPromoForContinuousDisplay() correctly registers
+// an already-registered promo for continuous display—for the very first time—by
+// first erasing, and then re-writing, the promo's name to the pref
+// `kIosPromosManagerActivePromos`; tests no duplicate entries are created.
+TEST_F(
+ PromosManagerTest,
+ RegistersAlreadyRegisteredPromoForContinuousDisplayForEmptyActivePromos) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).empty());
+
+ // Initial active promos state.
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+
+ // Register existing promo.
+ promos_manager_->RegisterPromoForContinuousDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+
+ EXPECT_EQ(
+ local_state_->GetValueList(prefs::kIosPromosManagerActivePromos).size(),
+ (size_t)1);
+ EXPECT_EQ(local_state_->GetValueList(prefs::kIosPromosManagerActivePromos)[0],
+ promos_manager::NameForPromo(
+ promos_manager::Promo::CredentialProviderExtension));
+}
+
+// Tests PromosManager::RegisterPromoForSingleDisplay() correctly registers
+// a promo for single display by writing the promo's name to the pref
+// `kIosPromosManagerSingleDisplayActivePromos`.
+TEST_F(PromosManagerTest, RegistersPromoForSingleDisplay) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .empty());
+
+ // Initial active promos state.
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::AppStoreRating);
+ EXPECT_EQ(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .size(),
+ (size_t)2);
+
+ // Register new promo.
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::DefaultBrowser);
+
+ EXPECT_EQ(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .size(),
+ (size_t)3);
+ EXPECT_EQ(local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[0],
+ promos_manager::NameForPromo(
+ promos_manager::Promo::CredentialProviderExtension));
+ EXPECT_EQ(
+ local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[1],
+ promos_manager::NameForPromo(promos_manager::Promo::AppStoreRating));
+ EXPECT_EQ(
+ local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[2],
+ promos_manager::NameForPromo(promos_manager::Promo::DefaultBrowser));
+}
+
+// Tests PromosManager::RegisterPromoForSingleDisplay() correctly registers
+// a promo for single display—for the very first time—by writing the promo's
+// name to the pref `kIosPromosManagerSingleDisplayActivePromos`.
+TEST_F(PromosManagerTest, RegistersPromoForSingleDisplayForEmptyActivePromos) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .empty());
+
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::DefaultBrowser);
+
+ EXPECT_EQ(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .size(),
+ (size_t)1);
+ EXPECT_EQ(
+ local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[0],
+ promos_manager::NameForPromo(promos_manager::Promo::DefaultBrowser));
+}
+
+// Tests PromosManager::RegisterPromoForSingleDisplay() correctly registers
+// an already-registered promo for single display by first erasing, and then
+// re-writing, the promo's name to the pref
+// `kIosPromosManagerSingleDisplayActivePromos`; tests no duplicate entries are
+// created.
+TEST_F(PromosManagerTest, RegistersAlreadyRegisteredPromoForSingleDisplay) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .empty());
+
+ // Initial active promos state.
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::AppStoreRating);
+ EXPECT_EQ(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .size(),
+ (size_t)2);
+
+ // Register existing promo.
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+
+ EXPECT_EQ(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .size(),
+ (size_t)2);
+ EXPECT_EQ(
+ local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[0],
+ promos_manager::NameForPromo(promos_manager::Promo::AppStoreRating));
+ EXPECT_EQ(local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[1],
+ promos_manager::NameForPromo(
+ promos_manager::Promo::CredentialProviderExtension));
+}
+
+// Tests PromosManager::RegisterPromoForSingleDisplay() correctly registers
+// an already-registered promo for single display—for the very first time—by
+// first erasing, and then re-writing, the promo's name to the pref
+// `kIosPromosManagerSingleDisplayActivePromos`; tests no duplicate entries are
+// created.
+TEST_F(PromosManagerTest,
+ RegistersAlreadyRegisteredPromoForSingleDisplayForEmptyActivePromos) {
+ CreatePromosManager();
+ EXPECT_TRUE(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .empty());
+
+ // Initial active promos state.
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+
+ // Register existing promo.
+ promos_manager_->RegisterPromoForSingleDisplay(
+ promos_manager::Promo::CredentialProviderExtension);
+
+ EXPECT_EQ(
+ local_state_
+ ->GetValueList(prefs::kIosPromosManagerSingleDisplayActivePromos)
+ .size(),
+ (size_t)1);
+ EXPECT_EQ(local_state_->GetValueList(
+ prefs::kIosPromosManagerSingleDisplayActivePromos)[0],
+ promos_manager::NameForPromo(
+ promos_manager::Promo::CredentialProviderExtension));
+}
