diff options
Diffstat (limited to 'chromium/components/feature_engagement')
70 files changed, 1503 insertions, 225 deletions
diff --git a/chromium/components/feature_engagement/COMMON_METADATA b/chromium/components/feature_engagement/COMMON_METADATA new file mode 100644 index 00000000000..454508fdf62 --- /dev/null +++ b/chromium/components/feature_engagement/COMMON_METADATA @@ -0,0 +1,3 @@ +monorail { + component: "Internals>FeatureEngagement" +} diff --git a/chromium/components/feature_engagement/DIR_METADATA b/chromium/components/feature_engagement/DIR_METADATA index 454508fdf62..02a82b37501 100644 --- a/chromium/components/feature_engagement/DIR_METADATA +++ b/chromium/components/feature_engagement/DIR_METADATA @@ -1,3 +1 @@ -monorail { - component: "Internals>FeatureEngagement" -} +mixins: "//components/feature_engagement/COMMON_METADATA" diff --git a/chromium/components/feature_engagement/OWNERS b/chromium/components/feature_engagement/OWNERS index 60a55260bd0..354f99dd084 100644 --- a/chromium/components/feature_engagement/OWNERS +++ b/chromium/components/feature_engagement/OWNERS @@ -2,10 +2,10 @@ dtrainor@chromium.org nyquist@chromium.org shaktisahu@chromium.org -per-file *event_constants.*=twellington@chromium.org -per-file *feature_configurations.cc=twellington@chromium.org -per-file *feature_constants.*=twellington@chromium.org -per-file *feature_list.*=twellington@chromium.org -per-file *EventConstants.java=twellington@chromium.org -per-file *FeatureConstants.java=twellington@chromium.org +per-file ...event_constants.*=twellington@chromium.org +per-file ...feature_configurations.cc=twellington@chromium.org +per-file ...feature_constants.*=twellington@chromium.org +per-file ...feature_list.*=twellington@chromium.org +per-file ...EventConstants.java=twellington@chromium.org +per-file ...FeatureConstants.java=twellington@chromium.org diff --git a/chromium/components/feature_engagement/internal/android/tracker_impl_android.cc b/chromium/components/feature_engagement/internal/android/tracker_impl_android.cc index 4ff9d193bb3..175cff08d98 100644 --- a/chromium/components/feature_engagement/internal/android/tracker_impl_android.cc +++ b/chromium/components/feature_engagement/internal/android/tracker_impl_android.cc @@ -100,6 +100,20 @@ bool TrackerImplAndroid::ShouldTriggerHelpUI( return tracker_->ShouldTriggerHelpUI(*features_[feature]); } +base::android::ScopedJavaLocalRef<jobject> +TrackerImplAndroid::ShouldTriggerHelpUIWithSnooze( + JNIEnv* env, + const base::android::JavaRef<jobject>& jobj, + const base::android::JavaParamRef<jstring>& jfeature) { + std::string feature = ConvertJavaStringToUTF8(env, jfeature); + DCHECK(features_.find(feature) != features_.end()); + + Tracker::TriggerDetails triggerDetails = + tracker_->ShouldTriggerHelpUIWithSnooze(*features_[feature]); + return Java_TrackerImpl_createTriggerDetails( + env, triggerDetails.ShouldShowIph(), triggerDetails.ShouldShowSnooze()); +} + bool TrackerImplAndroid::WouldTriggerHelpUI( JNIEnv* env, const base::android::JavaRef<jobject>& jobj, @@ -141,6 +155,19 @@ void TrackerImplAndroid::Dismissed( tracker_->Dismissed(*features_[feature]); } +void TrackerImplAndroid::DismissedWithSnooze( + JNIEnv* env, + const base::android::JavaRef<jobject>& jobj, + const base::android::JavaParamRef<jstring>& jfeature, + const jint snooze_action) { + std::string feature = ConvertJavaStringToUTF8(env, jfeature); + DCHECK(features_.find(feature) != features_.end()); + + tracker_->DismissedWithSnooze( + *features_[feature], + absl::make_optional(static_cast<Tracker::SnoozeAction>(snooze_action))); +} + base::android::ScopedJavaLocalRef<jobject> TrackerImplAndroid::AcquireDisplayLock( JNIEnv* env, @@ -196,4 +223,5 @@ DisplayLockHandleAndroid::GetJavaObject() { void DisplayLockHandleAndroid::Release(JNIEnv* env) { delete this; } + } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/android/tracker_impl_android.h b/chromium/components/feature_engagement/internal/android/tracker_impl_android.h index 41ad55ed3ff..b9004dee63f 100644 --- a/chromium/components/feature_engagement/internal/android/tracker_impl_android.h +++ b/chromium/components/feature_engagement/internal/android/tracker_impl_android.h @@ -34,6 +34,10 @@ class DisplayLockHandleAndroid { public: DisplayLockHandleAndroid( std::unique_ptr<DisplayLockHandle> display_lock_handle); + + DisplayLockHandleAndroid(const DisplayLockHandleAndroid&) = delete; + DisplayLockHandleAndroid& operator=(const DisplayLockHandleAndroid&) = delete; + ~DisplayLockHandleAndroid(); // Returns the Java-side of this JNI bridge. @@ -48,8 +52,6 @@ class DisplayLockHandleAndroid { // The Java-side of this JNI bridge. base::android::ScopedJavaGlobalRef<jobject> java_obj_; - - DISALLOW_COPY_AND_ASSIGN(DisplayLockHandleAndroid); }; // JNI bridge between TrackerImpl in Java and C++. See the @@ -62,6 +64,10 @@ class TrackerImplAndroid : public base::SupportsUserData::Data { const base::android::JavaRef<jobject>& jobj); TrackerImplAndroid(Tracker* tracker, FeatureVector features); + + TrackerImplAndroid(const TrackerImplAndroid&) = delete; + TrackerImplAndroid& operator=(const TrackerImplAndroid&) = delete; + ~TrackerImplAndroid() override; base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); @@ -76,6 +82,11 @@ class TrackerImplAndroid : public base::SupportsUserData::Data { JNIEnv* env, const base::android::JavaRef<jobject>& jobj, const base::android::JavaParamRef<jstring>& jfeature); + virtual base::android::ScopedJavaLocalRef<jobject> + ShouldTriggerHelpUIWithSnooze( + JNIEnv* env, + const base::android::JavaRef<jobject>& jobj, + const base::android::JavaParamRef<jstring>& jfeature); virtual bool WouldTriggerHelpUI( JNIEnv* env, const base::android::JavaRef<jobject>& jobj, @@ -92,6 +103,11 @@ class TrackerImplAndroid : public base::SupportsUserData::Data { virtual void Dismissed(JNIEnv* env, const base::android::JavaRef<jobject>& jobj, const base::android::JavaParamRef<jstring>& jfeature); + virtual void DismissedWithSnooze( + JNIEnv* env, + const base::android::JavaRef<jobject>& jobj, + const base::android::JavaParamRef<jstring>& jfeature, + const jint snooze_action); virtual base::android::ScopedJavaLocalRef<jobject> AcquireDisplayLock( JNIEnv* env, const base::android::JavaRef<jobject>& jobj); @@ -113,8 +129,6 @@ class TrackerImplAndroid : public base::SupportsUserData::Data { // The Java-side of this JNI bridge. base::android::ScopedJavaGlobalRef<jobject> java_obj_; - - DISALLOW_COPY_AND_ASSIGN(TrackerImplAndroid); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/availability_model.h b/chromium/components/feature_engagement/internal/availability_model.h index fdf4f03d1b7..0e3df2a62c1 100644 --- a/chromium/components/feature_engagement/internal/availability_model.h +++ b/chromium/components/feature_engagement/internal/availability_model.h @@ -26,6 +26,9 @@ class AvailabilityModel { // GetAvailability(...). using OnInitializedCallback = base::OnceCallback<void(bool success)>; + AvailabilityModel(const AvailabilityModel&) = delete; + AvailabilityModel& operator=(const AvailabilityModel&) = delete; + virtual ~AvailabilityModel() = default; // Starts initialization of the AvailabilityModel. @@ -44,9 +47,6 @@ class AvailabilityModel { protected: AvailabilityModel() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(AvailabilityModel); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/availability_model_impl.h b/chromium/components/feature_engagement/internal/availability_model_impl.h index 23721d77615..30f0898962c 100644 --- a/chromium/components/feature_engagement/internal/availability_model_impl.h +++ b/chromium/components/feature_engagement/internal/availability_model_impl.h @@ -31,6 +31,10 @@ class AvailabilityModelImpl : public AvailabilityModel { uint32_t current_day)>; explicit AvailabilityModelImpl(StoreLoadCallback load_callback); + + AvailabilityModelImpl(const AvailabilityModelImpl&) = delete; + AvailabilityModelImpl& operator=(const AvailabilityModelImpl&) = delete; + ~AvailabilityModelImpl() override; // AvailabilityModel implementation. @@ -60,8 +64,6 @@ class AvailabilityModelImpl : public AvailabilityModel { StoreLoadCallback store_load_callback_; base::WeakPtrFactory<AvailabilityModelImpl> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(AvailabilityModelImpl); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/availability_model_impl_unittest.cc b/chromium/components/feature_engagement/internal/availability_model_impl_unittest.cc index 143ce3e9e57..5a60599f787 100644 --- a/chromium/components/feature_engagement/internal/availability_model_impl_unittest.cc +++ b/chromium/components/feature_engagement/internal/availability_model_impl_unittest.cc @@ -34,6 +34,10 @@ class AvailabilityModelImplTest : public testing::Test { &AvailabilityModelImplTest::OnInitialized, base::Unretained(this)); } + AvailabilityModelImplTest(const AvailabilityModelImplTest&) = delete; + AvailabilityModelImplTest& operator=(const AvailabilityModelImplTest&) = + delete; + ~AvailabilityModelImplTest() override = default; // SetUpModel exists so that the filter can be changed for any test. @@ -64,9 +68,6 @@ class AvailabilityModelImplTest : public testing::Test { AvailabilityModel::OnInitializedCallback initialized_callback_; absl::optional<bool> success_; absl::optional<uint32_t> current_day_; - - private: - DISALLOW_COPY_AND_ASSIGN(AvailabilityModelImplTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/chrome_variations_configuration.cc b/chromium/components/feature_engagement/internal/chrome_variations_configuration.cc index ae2f6190169..e3109be15ef 100644 --- a/chromium/components/feature_engagement/internal/chrome_variations_configuration.cc +++ b/chromium/components/feature_engagement/internal/chrome_variations_configuration.cc @@ -45,6 +45,10 @@ const char kAvailabilityKey[] = "availability"; const char kTrackingOnlyKey[] = "tracking_only"; const char kIgnoredKeyPrefix[] = "x_"; +const char kSnoozeParams[] = "snooze_params"; +const char kSnoozeParamsMaxLimit[] = "max_limit"; +const char kSnoozeParamsInterval[] = "snooze_interval"; + const char kEventConfigDataNameKey[] = "name"; const char kEventConfigDataComparatorKey[] = "comparator"; const char kEventConfigDataWindowKey[] = "window"; @@ -265,6 +269,46 @@ bool ParseSessionRateImpact(const base::StringPiece& definition, return true; } +bool ParseSnoozeParams(const base::StringPiece& definition, + SnoozeParams* snooze_params) { + auto tokens = base::SplitStringPiece(definition, ",", base::TRIM_WHITESPACE, + base::SPLIT_WANT_ALL); + if (tokens.size() != 2) + return false; + + bool has_max_limit = false; + bool has_snooze_interval = false; + for (const auto& token : tokens) { + auto pair = base::SplitStringPiece(token, ":", base::TRIM_WHITESPACE, + base::SPLIT_WANT_ALL); + + if (pair.size() != 2) + return false; + + const base::StringPiece& key = pair[0]; + const base::StringPiece& value = pair[1]; + if (base::LowerCaseEqualsASCII(key, kSnoozeParamsMaxLimit)) { + uint32_t parsed_value; + if (!base::StringToUint(value, &parsed_value)) { + snooze_params->snooze_interval = 0u; + return false; + } + snooze_params->max_limit = parsed_value; + has_max_limit = true; + } else if (base::LowerCaseEqualsASCII(key, kSnoozeParamsInterval)) { + uint32_t parsed_value; + if (!base::StringToUint(value, &parsed_value)) { + snooze_params->max_limit = 0u; + return false; + } + snooze_params->snooze_interval = parsed_value; + has_snooze_interval = true; + } + } + + return has_max_limit && has_snooze_interval; +} + bool ParseTrackingOnly(const base::StringPiece& definition, bool* tracking_only) { // Since |tracking_only| is a primitive, ensure it set. @@ -402,6 +446,15 @@ void ChromeVariationsConfiguration::ParseFeatureConfig( continue; } config.availability = comparator; + } else if (key == kSnoozeParams) { + SnoozeParams snooze_params; + if (!ParseSnoozeParams(param_value, &snooze_params)) { + stats::RecordConfigParsingEvent( + stats::ConfigParsingEvent::FAILURE_SNOOZE_PARAMS_PARSE); + ++parse_errors; + continue; + } + config.snooze_params = snooze_params; } else if (base::StartsWith(key, kEventConfigKeyPrefix, base::CompareCase::INSENSITIVE_ASCII)) { EventConfig event_config; diff --git a/chromium/components/feature_engagement/internal/chrome_variations_configuration.h b/chromium/components/feature_engagement/internal/chrome_variations_configuration.h index f2e756e0494..f5aa07d7518 100644 --- a/chromium/components/feature_engagement/internal/chrome_variations_configuration.h +++ b/chromium/components/feature_engagement/internal/chrome_variations_configuration.h @@ -21,6 +21,11 @@ namespace feature_engagement { class ChromeVariationsConfiguration : public Configuration { public: ChromeVariationsConfiguration(); + + ChromeVariationsConfiguration(const ChromeVariationsConfiguration&) = delete; + ChromeVariationsConfiguration& operator=( + const ChromeVariationsConfiguration&) = delete; + ~ChromeVariationsConfiguration() override; // Configuration implementation. @@ -44,8 +49,6 @@ class ChromeVariationsConfiguration : public Configuration { // The current configurations. ConfigMap configs_; - - DISALLOW_COPY_AND_ASSIGN(ChromeVariationsConfiguration); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/chrome_variations_configuration_unittest.cc b/chromium/components/feature_engagement/internal/chrome_variations_configuration_unittest.cc index 94f8ea022f6..5f6e0875740 100644 --- a/chromium/components/feature_engagement/internal/chrome_variations_configuration_unittest.cc +++ b/chromium/components/feature_engagement/internal/chrome_variations_configuration_unittest.cc @@ -76,6 +76,11 @@ class ChromeVariationsConfigurationTest : public ::testing::Test { base::FeatureList::GetFieldTrial(kChromeTestFeatureQux)); } + ChromeVariationsConfigurationTest(const ChromeVariationsConfigurationTest&) = + delete; + ChromeVariationsConfigurationTest& operator=( + const ChromeVariationsConfigurationTest&) = delete; + void TearDown() override { // This is required to ensure each test can define its own params. base::FieldTrialParamAssociator::GetInstance()->ClearAllParamsForTesting(); @@ -113,8 +118,6 @@ class ChromeVariationsConfigurationTest : public ::testing::Test { private: base::test::ScopedFeatureList scoped_feature_list_; std::map<std::string, base::FieldTrial*> trials_; - - DISALLOW_COPY_AND_ASSIGN(ChromeVariationsConfigurationTest); }; } // namespace @@ -1162,4 +1165,84 @@ TEST_F(ChromeVariationsConfigurationTest, NonExistingConfigIsInvalid) { histogram_tester.ExpectTotalCount(kConfigParseEventName, 1); } +TEST_F(ChromeVariationsConfigurationTest, ParseValidSnoozeParams) { + std::map<std::string, std::string> foo_params; + foo_params["snooze_params"] = "max_limit:5, snooze_interval:3"; + foo_params["event_used"] = "name:eu;comparator:any;window:0;storage:360"; + foo_params["event_trigger"] = "name:et;comparator:any;window:0;storage:360"; + SetFeatureParams(kChromeTestFeatureFoo, foo_params); + + std::vector<const base::Feature*> features = {&kChromeTestFeatureFoo}; + configuration_.ParseFeatureConfigs(features); + + FeatureConfig foo = configuration_.GetFeatureConfig(kChromeTestFeatureFoo); + EXPECT_EQ(foo.snooze_params.max_limit, 5u); + EXPECT_EQ(foo.snooze_params.snooze_interval, 3u); + EXPECT_TRUE(foo.valid); +} + +TEST_F(ChromeVariationsConfigurationTest, UnorderedSnoozeParams) { + std::map<std::string, std::string> foo_params; + foo_params["snooze_params"] = "snooze_interval:3, max_limit:3"; + foo_params["event_used"] = "name:eu;comparator:any;window:0;storage:360"; + foo_params["event_trigger"] = "name:et;comparator:any;window:0;storage:360"; + SetFeatureParams(kChromeTestFeatureFoo, foo_params); + + std::vector<const base::Feature*> features = {&kChromeTestFeatureFoo}; + configuration_.ParseFeatureConfigs(features); + + FeatureConfig foo = configuration_.GetFeatureConfig(kChromeTestFeatureFoo); + EXPECT_EQ(foo.snooze_params.max_limit, 3u); + EXPECT_EQ(foo.snooze_params.snooze_interval, 3u); + EXPECT_TRUE(foo.valid); +} + +TEST_F(ChromeVariationsConfigurationTest, InvalidMaxLimitSnoozeParams) { + std::map<std::string, std::string> foo_params; + foo_params["snooze_params"] = "max_limit:e, snooze_interval:3"; + foo_params["event_used"] = "name:eu;comparator:any;window:0;storage:360"; + foo_params["event_trigger"] = "name:et;comparator:any;window:0;storage:360"; + SetFeatureParams(kChromeTestFeatureFoo, foo_params); + + std::vector<const base::Feature*> features = {&kChromeTestFeatureFoo}; + configuration_.ParseFeatureConfigs(features); + + FeatureConfig foo = configuration_.GetFeatureConfig(kChromeTestFeatureFoo); + EXPECT_EQ(foo.snooze_params.max_limit, 0u); + EXPECT_EQ(foo.snooze_params.snooze_interval, 0u); + EXPECT_FALSE(foo.valid); +} + +TEST_F(ChromeVariationsConfigurationTest, InvalidIntervalSnoozeParams) { + std::map<std::string, std::string> foo_params; + foo_params["snooze_params"] = "max_limit:3, snooze_interval:e"; + foo_params["event_used"] = "name:eu;comparator:any;window:0;storage:360"; + foo_params["event_trigger"] = "name:et;comparator:any;window:0;storage:360"; + SetFeatureParams(kChromeTestFeatureFoo, foo_params); + + std::vector<const base::Feature*> features = {&kChromeTestFeatureFoo}; + configuration_.ParseFeatureConfigs(features); + + FeatureConfig foo = configuration_.GetFeatureConfig(kChromeTestFeatureFoo); + EXPECT_EQ(foo.snooze_params.max_limit, 0u); + EXPECT_EQ(foo.snooze_params.snooze_interval, 0u); + EXPECT_FALSE(foo.valid); +} + +TEST_F(ChromeVariationsConfigurationTest, IncompleteSnoozeParams) { + std::map<std::string, std::string> foo_params; + foo_params["snooze_params"] = "max_limit:3"; + foo_params["event_used"] = "name:eu;comparator:any;window:0;storage:360"; + foo_params["event_trigger"] = "name:et;comparator:any;window:0;storage:360"; + SetFeatureParams(kChromeTestFeatureFoo, foo_params); + + std::vector<const base::Feature*> features = {&kChromeTestFeatureFoo}; + configuration_.ParseFeatureConfigs(features); + + FeatureConfig foo = configuration_.GetFeatureConfig(kChromeTestFeatureFoo); + EXPECT_EQ(foo.snooze_params.max_limit, 0u); + EXPECT_EQ(foo.snooze_params.snooze_interval, 0u); + EXPECT_FALSE(foo.valid); +} + } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/condition_validator.cc b/chromium/components/feature_engagement/internal/condition_validator.cc index 5b9bdd2785c..155cbdf79ed 100644 --- a/chromium/components/feature_engagement/internal/condition_validator.cc +++ b/chromium/components/feature_engagement/internal/condition_validator.cc @@ -19,7 +19,9 @@ ConditionValidator::Result::Result(bool initial_values) session_rate_ok(initial_values), availability_model_ready_ok(initial_values), availability_ok(initial_values), - display_lock_ok(initial_values) {} + display_lock_ok(initial_values), + snooze_expiration_ok(initial_values), + should_show_snooze(initial_values) {} ConditionValidator::Result::Result(const Result& other) = default; @@ -30,7 +32,7 @@ bool ConditionValidator::Result::NoErrors() const { return event_model_ready_ok && currently_showing_ok && feature_enabled_ok && config_ok && used_ok && trigger_ok && preconditions_ok && session_rate_ok && availability_model_ready_ok && availability_ok && - display_lock_ok; + display_lock_ok && snooze_expiration_ok; } std::ostream& operator<<(std::ostream& os, @@ -46,7 +48,9 @@ std::ostream& operator<<(std::ostream& os, << ", availability_model_ready_ok=" << result.availability_model_ready_ok << ", availability_ok=" << result.availability_ok - << ", display_lock_ok=" << result.display_lock_ok << " }"; + << ", display_lock_ok=" << result.display_lock_ok + << ", snooze_expiration_ok=" << result.snooze_expiration_ok + << ", should_show_snooze=" << result.should_show_snooze << " }"; } } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/condition_validator.h b/chromium/components/feature_engagement/internal/condition_validator.h index b2a096a25f2..8fb2f47373a 100644 --- a/chromium/components/feature_engagement/internal/condition_validator.h +++ b/chromium/components/feature_engagement/internal/condition_validator.h @@ -70,11 +70,21 @@ class ConditionValidator { // Whether there are currently held display locks. bool display_lock_ok; + // Whether the current snooze timer has expired. + bool snooze_expiration_ok; + + // Whether the snooze option should be shown. + // This value is excluded from the NoErrors() check. + bool should_show_snooze; + // Returns true if this result object has no errors, i.e. no values that // are false. bool NoErrors() const; }; + ConditionValidator(const ConditionValidator&) = delete; + ConditionValidator& operator=(const ConditionValidator&) = delete; + virtual ~ConditionValidator() = default; // Returns a Result object that describes whether each condition has been met. @@ -97,9 +107,6 @@ class ConditionValidator { protected: ConditionValidator() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(ConditionValidator); }; std::ostream& operator<<(std::ostream& os, diff --git a/chromium/components/feature_engagement/internal/condition_validator_unittest.cc b/chromium/components/feature_engagement/internal/condition_validator_unittest.cc index 7b3e392bd07..860d396d2da 100644 --- a/chromium/components/feature_engagement/internal/condition_validator_unittest.cc +++ b/chromium/components/feature_engagement/internal/condition_validator_unittest.cc @@ -89,4 +89,16 @@ TEST(ConditionValidatorResultTest, TestMultipleErrors) { EXPECT_FALSE(result.NoErrors()); } +TEST(ConditionValidatorResultTest, TestSnoozeExpirationFailed) { + ConditionValidator::Result result(true); + result.snooze_expiration_ok = false; + EXPECT_FALSE(result.NoErrors()); +} + +TEST(ConditionValidatorResultTest, TestShouldShowSnoozeFailed) { + ConditionValidator::Result result(true); + result.should_show_snooze = false; + EXPECT_TRUE(result.NoErrors()); +} + } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/display_lock_controller.h b/chromium/components/feature_engagement/internal/display_lock_controller.h index e1a9696ab5e..247d5f89c39 100644 --- a/chromium/components/feature_engagement/internal/display_lock_controller.h +++ b/chromium/components/feature_engagement/internal/display_lock_controller.h @@ -17,6 +17,9 @@ class DisplayLockHandle; // held. class DisplayLockController { public: + DisplayLockController(const DisplayLockController&) = delete; + DisplayLockController& operator=(const DisplayLockController&) = delete; + virtual ~DisplayLockController() = default; // Acquiring a display lock means that no in-product help can be displayed @@ -28,9 +31,6 @@ class DisplayLockController { protected: DisplayLockController() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(DisplayLockController); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/display_lock_controller_impl.h b/chromium/components/feature_engagement/internal/display_lock_controller_impl.h index 79fb7b76eb2..099b7b6d798 100644 --- a/chromium/components/feature_engagement/internal/display_lock_controller_impl.h +++ b/chromium/components/feature_engagement/internal/display_lock_controller_impl.h @@ -21,6 +21,11 @@ class DisplayLockHandle; class DisplayLockControllerImpl : public DisplayLockController { public: DisplayLockControllerImpl(); + + DisplayLockControllerImpl(const DisplayLockControllerImpl&) = delete; + DisplayLockControllerImpl& operator=(const DisplayLockControllerImpl&) = + delete; + ~DisplayLockControllerImpl() override; // DisplayLockController implementation. @@ -40,8 +45,6 @@ class DisplayLockControllerImpl : public DisplayLockController { THREAD_CHECKER(thread_checker_); base::WeakPtrFactory<DisplayLockControllerImpl> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(DisplayLockControllerImpl); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/editable_configuration.h b/chromium/components/feature_engagement/internal/editable_configuration.h index 47cd0853580..4d6f1ece033 100644 --- a/chromium/components/feature_engagement/internal/editable_configuration.h +++ b/chromium/components/feature_engagement/internal/editable_configuration.h @@ -20,6 +20,10 @@ namespace feature_engagement { class EditableConfiguration : public Configuration { public: EditableConfiguration(); + + EditableConfiguration(const EditableConfiguration&) = delete; + EditableConfiguration& operator=(const EditableConfiguration&) = delete; + ~EditableConfiguration() override; // Configuration implementation. @@ -38,8 +42,6 @@ class EditableConfiguration : public Configuration { private: // The current configurations. ConfigMap configs_; - - DISALLOW_COPY_AND_ASSIGN(EditableConfiguration); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/event_model.h b/chromium/components/feature_engagement/internal/event_model.h index 7895c9b0113..51d192b8e84 100644 --- a/chromium/components/feature_engagement/internal/event_model.h +++ b/chromium/components/feature_engagement/internal/event_model.h @@ -10,6 +10,7 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/time/time.h" namespace feature_engagement { class Event; @@ -21,6 +22,9 @@ class EventModel { // argument denotes whether the model was successfully initialized. using OnModelInitializationFinished = base::OnceCallback<void(bool success)>; + EventModel(const EventModel&) = delete; + EventModel& operator=(const EventModel&) = delete; + virtual ~EventModel() = default; // Initialize the model, including all underlying sub systems. When all @@ -38,9 +42,10 @@ class EventModel { virtual const Event* GetEvent(const std::string& event_name) const = 0; // Returns the number of times the event with |event_name| happened in the - // past |window_size| days from and including |current_day|. - // If |window_size| > |current_day|, all events since UNIX epoch will - // be counted, since |current_day| represents number of days since UNIX epoch. + // past |window_size| days from and including |current_day|, subtracted by + // the number of times the event has been snoozed. + // If |window_size| > |current_day|, all events since UNIX epoch will be + // counted, since |current_day| represents number of days since UNIX epoch. virtual uint32_t GetEventCount(const std::string& event_name, uint32_t current_day, uint32_t window_size) const = 0; @@ -52,11 +57,31 @@ class EventModel { virtual void IncrementEvent(const std::string& event_name, uint32_t current_day) = 0; + // Increments the snooze count for the day. + // Updates the last_snooze_time_us. + virtual void IncrementSnooze(const std::string& event_name, + uint32_t current_day, + base::Time current_time) = 0; + + // Sets the snooze_dismissed flag. + virtual void DismissSnooze(const std::string& event_name) = 0; + + // Returns the last snooze timestamp for the feature associated with + // |event_name|. + virtual base::Time GetLastSnoozeTimestamp( + const std::string& event_name) const = 0; + + // Returns the snooze count. Used for comparison against the max limit. + virtual uint32_t GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const = 0; + + // Returns whether the user has dismissed the snooze associated with + // |event_name|. + virtual bool IsSnoozeDismissed(const std::string& event_name) const = 0; + protected: EventModel() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(EventModel); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/event_model_impl.cc b/chromium/components/feature_engagement/internal/event_model_impl.cc index 8ca761b2aa3..775ef564b13 100644 --- a/chromium/components/feature_engagement/internal/event_model_impl.cc +++ b/chromium/components/feature_engagement/internal/event_model_impl.cc @@ -52,34 +52,13 @@ const Event* EventModelImpl::GetEvent(const std::string& event_name) const { uint32_t EventModelImpl::GetEventCount(const std::string& event_name, uint32_t current_day, uint32_t window_size) const { - const Event* event = GetEvent(event_name); - - // If the Event object is not found, or if the window is 0 days, there will - // never be any events. - if (event == nullptr || window_size == 0u) - return 0; - - DCHECK(window_size >= 0); - - // A window of N=0: Nothing should be counted. - // A window of N=1: |current_day| should be counted. - // A window of N=2+: |current_day| plus |N-1| more days should be counted. - uint32_t oldest_accepted_day = current_day - window_size + 1; - - // Cap |oldest_accepted_day| to UNIX epoch. - if (window_size > current_day) - oldest_accepted_day = 0u; - - // Calculate the number of events within the window. - uint32_t event_count = 0; - for (const auto& event_day : event->events()) { - if (event_day.day() < oldest_accepted_day) - continue; - - event_count += event_day.count(); - } - - return event_count; + uint32_t event_count = + GetEventCountOrSnooze(event_name, current_day, window_size, + /*is_snooze=*/false); + uint32_t snooze_count = + GetEventCountOrSnooze(event_name, current_day, window_size, + /*is_snooze=*/true); + return event_count - snooze_count; } void EventModelImpl::IncrementEvent(const std::string& event_name, @@ -112,6 +91,60 @@ void EventModelImpl::IncrementEvent(const std::string& event_name, store_->WriteEvent(event); } +void EventModelImpl::IncrementSnooze(const std::string& event_name, + uint32_t current_day, + base::Time current_time) { + DCHECK(ready_); + + DVLOG(2) << "Incrementing snooze for event " << event_name << " on " + << current_day << " @ " << current_time; + + Event& event = GetNonConstEvent(event_name); + for (int i = 0; i < event.events_size(); ++i) { + Event_Count* event_count = event.mutable_events(i); + DCHECK(event_count->has_day()); + DCHECK(event_count->has_count()); + if (event_count->day() != current_day) + continue; + event_count->set_snooze_count( + event_count->has_snooze_count() ? event_count->snooze_count() + 1 : 1u); + } + event.set_last_snooze_time_us( + current_time.ToDeltaSinceWindowsEpoch().InMicroseconds()); + store_->WriteEvent(event); +} + +void EventModelImpl::DismissSnooze(const std::string& event_name) { + DCHECK(ready_); + Event& event = GetNonConstEvent(event_name); + event.set_snooze_dismissed(true); + store_->WriteEvent(event); +} + +base::Time EventModelImpl::GetLastSnoozeTimestamp( + const std::string& event_name) const { + const Event* event = GetEvent(event_name); + + // If the Event object is not found, return. + if (!event) + return base::Time(); + + return base::Time::FromDeltaSinceWindowsEpoch( + base::Microseconds(event->last_snooze_time_us())); +} + +uint32_t EventModelImpl::GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const { + return GetEventCountOrSnooze(event_name, current_day, window, + /*is_snooze=*/true); +} + +bool EventModelImpl::IsSnoozeDismissed(const std::string& event_name) const { + const Event* event = GetEvent(event_name); + return event ? event->snooze_dismissed() : false; +} + void EventModelImpl::OnStoreLoaded(OnModelInitializationFinished callback, uint32_t current_day, bool success, @@ -134,11 +167,13 @@ void EventModelImpl::OnStoreLoaded(OnModelInitializationFinished callback, Event_Count* new_event_count = new_event.add_events(); new_event_count->set_day(event_count.day()); new_event_count->set_count(event_count.count()); + new_event_count->set_snooze_count(event_count.snooze_count()); } // Only keep Event object that have days with activity. if (new_event.events_size() > 0) { new_event.set_name(event.name()); + new_event.set_last_snooze_time_us(event.last_snooze_time_us()); events_[event.name()] = new_event; // If the number of events is not the same, overwrite DB entry. @@ -154,6 +189,39 @@ void EventModelImpl::OnStoreLoaded(OnModelInitializationFinished callback, std::move(callback).Run(true); } +int EventModelImpl::GetEventCountOrSnooze(const std::string& event_name, + int current_day, + int window, + bool is_snooze) const { + const Event* event = GetEvent(event_name); + + // If the Event object is not found, or if the window is 0 days, there will + // never be any events. + if (!event || window == 0u) + return 0; + + DCHECK(window >= 0); + + // A window of N=0: Nothing should be counted. + // A window of N=1: |current_day| should be counted. + // A window of N=2+: |current_day| plus |N-1| more days should be counted. + uint32_t oldest_accepted_day = current_day - window + 1; + + // Cap |oldest_accepted_day| to UNIX epoch. + if (window > current_day) + oldest_accepted_day = 0u; + + // Calculate the number of events within the window. + uint32_t count = 0; + for (const auto& event_day : event->events()) { + if (event_day.day() < oldest_accepted_day) + continue; + count += is_snooze ? event_day.snooze_count() : event_day.count(); + } + + return count; +} + Event& EventModelImpl::GetNonConstEvent(const std::string& event_name) { if (events_.find(event_name) == events_.end()) { // Event does not exist yet, so create it. diff --git a/chromium/components/feature_engagement/internal/event_model_impl.h b/chromium/components/feature_engagement/internal/event_model_impl.h index 447cfa3e772..def541d1501 100644 --- a/chromium/components/feature_engagement/internal/event_model_impl.h +++ b/chromium/components/feature_engagement/internal/event_model_impl.h @@ -24,6 +24,10 @@ class EventModelImpl : public EventModel { public: EventModelImpl(std::unique_ptr<EventStore> store, std::unique_ptr<EventStorageValidator> storage_validator); + + EventModelImpl(const EventModelImpl&) = delete; + EventModelImpl& operator=(const EventModelImpl&) = delete; + ~EventModelImpl() override; // EventModel implementation. @@ -36,6 +40,16 @@ class EventModelImpl : public EventModel { uint32_t window_size) const override; void IncrementEvent(const std::string& event_name, uint32_t current_day) override; + void IncrementSnooze(const std::string& event_name, + uint32_t current_day, + base::Time current_time) override; + void DismissSnooze(const std::string& event_name) override; + base::Time GetLastSnoozeTimestamp( + const std::string& event_name) const override; + uint32_t GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const override; + bool IsSnoozeDismissed(const std::string& event_name) const override; private: // Callback for loading the underlying store. @@ -44,6 +58,11 @@ class EventModelImpl : public EventModel { bool success, std::unique_ptr<std::vector<Event>> events); + int GetEventCountOrSnooze(const std::string& event_name, + int current_day, + int window, + bool is_snooze) const; + // Internal version for getting the non-const version of a stored Event. // Creates the event if it is not already stored. Event& GetNonConstEvent(const std::string& event_name); @@ -62,8 +81,6 @@ class EventModelImpl : public EventModel { bool ready_; base::WeakPtrFactory<EventModelImpl> weak_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(EventModelImpl); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/event_model_impl_unittest.cc b/chromium/components/feature_engagement/internal/event_model_impl_unittest.cc index c7009e57574..fdb07873b7b 100644 --- a/chromium/components/feature_engagement/internal/event_model_impl_unittest.cc +++ b/chromium/components/feature_engagement/internal/event_model_impl_unittest.cc @@ -71,6 +71,10 @@ class TestEventStorageValidator : public EventStorageValidator { public: TestEventStorageValidator() : should_store_(true) {} + TestEventStorageValidator(const TestEventStorageValidator&) = delete; + TestEventStorageValidator& operator=(const TestEventStorageValidator&) = + delete; + bool ShouldStore(const std::string& event_name) const override { return should_store_; } @@ -94,8 +98,6 @@ class TestEventStorageValidator : public EventStorageValidator { private: bool should_store_; std::map<std::string, uint32_t> max_keep_ages_; - - DISALLOW_COPY_AND_ASSIGN(TestEventStorageValidator); }; // Creates a TestInMemoryEventStore containing three hard coded events. @@ -452,6 +454,68 @@ TEST_F(EventModelImplTest, IncrementingExistingMultiDayEventNewDay) { test::VerifyEventsEqual(bar_event2, store_->GetLastWrittenEvent()); } +TEST_F(EventModelImplTest, IncrementingSnoozeEvent) { + model_->Initialize( + base::BindOnce(&EventModelImplTest::OnModelInitializationFinished, + base::Unretained(this)), + 1000u); + task_runner_->RunUntilIdle(); + EXPECT_TRUE(model_->IsReady()); + + // Verify that incrementing snooze across multiple days update the snooze + // count and the last_snooze_time_us field. + base::Time snooze_time = + base::Time::FromDeltaSinceWindowsEpoch(base::Microseconds(5)); + model_->IncrementEvent("snooze", 1u); + model_->IncrementSnooze("snooze", 1u, base::Time()); + model_->IncrementEvent("snooze", 2u); + model_->IncrementEvent("snooze", 3u); + model_->IncrementSnooze("snooze", 3u, base::Time()); + model_->IncrementEvent("snooze", 3u); + model_->IncrementSnooze("snooze", 3u, snooze_time); + model_->IncrementEvent("snooze", 5u); + const Event* bar_event = model_->GetEvent("snooze"); + EXPECT_EQ(snooze_time.ToDeltaSinceWindowsEpoch().InMicroseconds(), + bar_event->last_snooze_time_us()); + EXPECT_EQ(0u, model_->GetSnoozeCount("snooze", 1u, 5u)); + EXPECT_EQ(2u, model_->GetSnoozeCount("snooze", 3u, 5u)); + EXPECT_EQ(3u, model_->GetSnoozeCount("snooze", 5u, 5u)); + EXPECT_EQ(2u, model_->GetEventCount("snooze", 5u, 5u)); +} + +TEST_F(EventModelImplTest, DismissSnoozeEvent) { + model_->Initialize( + base::BindOnce(&EventModelImplTest::OnModelInitializationFinished, + base::Unretained(this)), + 1000u); + task_runner_->RunUntilIdle(); + EXPECT_TRUE(model_->IsReady()); + + // Verify that dismissing a snooze event update the snooze_dismissed flag. + model_->DismissSnooze("bar"); + EXPECT_EQ(true, model_->IsSnoozeDismissed("bar")); +} + +TEST_F(EventModelImplTest, GetLastSnoozeTimestamp) { + model_->Initialize( + base::BindOnce(&EventModelImplTest::OnModelInitializationFinished, + base::Unretained(this)), + 1000u); + task_runner_->RunUntilIdle(); + EXPECT_TRUE(model_->IsReady()); + + // Verify the correct last_snooze_time_us is returned. + base::Time snooze_time1 = + base::Time::FromDeltaSinceWindowsEpoch(base::Microseconds(4)); + base::Time snooze_time2 = + base::Time::FromDeltaSinceWindowsEpoch(base::Microseconds(5)); + + model_->IncrementSnooze("bar", 10u, snooze_time1); + EXPECT_EQ(snooze_time1, model_->GetLastSnoozeTimestamp("bar")); + model_->IncrementSnooze("bar", 10u, snooze_time2); + EXPECT_EQ(snooze_time2, model_->GetLastSnoozeTimestamp("bar")); +} + TEST_F(EventModelImplTest, GetEventCount) { model_->Initialize( base::BindOnce(&EventModelImplTest::OnModelInitializationFinished, diff --git a/chromium/components/feature_engagement/internal/event_storage_validator.h b/chromium/components/feature_engagement/internal/event_storage_validator.h index db41c843b74..9643a84b98c 100644 --- a/chromium/components/feature_engagement/internal/event_storage_validator.h +++ b/chromium/components/feature_engagement/internal/event_storage_validator.h @@ -15,6 +15,9 @@ namespace feature_engagement { // event, and checks if all conditions are met for storing it. class EventStorageValidator { public: + EventStorageValidator(const EventStorageValidator&) = delete; + EventStorageValidator& operator=(const EventStorageValidator&) = delete; + virtual ~EventStorageValidator() = default; // Returns true iff new events of this type should be stored. @@ -30,9 +33,6 @@ class EventStorageValidator { protected: EventStorageValidator() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(EventStorageValidator); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/event_store.h b/chromium/components/feature_engagement/internal/event_store.h index e77a5e5fac1..df49edeef18 100644 --- a/chromium/components/feature_engagement/internal/event_store.h +++ b/chromium/components/feature_engagement/internal/event_store.h @@ -20,6 +20,9 @@ class EventStore { base::OnceCallback<void(bool success, std::unique_ptr<std::vector<Event>>)>; + EventStore(const EventStore&) = delete; + EventStore& operator=(const EventStore&) = delete; + virtual ~EventStore() = default; // Loads the database from storage and asynchronously posts the result back @@ -39,9 +42,6 @@ class EventStore { protected: EventStore() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(EventStore); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/feature_config_condition_validator.cc b/chromium/components/feature_engagement/internal/feature_config_condition_validator.cc index 8cd8d93caac..803f1451abc 100644 --- a/chromium/components/feature_engagement/internal/feature_config_condition_validator.cc +++ b/chromium/components/feature_engagement/internal/feature_config_condition_validator.cc @@ -58,6 +58,16 @@ ConditionValidator::Result FeatureConfigConditionValidator::MeetsConditions( result.display_lock_ok = !display_lock_controller.IsDisplayLocked(); + result.snooze_expiration_ok = + !event_model.IsSnoozeDismissed(config.trigger.name) && + (event_model.GetLastSnoozeTimestamp(config.trigger.name) < + base::Time::Now() - base::Days(config.snooze_params.snooze_interval)); + + result.should_show_snooze = + result.snooze_expiration_ok && + event_model.GetSnoozeCount(config.trigger.name, config.trigger.window, + current_day) < config.snooze_params.max_limit; + return result; } diff --git a/chromium/components/feature_engagement/internal/feature_config_condition_validator.h b/chromium/components/feature_engagement/internal/feature_config_condition_validator.h index 12da9371c00..1fc8dbef79b 100644 --- a/chromium/components/feature_engagement/internal/feature_config_condition_validator.h +++ b/chromium/components/feature_engagement/internal/feature_config_condition_validator.h @@ -21,6 +21,12 @@ class EventModel; class FeatureConfigConditionValidator : public ConditionValidator { public: FeatureConfigConditionValidator(); + + FeatureConfigConditionValidator(const FeatureConfigConditionValidator&) = + delete; + FeatureConfigConditionValidator& operator=( + const FeatureConfigConditionValidator&) = delete; + ~FeatureConfigConditionValidator() override; // ConditionValidator implementation. @@ -57,8 +63,6 @@ class FeatureConfigConditionValidator : public ConditionValidator { // By default, all features impact each other, but some features override this // through the use of |session_rate_impact|. std::map<std::string, uint32_t> times_shown_for_feature_; - - DISALLOW_COPY_AND_ASSIGN(FeatureConfigConditionValidator); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/feature_config_condition_validator_unittest.cc b/chromium/components/feature_engagement/internal/feature_config_condition_validator_unittest.cc index 85e41a8247c..84d5333c94b 100644 --- a/chromium/components/feature_engagement/internal/feature_config_condition_validator_unittest.cc +++ b/chromium/components/feature_engagement/internal/feature_config_condition_validator_unittest.cc @@ -59,7 +59,7 @@ SessionRateImpact CreateSessionRateImpactTypeExplicit( class TestEventModel : public EventModel { public: - TestEventModel() : ready_(true) {} + TestEventModel() = default; void Initialize(OnModelInitializationFinished callback, uint32_t current_day) override {} @@ -106,14 +106,65 @@ class TestEventModel : public EventModel { void IncrementEvent(const std::string& event_name, uint32_t day) override {} + void IncrementSnooze(const std::string& event_name, + uint32_t day, + base::Time time) override { + last_snooze_time_us_ = time; + } + + void DismissSnooze(const std::string& event_name) override { + snooze_dismissed_ = true; + } + + base::Time GetLastSnoozeTimestamp( + const std::string& event_name) const override { + return last_snooze_time_us_; + } + + uint32_t GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const override { + const Event* event = GetEvent(event_name); + if (!event || window == 0u) + return 0; + + DCHECK(window >= 0); + + uint32_t oldest_accepted_day = current_day - window + 1; + if (window > current_day) + oldest_accepted_day = 0u; + + // Calculate the number of snooze within the window. + uint32_t count = 0; + for (const auto& event_day : event->events()) { + if (event_day.day() < oldest_accepted_day || + event_day.day() > current_day) + continue; + + count += event_day.snooze_count(); + } + + return count; + } + + bool IsSnoozeDismissed(const std::string& event_name) const override { + return snooze_dismissed_; + } + private: std::map<std::string, Event> events_; - bool ready_; + base::Time last_snooze_time_us_; + bool ready_ = true; + bool snooze_dismissed_ = false; }; class TestAvailabilityModel : public AvailabilityModel { public: TestAvailabilityModel() : ready_(true) {} + + TestAvailabilityModel(const TestAvailabilityModel&) = delete; + TestAvailabilityModel& operator=(const TestAvailabilityModel&) = delete; + ~TestAvailabilityModel() override = default; void Initialize(AvailabilityModel::OnInitializedCallback callback, @@ -141,13 +192,16 @@ class TestAvailabilityModel : public AvailabilityModel { bool ready_; std::map<std::string, absl::optional<uint32_t>> availabilities_; - - DISALLOW_COPY_AND_ASSIGN(TestAvailabilityModel); }; class TestDisplayLockController : public DisplayLockController { public: TestDisplayLockController() = default; + + TestDisplayLockController(const TestDisplayLockController&) = delete; + TestDisplayLockController& operator=(const TestDisplayLockController&) = + delete; + ~TestDisplayLockController() override = default; std::unique_ptr<DisplayLockHandle> AcquireDisplayLock() override { @@ -163,14 +217,17 @@ class TestDisplayLockController : public DisplayLockController { private: // The next result to return from IsDisplayLocked(). bool next_display_locked_result_ = false; - - DISALLOW_COPY_AND_ASSIGN(TestDisplayLockController); }; class FeatureConfigConditionValidatorTest : public ::testing::Test { public: FeatureConfigConditionValidatorTest() = default; + FeatureConfigConditionValidatorTest( + const FeatureConfigConditionValidatorTest&) = delete; + FeatureConfigConditionValidatorTest& operator=( + const FeatureConfigConditionValidatorTest&) = delete; + protected: ConditionValidator::Result GetResultForDayAndEventWindow( Comparator comparator, @@ -209,9 +266,6 @@ class FeatureConfigConditionValidatorTest : public ::testing::Test { TestDisplayLockController display_lock_controller_; FeatureConfigConditionValidator validator_; uint32_t current_day_; - - private: - DISALLOW_COPY_AND_ASSIGN(FeatureConfigConditionValidatorTest); }; } // namespace @@ -634,6 +688,94 @@ TEST_F(FeatureConfigConditionValidatorTest, Availability) { EXPECT_TRUE(GetResultForDay(config, 14u).NoErrors()); } +TEST_F(FeatureConfigConditionValidatorTest, SnoozeExpiration) { + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitWithFeatures({kFeatureConfigTestFeatureFoo}, {}); + FeatureConfig config = GetAcceptingFeatureConfig(); + base::Time baseline = base::Time::Now(); + + // Set up snooze params. + SnoozeParams snooze_params; + snooze_params.max_limit = 5; + snooze_params.snooze_interval = 3; + config.snooze_params = snooze_params; + config.trigger.window = 5; + + ConditionValidator::Result result = GetResultForDayZero(config); + EXPECT_TRUE(result.NoErrors()); + EXPECT_TRUE(result.snooze_expiration_ok); + EXPECT_TRUE(result.should_show_snooze); + + // Adding snooze count for |event|. + Event event; + event.set_name(config.trigger.name); + test::SetSnoozeCountForDay(&event, 1u, 1u); + test::SetSnoozeCountForDay(&event, 3u, 2u); + test::SetSnoozeCountForDay(&event, 5u, 2u); + event_model_.SetEvent(event); + + // Updating last snooze timestamp. + event_model_.IncrementSnooze(config.trigger.name, 1u, + baseline - base::Days(4)); + + // Verify that snooze conditions are met at day 3. + result = GetResultForDay(config, 3u); + EXPECT_TRUE(result.NoErrors()); + EXPECT_TRUE(result.snooze_expiration_ok); + EXPECT_TRUE(result.should_show_snooze); + + // When last snooze timestamp is too recent. + event_model_.IncrementSnooze(config.trigger.name, 1u, + baseline - base::Days(2)); + result = GetResultForDay(config, 3u); + EXPECT_FALSE(result.NoErrors()); + EXPECT_FALSE(result.snooze_expiration_ok); + EXPECT_FALSE(result.should_show_snooze); + + // Reset the last snooze timestamp. + event_model_.IncrementSnooze(config.trigger.name, 1u, + baseline - base::Days(4)); + result = GetResultForDay(config, 3u); + EXPECT_TRUE(result.NoErrors()); + EXPECT_TRUE(result.snooze_expiration_ok); + EXPECT_TRUE(result.should_show_snooze); + + // When snooze is dismissed. + event_model_.DismissSnooze(config.trigger.name); + result = GetResultForDay(config, 3u); + EXPECT_FALSE(result.NoErrors()); + EXPECT_FALSE(result.snooze_expiration_ok); + EXPECT_FALSE(result.should_show_snooze); +} + +TEST_F(FeatureConfigConditionValidatorTest, ShouldShowSnooze) { + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitWithFeatures({kFeatureConfigTestFeatureFoo}, {}); + FeatureConfig config = GetAcceptingFeatureConfig(); + + // Set up snooze params. + SnoozeParams snooze_params; + snooze_params.max_limit = 5; + snooze_params.snooze_interval = 3; + config.snooze_params = snooze_params; + config.trigger.window = 5; + + ConditionValidator::Result result = GetResultForDayZero(config); + EXPECT_TRUE(result.NoErrors()); + EXPECT_TRUE(result.should_show_snooze); + + // Adding snooze count for |event|. + Event event; + event.set_name(config.trigger.name); + test::SetSnoozeCountForDay(&event, 1u, 10u); + event_model_.SetEvent(event); + + // When snooze count exceeds the maximum limit. + result = GetResultForDay(config, 5u); + EXPECT_TRUE(result.NoErrors()); + EXPECT_FALSE(result.should_show_snooze); +} + TEST_F(FeatureConfigConditionValidatorTest, SingleEventChangingComparator) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures({kFeatureConfigTestFeatureFoo}, {}); diff --git a/chromium/components/feature_engagement/internal/feature_config_event_storage_validator.h b/chromium/components/feature_engagement/internal/feature_config_event_storage_validator.h index b7dd0683a1f..70b239fb8ee 100644 --- a/chromium/components/feature_engagement/internal/feature_config_event_storage_validator.h +++ b/chromium/components/feature_engagement/internal/feature_config_event_storage_validator.h @@ -22,6 +22,12 @@ struct FeatureConfig; class FeatureConfigEventStorageValidator : public EventStorageValidator { public: FeatureConfigEventStorageValidator(); + + FeatureConfigEventStorageValidator( + const FeatureConfigEventStorageValidator&) = delete; + FeatureConfigEventStorageValidator& operator=( + const FeatureConfigEventStorageValidator&) = delete; + ~FeatureConfigEventStorageValidator() override; // EventStorageValidator implementation. @@ -54,8 +60,6 @@ class FeatureConfigEventStorageValidator : public EventStorageValidator { // Contains the longest time to store each event across all EventConfigs, // as a number of days. std::unordered_map<std::string, uint32_t> longest_storage_times_; - - DISALLOW_COPY_AND_ASSIGN(FeatureConfigEventStorageValidator); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/feature_config_event_storage_validator_unittest.cc b/chromium/components/feature_engagement/internal/feature_config_event_storage_validator_unittest.cc index f26143913e1..9774f891ce2 100644 --- a/chromium/components/feature_engagement/internal/feature_config_event_storage_validator_unittest.cc +++ b/chromium/components/feature_engagement/internal/feature_config_event_storage_validator_unittest.cc @@ -104,6 +104,11 @@ class FeatureConfigEventStorageValidatorTest : public ::testing::Test { InitializeStorageFeatureConfigs(); } + FeatureConfigEventStorageValidatorTest( + const FeatureConfigEventStorageValidatorTest&) = delete; + FeatureConfigEventStorageValidatorTest& operator=( + const FeatureConfigEventStorageValidatorTest&) = delete; + void UseConfig(const FeatureConfig& foo_config) { FeatureVector features = {&kEventStorageTestFeatureFoo}; @@ -173,9 +178,6 @@ class FeatureConfigEventStorageValidatorTest : public ::testing::Test { FeatureConfigEventStorageValidator validator_; uint32_t current_day_; base::test::ScopedFeatureList scoped_feature_list_; - - private: - DISALLOW_COPY_AND_ASSIGN(FeatureConfigEventStorageValidatorTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/in_memory_event_store.h b/chromium/components/feature_engagement/internal/in_memory_event_store.h index 178b80d1b68..5912e270a5c 100644 --- a/chromium/components/feature_engagement/internal/in_memory_event_store.h +++ b/chromium/components/feature_engagement/internal/in_memory_event_store.h @@ -18,6 +18,10 @@ class InMemoryEventStore : public EventStore { public: explicit InMemoryEventStore(std::unique_ptr<std::vector<Event>> events); InMemoryEventStore(); + + InMemoryEventStore(const InMemoryEventStore&) = delete; + InMemoryEventStore& operator=(const InMemoryEventStore&) = delete; + ~InMemoryEventStore() override; // EventStore implementation. @@ -39,8 +43,6 @@ class InMemoryEventStore : public EventStore { // Whether the store is ready or not. It is true after Load(...) has been // invoked. bool ready_; - - DISALLOW_COPY_AND_ASSIGN(InMemoryEventStore); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/init_aware_event_model.cc b/chromium/components/feature_engagement/internal/init_aware_event_model.cc index 10951a68bc4..0c4a527b661 100644 --- a/chromium/components/feature_engagement/internal/init_aware_event_model.cc +++ b/chromium/components/feature_engagement/internal/init_aware_event_model.cc @@ -52,6 +52,32 @@ void InitAwareEventModel::IncrementEvent(const std::string& event_name, queued_events_.push_back(std::tie(event_name, current_day)); } +void InitAwareEventModel::IncrementSnooze(const std::string& event_name, + uint32_t current_day, + base::Time current_time) { + event_model_->IncrementSnooze(event_name, current_day, current_time); +} + +void InitAwareEventModel::DismissSnooze(const std::string& event_name) { + event_model_->DismissSnooze(event_name); +} + +base::Time InitAwareEventModel::GetLastSnoozeTimestamp( + const std::string& event_name) const { + return event_model_->GetLastSnoozeTimestamp(event_name); +} + +uint32_t InitAwareEventModel::GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const { + return event_model_->GetSnoozeCount(event_name, window, current_day); +} + +bool InitAwareEventModel::IsSnoozeDismissed( + const std::string& event_name) const { + return event_model_->IsSnoozeDismissed(event_name); +} + void InitAwareEventModel::OnInitializeComplete( OnModelInitializationFinished callback, bool success) { diff --git a/chromium/components/feature_engagement/internal/init_aware_event_model.h b/chromium/components/feature_engagement/internal/init_aware_event_model.h index c25a293cd84..3154266af86 100644 --- a/chromium/components/feature_engagement/internal/init_aware_event_model.h +++ b/chromium/components/feature_engagement/internal/init_aware_event_model.h @@ -20,6 +20,10 @@ namespace feature_engagement { class InitAwareEventModel : public EventModel { public: InitAwareEventModel(std::unique_ptr<EventModel> event_model); + + InitAwareEventModel(const InitAwareEventModel&) = delete; + InitAwareEventModel& operator=(const InitAwareEventModel&) = delete; + ~InitAwareEventModel() override; // EventModel implementation. @@ -32,6 +36,16 @@ class InitAwareEventModel : public EventModel { uint32_t window_size) const override; void IncrementEvent(const std::string& event_name, uint32_t current_day) override; + void IncrementSnooze(const std::string& event_name, + uint32_t current_day, + base::Time current_time) override; + void DismissSnooze(const std::string& event_name) override; + base::Time GetLastSnoozeTimestamp( + const std::string& event_name) const override; + uint32_t GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const override; + bool IsSnoozeDismissed(const std::string& event_name) const override; size_t GetQueuedEventCountForTesting(); @@ -48,8 +62,6 @@ class InitAwareEventModel : public EventModel { bool initialization_complete_; base::WeakPtrFactory<InitAwareEventModel> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(InitAwareEventModel); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/init_aware_event_model_unittest.cc b/chromium/components/feature_engagement/internal/init_aware_event_model_unittest.cc index 1ffe69baa20..427c00baf6b 100644 --- a/chromium/components/feature_engagement/internal/init_aware_event_model_unittest.cc +++ b/chromium/components/feature_engagement/internal/init_aware_event_model_unittest.cc @@ -27,6 +27,10 @@ namespace { class MockEventModel : public EventModel { public: MockEventModel() = default; + + MockEventModel(const MockEventModel&) = delete; + MockEventModel& operator=(const MockEventModel&) = delete; + ~MockEventModel() override = default; // EventModel implementation. @@ -36,9 +40,12 @@ class MockEventModel : public EventModel { MOCK_CONST_METHOD3(GetEventCount, uint32_t(const std::string&, uint32_t, uint32_t)); MOCK_METHOD2(IncrementEvent, void(const std::string&, uint32_t)); - - private: - DISALLOW_COPY_AND_ASSIGN(MockEventModel); + MOCK_METHOD3(IncrementSnooze, void(const std::string&, uint32_t, base::Time)); + MOCK_METHOD1(DismissSnooze, void(const std::string&)); + MOCK_CONST_METHOD1(GetLastSnoozeTimestamp, base::Time(const std::string&)); + MOCK_CONST_METHOD3(GetSnoozeCount, + uint32_t(const std::string&, uint32_t, uint32_t)); + MOCK_CONST_METHOD1(IsSnoozeDismissed, bool(const std::string&)); }; class InitAwareEventModelTest : public testing::Test { @@ -48,6 +55,9 @@ class InitAwareEventModelTest : public testing::Test { &InitAwareEventModelTest::OnModelInitialized, base::Unretained(this)); } + InitAwareEventModelTest(const InitAwareEventModelTest&) = delete; + InitAwareEventModelTest& operator=(const InitAwareEventModelTest&) = delete; + ~InitAwareEventModelTest() override = default; void SetUp() override { @@ -65,9 +75,6 @@ class InitAwareEventModelTest : public testing::Test { // Load callback tracking. absl::optional<bool> load_success_; EventModel::OnModelInitializationFinished load_callback_; - - private: - DISALLOW_COPY_AND_ASSIGN(InitAwareEventModelTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/never_availability_model.h b/chromium/components/feature_engagement/internal/never_availability_model.h index 27fef7fe235..7d38b42eb1c 100644 --- a/chromium/components/feature_engagement/internal/never_availability_model.h +++ b/chromium/components/feature_engagement/internal/never_availability_model.h @@ -17,6 +17,10 @@ namespace feature_engagement { class NeverAvailabilityModel : public AvailabilityModel { public: NeverAvailabilityModel(); + + NeverAvailabilityModel(const NeverAvailabilityModel&) = delete; + NeverAvailabilityModel& operator=(const NeverAvailabilityModel&) = delete; + ~NeverAvailabilityModel() override; // AvailabilityModel implementation. @@ -34,8 +38,6 @@ class NeverAvailabilityModel : public AvailabilityModel { // Whether the model has been successfully initialized. bool ready_; - - DISALLOW_COPY_AND_ASSIGN(NeverAvailabilityModel); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/never_availability_model_unittest.cc b/chromium/components/feature_engagement/internal/never_availability_model_unittest.cc index 294625dd63e..e3fce69a0cb 100644 --- a/chromium/components/feature_engagement/internal/never_availability_model_unittest.cc +++ b/chromium/components/feature_engagement/internal/never_availability_model_unittest.cc @@ -24,6 +24,10 @@ class NeverAvailabilityModelTest : public ::testing::Test { public: NeverAvailabilityModelTest() = default; + NeverAvailabilityModelTest(const NeverAvailabilityModelTest&) = delete; + NeverAvailabilityModelTest& operator=(const NeverAvailabilityModelTest&) = + delete; + void OnInitializedCallback(bool success) { success_ = success; } protected: @@ -32,8 +36,6 @@ class NeverAvailabilityModelTest : public ::testing::Test { private: base::test::SingleThreadTaskEnvironment task_environment_; - - DISALLOW_COPY_AND_ASSIGN(NeverAvailabilityModelTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/never_condition_validator.h b/chromium/components/feature_engagement/internal/never_condition_validator.h index 65888d2c334..834f8b3799d 100644 --- a/chromium/components/feature_engagement/internal/never_condition_validator.h +++ b/chromium/components/feature_engagement/internal/never_condition_validator.h @@ -23,6 +23,10 @@ class EventModel; class NeverConditionValidator : public ConditionValidator { public: NeverConditionValidator(); + + NeverConditionValidator(const NeverConditionValidator&) = delete; + NeverConditionValidator& operator=(const NeverConditionValidator&) = delete; + ~NeverConditionValidator() override; // ConditionValidator implementation. @@ -38,9 +42,6 @@ class NeverConditionValidator : public ConditionValidator { const FeatureConfig& config, const std::vector<std::string>& all_feature_names) override; void NotifyDismissed(const base::Feature& feature) override; - - private: - DISALLOW_COPY_AND_ASSIGN(NeverConditionValidator); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/never_condition_validator_unittest.cc b/chromium/components/feature_engagement/internal/never_condition_validator_unittest.cc index f693570225a..800864f1527 100644 --- a/chromium/components/feature_engagement/internal/never_condition_validator_unittest.cc +++ b/chromium/components/feature_engagement/internal/never_condition_validator_unittest.cc @@ -28,6 +28,9 @@ class NeverTestEventModel : public EventModel { public: NeverTestEventModel() = default; + NeverTestEventModel(const NeverTestEventModel&) = delete; + NeverTestEventModel& operator=(const NeverTestEventModel&) = delete; + void Initialize(OnModelInitializationFinished callback, uint32_t current_day) override {} @@ -45,22 +48,41 @@ class NeverTestEventModel : public EventModel { void IncrementEvent(const std::string& event_name, uint32_t day) override {} - private: - DISALLOW_COPY_AND_ASSIGN(NeverTestEventModel); + void IncrementSnooze(const std::string& event_name, + uint32_t day, + base::Time time) override {} + + void DismissSnooze(const std::string& event_name) override {} + + base::Time GetLastSnoozeTimestamp( + const std::string& event_name) const override { + return base::Time(); + } + + uint32_t GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const override { + return 0; + } + + bool IsSnoozeDismissed(const std::string& event_name) const override { + return false; + } }; class NeverConditionValidatorTest : public ::testing::Test { public: NeverConditionValidatorTest() = default; + NeverConditionValidatorTest(const NeverConditionValidatorTest&) = delete; + NeverConditionValidatorTest& operator=(const NeverConditionValidatorTest&) = + delete; + protected: NeverTestEventModel event_model_; NeverAvailabilityModel availability_model_; NoopDisplayLockController display_lock_controller_; NeverConditionValidator validator_; - - private: - DISALLOW_COPY_AND_ASSIGN(NeverConditionValidatorTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/never_event_storage_validator.h b/chromium/components/feature_engagement/internal/never_event_storage_validator.h index 10ed6798176..55aebc7623c 100644 --- a/chromium/components/feature_engagement/internal/never_event_storage_validator.h +++ b/chromium/components/feature_engagement/internal/never_event_storage_validator.h @@ -17,6 +17,11 @@ namespace feature_engagement { class NeverEventStorageValidator : public EventStorageValidator { public: NeverEventStorageValidator(); + + NeverEventStorageValidator(const NeverEventStorageValidator&) = delete; + NeverEventStorageValidator& operator=(const NeverEventStorageValidator&) = + delete; + ~NeverEventStorageValidator() override; // EventStorageValidator implementation. @@ -24,9 +29,6 @@ class NeverEventStorageValidator : public EventStorageValidator { bool ShouldKeep(const std::string& event_name, uint32_t event_day, uint32_t current_day) const override; - - private: - DISALLOW_COPY_AND_ASSIGN(NeverEventStorageValidator); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/never_event_storage_validator_unittest.cc b/chromium/components/feature_engagement/internal/never_event_storage_validator_unittest.cc index 42157625e76..27d035b144c 100644 --- a/chromium/components/feature_engagement/internal/never_event_storage_validator_unittest.cc +++ b/chromium/components/feature_engagement/internal/never_event_storage_validator_unittest.cc @@ -14,11 +14,13 @@ class NeverEventStorageValidatorTest : public ::testing::Test { public: NeverEventStorageValidatorTest() = default; + NeverEventStorageValidatorTest(const NeverEventStorageValidatorTest&) = + delete; + NeverEventStorageValidatorTest& operator=( + const NeverEventStorageValidatorTest&) = delete; + protected: NeverEventStorageValidator validator_; - - private: - DISALLOW_COPY_AND_ASSIGN(NeverEventStorageValidatorTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/noop_display_lock_controller.h b/chromium/components/feature_engagement/internal/noop_display_lock_controller.h index 6f2950b9b5c..cad33f0eaed 100644 --- a/chromium/components/feature_engagement/internal/noop_display_lock_controller.h +++ b/chromium/components/feature_engagement/internal/noop_display_lock_controller.h @@ -18,14 +18,16 @@ class DisplayLockHandle; class NoopDisplayLockController : public DisplayLockController { public: NoopDisplayLockController(); + + NoopDisplayLockController(const NoopDisplayLockController&) = delete; + NoopDisplayLockController& operator=(const NoopDisplayLockController&) = + delete; + ~NoopDisplayLockController() override; // DisplayLockController implementation. std::unique_ptr<DisplayLockHandle> AcquireDisplayLock() override; bool IsDisplayLocked() const override; - - private: - DISALLOW_COPY_AND_ASSIGN(NoopDisplayLockController); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/once_condition_validator.cc b/chromium/components/feature_engagement/internal/once_condition_validator.cc index b12db874bd6..b1feb00312b 100644 --- a/chromium/components/feature_engagement/internal/once_condition_validator.cc +++ b/chromium/components/feature_engagement/internal/once_condition_validator.cc @@ -32,6 +32,16 @@ ConditionValidator::Result OnceConditionValidator::MeetsConditions( result.session_rate_ok = shown_features_.find(feature.name) == shown_features_.end(); + result.snooze_expiration_ok = + !event_model.IsSnoozeDismissed(config.trigger.name) && + (event_model.GetLastSnoozeTimestamp(config.trigger.name) < + base::Time::Now() - base::Days(config.snooze_params.snooze_interval)); + + result.should_show_snooze = + result.snooze_expiration_ok && + event_model.GetSnoozeCount(config.trigger.name, config.trigger.window, + current_day) < config.snooze_params.max_limit; + return result; } diff --git a/chromium/components/feature_engagement/internal/once_condition_validator.h b/chromium/components/feature_engagement/internal/once_condition_validator.h index 434e1bddad2..497b1ee5e4f 100644 --- a/chromium/components/feature_engagement/internal/once_condition_validator.h +++ b/chromium/components/feature_engagement/internal/once_condition_validator.h @@ -36,6 +36,10 @@ class EventModel; class OnceConditionValidator : public ConditionValidator { public: OnceConditionValidator(); + + OnceConditionValidator(const OnceConditionValidator&) = delete; + OnceConditionValidator& operator=(const OnceConditionValidator&) = delete; + ~OnceConditionValidator() override; // ConditionValidator implementation. @@ -59,8 +63,6 @@ class OnceConditionValidator : public ConditionValidator { // Which feature that is currently being shown, or nullptr if nothing is // currently showing. std::string currently_showing_feature_; - - DISALLOW_COPY_AND_ASSIGN(OnceConditionValidator); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/once_condition_validator_unittest.cc b/chromium/components/feature_engagement/internal/once_condition_validator_unittest.cc index bc384705b70..8978b0c43b1 100644 --- a/chromium/components/feature_engagement/internal/once_condition_validator_unittest.cc +++ b/chromium/components/feature_engagement/internal/once_condition_validator_unittest.cc @@ -50,6 +50,27 @@ class OnceTestEventModel : public EventModel { void IncrementEvent(const std::string& event_name, uint32_t day) override {} + void IncrementSnooze(const std::string& event_name, + uint32_t day, + base::Time time) override {} + + void DismissSnooze(const std::string& event_name) override {} + + base::Time GetLastSnoozeTimestamp( + const std::string& event_name) const override { + return base::Time(); + } + + uint32_t GetSnoozeCount(const std::string& event_name, + uint32_t window, + uint32_t current_day) const override { + return 0; + } + + bool IsSnoozeDismissed(const std::string& event_name) const override { + return false; + } + private: bool ready_; }; @@ -61,15 +82,16 @@ class OnceConditionValidatorTest : public ::testing::Test { event_model_.SetIsReady(true); } + OnceConditionValidatorTest(const OnceConditionValidatorTest&) = delete; + OnceConditionValidatorTest& operator=(const OnceConditionValidatorTest&) = + delete; + protected: EditableConfiguration configuration_; OnceTestEventModel event_model_; NeverAvailabilityModel availability_model_; NoopDisplayLockController display_lock_controller_; OnceConditionValidator validator_; - - private: - DISALLOW_COPY_AND_ASSIGN(OnceConditionValidatorTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/persistent_availability_store.h b/chromium/components/feature_engagement/internal/persistent_availability_store.h index 5e0373ed6ec..383ba5ccbe2 100644 --- a/chromium/components/feature_engagement/internal/persistent_availability_store.h +++ b/chromium/components/feature_engagement/internal/persistent_availability_store.h @@ -34,6 +34,10 @@ class PersistentAvailabilityStore { using OnLoadedCallback = base::OnceCallback< void(bool success, std::unique_ptr<std::map<std::string, uint32_t>>)>; + PersistentAvailabilityStore(const PersistentAvailabilityStore&) = delete; + PersistentAvailabilityStore& operator=(const PersistentAvailabilityStore&) = + delete; + // Loads the availability data, updates the DB with newly enabled features, // deletes features that are not enabled anymore, and asynchronously invokes // |on_loaded_callback| with the result. The result will mirror the content @@ -52,8 +56,6 @@ class PersistentAvailabilityStore { private: PersistentAvailabilityStore() = default; ~PersistentAvailabilityStore() = default; - - DISALLOW_COPY_AND_ASSIGN(PersistentAvailabilityStore); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/persistent_availability_store_unittest.cc b/chromium/components/feature_engagement/internal/persistent_availability_store_unittest.cc index d2c32972430..9b7928e71da 100644 --- a/chromium/components/feature_engagement/internal/persistent_availability_store_unittest.cc +++ b/chromium/components/feature_engagement/internal/persistent_availability_store_unittest.cc @@ -51,6 +51,11 @@ class PersistentAvailabilityStoreTest : public testing::Test { &PersistentAvailabilityStoreTest::LoadCallback, base::Unretained(this)); } + PersistentAvailabilityStoreTest(const PersistentAvailabilityStoreTest&) = + delete; + PersistentAvailabilityStoreTest& operator=( + const PersistentAvailabilityStoreTest&) = delete; + ~PersistentAvailabilityStoreTest() override = default; // Creates a DB and stores off a pointer to it as a member. @@ -87,9 +92,6 @@ class PersistentAvailabilityStoreTest : public testing::Test { // Constant test data. base::FilePath storage_dir_; - - private: - DISALLOW_COPY_AND_ASSIGN(PersistentAvailabilityStoreTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/persistent_event_store.h b/chromium/components/feature_engagement/internal/persistent_event_store.h index 9a72760cf91..e438a318f8a 100644 --- a/chromium/components/feature_engagement/internal/persistent_event_store.h +++ b/chromium/components/feature_engagement/internal/persistent_event_store.h @@ -25,6 +25,10 @@ class PersistentEventStore : public EventStore { public: // Builds a PersistentEventStore backed by the ProtoDatabase |db|. PersistentEventStore(std::unique_ptr<leveldb_proto::ProtoDatabase<Event>> db); + + PersistentEventStore(const PersistentEventStore&) = delete; + PersistentEventStore& operator=(const PersistentEventStore&) = delete; + ~PersistentEventStore() override; // EventStore implementation. @@ -49,8 +53,6 @@ class PersistentEventStore : public EventStore { bool ready_; base::WeakPtrFactory<PersistentEventStore> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(PersistentEventStore); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/proto/feature_event.proto b/chromium/components/feature_engagement/internal/proto/feature_event.proto index 9f59b5266f5..09cdf684d15 100644 --- a/chromium/components/feature_engagement/internal/proto/feature_event.proto +++ b/chromium/components/feature_engagement/internal/proto/feature_event.proto @@ -12,16 +12,23 @@ package feature_engagement; // Event stores state for a specific event a count per day it has happened. message Event { - // Count stores a pair of a day and how many times something happened that - // day. + // Count stores a day, how many times something happened that + // day, and how many time it has been snoozed. message Count { optional uint32 day = 1; optional uint32 count = 2; + optional uint32 snooze_count = 3; } // The descriptive name of the event. optional string name = 1; - // The number of this event that happened per day. + // Per-day aggregation of event details. repeated Count events = 2; + + // The most recent timestamp of when this feature was snoozed. + optional int64 last_snooze_time_us = 3; + + // Whether the user has explicitly dismissed the snooze. + optional bool snooze_dismissed = 4; } diff --git a/chromium/components/feature_engagement/internal/single_invalid_configuration.h b/chromium/components/feature_engagement/internal/single_invalid_configuration.h index bcf23351e8b..68c24e7a305 100644 --- a/chromium/components/feature_engagement/internal/single_invalid_configuration.h +++ b/chromium/components/feature_engagement/internal/single_invalid_configuration.h @@ -22,6 +22,11 @@ namespace feature_engagement { class SingleInvalidConfiguration : public Configuration { public: SingleInvalidConfiguration(); + + SingleInvalidConfiguration(const SingleInvalidConfiguration&) = delete; + SingleInvalidConfiguration& operator=(const SingleInvalidConfiguration&) = + delete; + ~SingleInvalidConfiguration() override; // Configuration implementation. @@ -38,8 +43,6 @@ class SingleInvalidConfiguration : public Configuration { // An empty map. ConfigMap configs_; - - DISALLOW_COPY_AND_ASSIGN(SingleInvalidConfiguration); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/single_invalid_configuration_unittest.cc b/chromium/components/feature_engagement/internal/single_invalid_configuration_unittest.cc index cc37481f3a4..6b512d804c1 100644 --- a/chromium/components/feature_engagement/internal/single_invalid_configuration_unittest.cc +++ b/chromium/components/feature_engagement/internal/single_invalid_configuration_unittest.cc @@ -21,11 +21,13 @@ class SingleInvalidConfigurationTest : public ::testing::Test { public: SingleInvalidConfigurationTest() = default; + SingleInvalidConfigurationTest(const SingleInvalidConfigurationTest&) = + delete; + SingleInvalidConfigurationTest& operator=( + const SingleInvalidConfigurationTest&) = delete; + protected: SingleInvalidConfiguration configuration_; - - private: - DISALLOW_COPY_AND_ASSIGN(SingleInvalidConfigurationTest); }; } // namespace diff --git a/chromium/components/feature_engagement/internal/stats.cc b/chromium/components/feature_engagement/internal/stats.cc index 7d6577ed7ac..9b7e7925bae 100644 --- a/chromium/components/feature_engagement/internal/stats.cc +++ b/chromium/components/feature_engagement/internal/stats.cc @@ -178,6 +178,10 @@ void RecordUserDismiss() { base::RecordAction(base::UserMetricsAction("InProductHelp.Dismissed")); } +void RecordUserSnoozeAction(Tracker::SnoozeAction snooze_action) { + base::UmaHistogramEnumeration("InProductHelp.SnoozeAction", snooze_action); +} + void RecordDbUpdate(bool success, StoreType type) { std::string histogram_name = "InProductHelp.Db.Update." + ToDbHistogramSuffix(type); diff --git a/chromium/components/feature_engagement/internal/stats.h b/chromium/components/feature_engagement/internal/stats.h index b9ba3ff5d5d..cc018f5d904 100644 --- a/chromium/components/feature_engagement/internal/stats.h +++ b/chromium/components/feature_engagement/internal/stats.h @@ -11,6 +11,7 @@ #include "components/feature_engagement/internal/condition_validator.h" #include "components/feature_engagement/internal/proto/feature_event.pb.h" #include "components/feature_engagement/public/configuration.h" +#include "components/feature_engagement/public/tracker.h" namespace feature_engagement { namespace stats { @@ -71,6 +72,8 @@ enum class TriggerHelpUIResult { // Used in the metrics to track the configuration parsing event. // The failure reasons are not mutually exclusive. // Out-dated entries shouldn't be deleted but marked as obsolete. +// These values are persisted to logs. Entries should not be renumbered and +// numeric values should never be reused. // Keep this synced with the enum in //tools/metrics/histograms/enums.xml. enum class ConfigParsingEvent { // The configuration is parsed correctly. @@ -119,8 +122,11 @@ enum class ConfigParsingEvent { // Successfully read checked in configuration. SUCCESS_FROM_SOURCE = 14, + // Fails to parse the snooze parameters. + FAILURE_SNOOZE_PARAMS_PARSE = 15, + // Last entry for the enum. - COUNT = 15, + COUNT = 16, }; // Used in metrics to track database states. Each type will match to a suffix @@ -151,6 +157,9 @@ void RecordShouldTriggerHelpUI(const base::Feature& feature, // Records when the user dismisses the in-product help UI. void RecordUserDismiss(); +// Records when the user dismisses or snoozes the snoozable in-product help UI. +void RecordUserSnoozeAction(Tracker::SnoozeAction snooze_action); + // Records the result of database updates. void RecordDbUpdate(bool success, StoreType type); diff --git a/chromium/components/feature_engagement/internal/system_time_provider.h b/chromium/components/feature_engagement/internal/system_time_provider.h index bd48c687f21..8d1ba90665f 100644 --- a/chromium/components/feature_engagement/internal/system_time_provider.h +++ b/chromium/components/feature_engagement/internal/system_time_provider.h @@ -15,18 +15,15 @@ namespace feature_engagement { class SystemTimeProvider : public TimeProvider { public: SystemTimeProvider(); + + SystemTimeProvider(const SystemTimeProvider&) = delete; + SystemTimeProvider& operator=(const SystemTimeProvider&) = delete; + ~SystemTimeProvider() override; // TimeProvider implementation. uint32_t GetCurrentDay() const override; - - protected: - // Return the current time. - // virtual for testing. - virtual base::Time Now() const; - - private: - DISALLOW_COPY_AND_ASSIGN(SystemTimeProvider); + base::Time Now() const override; }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/system_time_provider_unittest.cc b/chromium/components/feature_engagement/internal/system_time_provider_unittest.cc index d1ab4720d23..eb9a7c94c48 100644 --- a/chromium/components/feature_engagement/internal/system_time_provider_unittest.cc +++ b/chromium/components/feature_engagement/internal/system_time_provider_unittest.cc @@ -33,6 +33,9 @@ class TestSystemTimeProvider : public SystemTimeProvider { public: TestSystemTimeProvider() = default; + TestSystemTimeProvider(const TestSystemTimeProvider&) = delete; + TestSystemTimeProvider& operator=(const TestSystemTimeProvider&) = delete; + // SystemTimeProvider implementation. base::Time Now() const override { return current_time_; } @@ -40,19 +43,17 @@ class TestSystemTimeProvider : public SystemTimeProvider { private: base::Time current_time_; - - DISALLOW_COPY_AND_ASSIGN(TestSystemTimeProvider); }; class SystemTimeProviderTest : public ::testing::Test { public: SystemTimeProviderTest() = default; + SystemTimeProviderTest(const SystemTimeProviderTest&) = delete; + SystemTimeProviderTest& operator=(const SystemTimeProviderTest&) = delete; + protected: TestSystemTimeProvider time_provider_; - - private: - DISALLOW_COPY_AND_ASSIGN(SystemTimeProviderTest); }; } // namespace @@ -65,26 +66,26 @@ TEST_F(SystemTimeProviderTest, EpochIs0Days) { TEST_F(SystemTimeProviderTest, TestDeltasFromEpoch) { base::Time epoch = base::Time::UnixEpoch(); - time_provider_.SetCurrentTime(epoch + base::TimeDelta::FromDays(1)); + time_provider_.SetCurrentTime(epoch + base::Days(1)); EXPECT_EQ(1u, time_provider_.GetCurrentDay()); - time_provider_.SetCurrentTime(epoch + base::TimeDelta::FromDays(2)); + time_provider_.SetCurrentTime(epoch + base::Days(2)); EXPECT_EQ(2u, time_provider_.GetCurrentDay()); - time_provider_.SetCurrentTime(epoch + base::TimeDelta::FromDays(100)); + time_provider_.SetCurrentTime(epoch + base::Days(100)); EXPECT_EQ(100u, time_provider_.GetCurrentDay()); } TEST_F(SystemTimeProviderTest, TestNegativeDeltasFromEpoch) { base::Time epoch = base::Time::UnixEpoch(); - time_provider_.SetCurrentTime(epoch - base::TimeDelta::FromDays(1)); + time_provider_.SetCurrentTime(epoch - base::Days(1)); EXPECT_EQ(0u, time_provider_.GetCurrentDay()); - time_provider_.SetCurrentTime(epoch - base::TimeDelta::FromDays(2)); + time_provider_.SetCurrentTime(epoch - base::Days(2)); EXPECT_EQ(0u, time_provider_.GetCurrentDay()); - time_provider_.SetCurrentTime(epoch - base::TimeDelta::FromDays(100)); + time_provider_.SetCurrentTime(epoch - base::Days(100)); EXPECT_EQ(0u, time_provider_.GetCurrentDay()); } diff --git a/chromium/components/feature_engagement/internal/time_provider.h b/chromium/components/feature_engagement/internal/time_provider.h index 50c73eeee69..ad316ded39c 100644 --- a/chromium/components/feature_engagement/internal/time_provider.h +++ b/chromium/components/feature_engagement/internal/time_provider.h @@ -8,22 +8,26 @@ #include <stdint.h> #include "base/macros.h" +#include "base/time/time.h" namespace feature_engagement { // A TimeProvider provides functionality related to time. class TimeProvider { public: + TimeProvider(const TimeProvider&) = delete; + TimeProvider& operator=(const TimeProvider&) = delete; + virtual ~TimeProvider() = default; // Returns the number of days since epoch (1970-01-01) in the local timezone. virtual uint32_t GetCurrentDay() const = 0; + // Returns the current time. + virtual base::Time Now() const = 0; + protected: TimeProvider() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(TimeProvider); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/tracker_impl.cc b/chromium/components/feature_engagement/internal/tracker_impl.cc index 4e83a903ff4..42d6a767009 100644 --- a/chromium/components/feature_engagement/internal/tracker_impl.cc +++ b/chromium/components/feature_engagement/internal/tracker_impl.cc @@ -176,6 +176,11 @@ void TrackerImpl::NotifyEvent(const std::string& event) { } bool TrackerImpl::ShouldTriggerHelpUI(const base::Feature& feature) { + return ShouldTriggerHelpUIWithSnooze(feature).ShouldShowIph(); +} + +TrackerImpl::TriggerDetails TrackerImpl::ShouldTriggerHelpUIWithSnooze( + const base::Feature& feature) { FeatureConfig feature_config = configuration_->GetFeatureConfig(feature); ConditionValidator::Result result = condition_validator_->MeetsConditions( feature, feature_config, *event_model_, *availability_model_, @@ -195,6 +200,8 @@ bool TrackerImpl::ShouldTriggerHelpUI(const base::Feature& feature) { << " tracking_only=" << feature_config.tracking_only << " " << result; + bool should_show_iph = false; + if (feature_config.tracking_only) { if (result.NoErrors()) { // Because tracking only features always return false to the client, @@ -203,10 +210,12 @@ bool TrackerImpl::ShouldTriggerHelpUI(const base::Feature& feature) { // showing. See https://crbug.com/1188679 for more details. Dismissed(feature); } - return false; + should_show_iph = false; } else { - return result.NoErrors(); + should_show_iph = result.NoErrors(); } + + return TriggerDetails(should_show_iph, result.should_show_snooze); } bool TrackerImpl::WouldTriggerHelpUI(const base::Feature& feature) const { @@ -266,6 +275,21 @@ void TrackerImpl::Dismissed(const base::Feature& feature) { stats::RecordUserDismiss(); } +void TrackerImpl::DismissedWithSnooze( + const base::Feature& feature, + absl::optional<SnoozeAction> snooze_action) { + FeatureConfig feature_config = configuration_->GetFeatureConfig(feature); + if (snooze_action == SnoozeAction::SNOOZED) { + event_model_->IncrementSnooze(feature_config.trigger.name, + time_provider_->GetCurrentDay(), + time_provider_->Now()); + } else if (snooze_action == SnoozeAction::DISMISSED) { + event_model_->DismissSnooze(feature_config.trigger.name); + } + if (snooze_action.has_value()) + stats::RecordUserSnoozeAction(snooze_action.value()); +} + std::unique_ptr<DisplayLockHandle> TrackerImpl::AcquireDisplayLock() { return display_lock_controller_->AcquireDisplayLock(); } diff --git a/chromium/components/feature_engagement/internal/tracker_impl.h b/chromium/components/feature_engagement/internal/tracker_impl.h index 83218427014..05fd637ac47 100644 --- a/chromium/components/feature_engagement/internal/tracker_impl.h +++ b/chromium/components/feature_engagement/internal/tracker_impl.h @@ -16,8 +16,8 @@ namespace feature_engagement { class AvailabilityModel; -class Configuration; class ConditionValidator; +class Configuration; class DisplayLockController; class DisplayLockHandle; class EventModel; @@ -32,17 +32,25 @@ class TrackerImpl : public Tracker { std::unique_ptr<DisplayLockController> display_lock_controller, std::unique_ptr<ConditionValidator> condition_validator, std::unique_ptr<TimeProvider> time_provider); + + TrackerImpl(const TrackerImpl&) = delete; + TrackerImpl& operator=(const TrackerImpl&) = delete; + ~TrackerImpl() override; // Tracker implementation. void NotifyEvent(const std::string& event) override; bool ShouldTriggerHelpUI(const base::Feature& feature) override; + TriggerDetails ShouldTriggerHelpUIWithSnooze( + const base::Feature& feature) override; bool WouldTriggerHelpUI(const base::Feature& feature) const override; Tracker::TriggerState GetTriggerState( const base::Feature& feature) const override; bool HasEverTriggered(const base::Feature& feature, bool from_window) const override; void Dismissed(const base::Feature& feature) override; + void DismissedWithSnooze(const base::Feature& feature, + absl::optional<SnoozeAction> snooze_action) override; std::unique_ptr<DisplayLockHandle> AcquireDisplayLock() override; bool IsInitialized() const override; void AddOnInitializedCallback(OnInitializedCallback callback) override; @@ -96,8 +104,6 @@ class TrackerImpl : public Tracker { std::vector<OnInitializedCallback> on_initialized_callbacks_; base::WeakPtrFactory<TrackerImpl> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(TrackerImpl); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/internal/tracker_impl_unittest.cc b/chromium/components/feature_engagement/internal/tracker_impl_unittest.cc index bf92e5a0f2e..d829988b151 100644 --- a/chromium/components/feature_engagement/internal/tracker_impl_unittest.cc +++ b/chromium/components/feature_engagement/internal/tracker_impl_unittest.cc @@ -41,17 +41,24 @@ const base::Feature kTrackerTestFeatureBaz{"test_baz", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kTrackerTestFeatureQux{"test_qux", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kTrackerTestFeatureSnooze{ + "test_snooze", base::FEATURE_DISABLED_BY_DEFAULT}; void RegisterFeatureConfig(EditableConfiguration* configuration, const base::Feature& feature, bool valid, - bool tracking_only) { + bool tracking_only, + bool snooze_params) { FeatureConfig config; config.valid = valid; config.used.name = feature.name + std::string("_used"); config.trigger.name = feature.name + std::string("_trigger"); config.trigger.storage = 1u; config.tracking_only = tracking_only; + if (snooze_params) { + config.snooze_params.snooze_interval = 7u; + config.snooze_params.max_limit = 3u; + } configuration->SetConfiguration(&feature, config); } @@ -61,6 +68,10 @@ class StoringInitializedCallback { public: StoringInitializedCallback() : invoked_(false), success_(false) {} + StoringInitializedCallback(const StoringInitializedCallback&) = delete; + StoringInitializedCallback& operator=(const StoringInitializedCallback&) = + delete; + void OnInitialized(bool success) { DCHECK(!invoked_); invoked_ = true; @@ -74,8 +85,6 @@ class StoringInitializedCallback { private: bool invoked_; bool success_; - - DISALLOW_COPY_AND_ASSIGN(StoringInitializedCallback); }; // An InMemoryEventStore that is able to fake successful and unsuccessful @@ -85,6 +94,10 @@ class TestTrackerInMemoryEventStore : public InMemoryEventStore { explicit TestTrackerInMemoryEventStore(bool load_should_succeed) : load_should_succeed_(load_should_succeed) {} + TestTrackerInMemoryEventStore(const TestTrackerInMemoryEventStore&) = delete; + TestTrackerInMemoryEventStore& operator=( + const TestTrackerInMemoryEventStore&) = delete; + void Load(OnLoadedCallback callback) override { HandleLoadResult(std::move(callback), load_should_succeed_); } @@ -101,13 +114,17 @@ class TestTrackerInMemoryEventStore : public InMemoryEventStore { bool load_should_succeed_; std::map<std::string, Event> events_; - - DISALLOW_COPY_AND_ASSIGN(TestTrackerInMemoryEventStore); }; class StoreEverythingEventStorageValidator : public EventStorageValidator { public: StoreEverythingEventStorageValidator() = default; + + StoreEverythingEventStorageValidator( + const StoreEverythingEventStorageValidator&) = delete; + StoreEverythingEventStorageValidator& operator=( + const StoreEverythingEventStorageValidator&) = delete; + ~StoreEverythingEventStorageValidator() override = default; bool ShouldStore(const std::string& event_name) const override { @@ -119,26 +136,36 @@ class StoreEverythingEventStorageValidator : public EventStorageValidator { uint32_t current_day) const override { return true; } - - private: - DISALLOW_COPY_AND_ASSIGN(StoreEverythingEventStorageValidator); }; class TestTimeProvider : public TimeProvider { public: TestTimeProvider() = default; + + TestTimeProvider(const TestTimeProvider&) = delete; + TestTimeProvider& operator=(const TestTimeProvider&) = delete; + ~TestTimeProvider() override = default; // TimeProvider implementation. uint32_t GetCurrentDay() const override { return 1u; } + base::Time Now() const override { return now_; } + + void SetCurrentTime(base::Time now) { now_ = now; } + private: - DISALLOW_COPY_AND_ASSIGN(TestTimeProvider); + base::Time now_; }; class TestTrackerAvailabilityModel : public AvailabilityModel { public: TestTrackerAvailabilityModel() : ready_(true) {} + + TestTrackerAvailabilityModel(const TestTrackerAvailabilityModel&) = delete; + TestTrackerAvailabilityModel& operator=(const TestTrackerAvailabilityModel&) = + delete; + ~TestTrackerAvailabilityModel() override = default; void Initialize(AvailabilityModel::OnInitializedCallback callback, @@ -158,13 +185,17 @@ class TestTrackerAvailabilityModel : public AvailabilityModel { private: bool ready_; - - DISALLOW_COPY_AND_ASSIGN(TestTrackerAvailabilityModel); }; class TestTrackerDisplayLockController : public DisplayLockController { public: TestTrackerDisplayLockController() = default; + + TestTrackerDisplayLockController(const TestTrackerDisplayLockController&) = + delete; + TestTrackerDisplayLockController& operator=( + const TestTrackerDisplayLockController&) = delete; + ~TestTrackerDisplayLockController() override = default; std::unique_ptr<DisplayLockHandle> AcquireDisplayLock() override { @@ -181,27 +212,35 @@ class TestTrackerDisplayLockController : public DisplayLockController { private: // The next DisplayLockHandle to return. std::unique_ptr<DisplayLockHandle> next_display_lock_handle_; - - DISALLOW_COPY_AND_ASSIGN(TestTrackerDisplayLockController); }; class TrackerImplTest : public ::testing::Test { public: TrackerImplTest() = default; + TrackerImplTest(const TrackerImplTest&) = delete; + TrackerImplTest& operator=(const TrackerImplTest&) = delete; + void SetUp() override { std::unique_ptr<EditableConfiguration> configuration = std::make_unique<EditableConfiguration>(); configuration_ = configuration.get(); RegisterFeatureConfig(configuration.get(), kTrackerTestFeatureFoo, - true /* is_valid */, false /* tracking_only */); + true /* is_valid */, false /* tracking_only */, + false /* snooze_params */); RegisterFeatureConfig(configuration.get(), kTrackerTestFeatureBar, - true /* is_valid */, false /* tracking_only */); + true /* is_valid */, false /* tracking_only */, + false /* snooze_params */); RegisterFeatureConfig(configuration.get(), kTrackerTestFeatureBaz, - true /* is_valid */, true /* tracking_only */); + true /* is_valid */, true /* tracking_only */, + false /* snooze_params */); RegisterFeatureConfig(configuration.get(), kTrackerTestFeatureQux, - false /* is_valid */, false /* tracking_only */); + false /* is_valid */, false /* tracking_only */, + false /* snooze_params */); + RegisterFeatureConfig(configuration.get(), kTrackerTestFeatureSnooze, + true /* is_valid */, false /* tracking_only */, + true /* snooze_params */); std::unique_ptr<TestTrackerInMemoryEventStore> event_store = CreateEventStore(); @@ -219,11 +258,13 @@ class TrackerImplTest : public ::testing::Test { std::make_unique<TestTrackerDisplayLockController>(); display_lock_controller_ = display_lock_controller.get(); + auto time_provider = std::make_unique<TestTimeProvider>(); + time_provider_ = time_provider.get(); + tracker_ = std::make_unique<TrackerImpl>( std::move(event_model), std::move(availability_model), std::move(configuration), std::move(display_lock_controller), - std::make_unique<OnceConditionValidator>(), - std::make_unique<TestTimeProvider>()); + std::make_unique<OnceConditionValidator>(), std::move(time_provider)); } void VerifyEventTriggerEvents(const base::Feature& feature, uint32_t count) { @@ -423,9 +464,7 @@ class TrackerImplTest : public ::testing::Test { TestTrackerDisplayLockController* display_lock_controller_; Configuration* configuration_; base::HistogramTester histogram_tester_; - - private: - DISALLOW_COPY_AND_ASSIGN(TrackerImplTest); + TestTimeProvider* time_provider_; }; // A top-level test class where the store fails to initialize. @@ -433,14 +472,16 @@ class FailingStoreInitTrackerImplTest : public TrackerImplTest { public: FailingStoreInitTrackerImplTest() = default; + FailingStoreInitTrackerImplTest(const FailingStoreInitTrackerImplTest&) = + delete; + FailingStoreInitTrackerImplTest& operator=( + const FailingStoreInitTrackerImplTest&) = delete; + protected: std::unique_ptr<TestTrackerInMemoryEventStore> CreateEventStore() override { // Returns a EventStore that will fail to initialize. return std::make_unique<TestTrackerInMemoryEventStore>(false); } - - private: - DISALLOW_COPY_AND_ASSIGN(FailingStoreInitTrackerImplTest); }; // A top-level test class where the AvailabilityModel fails to initialize. @@ -448,11 +489,13 @@ class FailingAvailabilityModelInitTrackerImplTest : public TrackerImplTest { public: FailingAvailabilityModelInitTrackerImplTest() = default; + FailingAvailabilityModelInitTrackerImplTest( + const FailingAvailabilityModelInitTrackerImplTest&) = delete; + FailingAvailabilityModelInitTrackerImplTest& operator=( + const FailingAvailabilityModelInitTrackerImplTest&) = delete; + protected: bool ShouldAvailabilityStoreBeReady() override { return false; } - - private: - DISALLOW_COPY_AND_ASSIGN(FailingAvailabilityModelInitTrackerImplTest); }; } // namespace @@ -671,6 +714,54 @@ TEST_F(TrackerImplTest, TestTriggering) { VerifyHistograms(true, 1, 3, 0, true, 1, 2, 0, false, 0, 0, 0, true, 0, 4, 0); } +TEST_F(TrackerImplTest, TestTriggeringWithSnooze) { + // Ensure all initialization is finished. + StoringInitializedCallback callback; + tracker_->AddOnInitializedCallback(base::BindOnce( + &StoringInitializedCallback::OnInitialized, base::Unretained(&callback))); + base::RunLoop().RunUntilIdle(); + + base::Time now = base::Time::Now(); + time_provider_->SetCurrentTime(now); + + // The first time a feature with snooze params triggers, it should be shown + // with snooze. + Tracker::TriggerDetails trigger_details = + tracker_->ShouldTriggerHelpUIWithSnooze(kTrackerTestFeatureSnooze); + EXPECT_TRUE(trigger_details.ShouldShowIph()); + EXPECT_TRUE(trigger_details.ShouldShowSnooze()); + + Event snooze_event = event_store_->GetEvent( + configuration_->GetFeatureConfig(kTrackerTestFeatureSnooze).trigger.name); + ASSERT_EQ(1, snooze_event.events_size()); + ASSERT_EQ(1u, snooze_event.events(0).day()); + ASSERT_EQ(1u, snooze_event.events(0).count()); + ASSERT_EQ(0u, snooze_event.events(0).snooze_count()); + + tracker_->DismissedWithSnooze(kTrackerTestFeatureSnooze, + Tracker::SnoozeAction::SNOOZED); + snooze_event = event_store_->GetEvent( + configuration_->GetFeatureConfig(kTrackerTestFeatureSnooze).trigger.name); + trigger_details = + tracker_->ShouldTriggerHelpUIWithSnooze(kTrackerTestFeatureSnooze); + EXPECT_FALSE(trigger_details.ShouldShowIph()); + EXPECT_FALSE(trigger_details.ShouldShowSnooze()); + ASSERT_EQ(1u, snooze_event.events(0).snooze_count()); + ASSERT_EQ(now.ToDeltaSinceWindowsEpoch().InMicroseconds(), + snooze_event.last_snooze_time_us()); + ASSERT_EQ(false, snooze_event.snooze_dismissed()); + + // TODO(crbug.com/1238924): Investigate using FeatureConfigConditionValidator + // here to test for snooze expiration after snooze's time interval. + + tracker_->DismissedWithSnooze(kTrackerTestFeatureSnooze, + Tracker::SnoozeAction::DISMISSED); + snooze_event = event_store_->GetEvent( + configuration_->GetFeatureConfig(kTrackerTestFeatureSnooze).trigger.name); + ASSERT_EQ(1u, snooze_event.events(0).snooze_count()); + ASSERT_EQ(true, snooze_event.snooze_dismissed()); +} + TEST_F(TrackerImplTest, TestTrackingOnlyTriggering) { // Ensure all initialization is finished. StoringInitializedCallback callback; diff --git a/chromium/components/feature_engagement/public/BUILD.gn b/chromium/components/feature_engagement/public/BUILD.gn index f458072d849..8b523191bd7 100644 --- a/chromium/components/feature_engagement/public/BUILD.gn +++ b/chromium/components/feature_engagement/public/BUILD.gn @@ -62,6 +62,7 @@ if (is_android) { "android/java/src/org/chromium/components/feature_engagement/EventConstants.java", "android/java/src/org/chromium/components/feature_engagement/FeatureConstants.java", "android/java/src/org/chromium/components/feature_engagement/Tracker.java", + "android/java/src/org/chromium/components/feature_engagement/TriggerDetails.java", ] deps = [ diff --git a/chromium/components/feature_engagement/public/android/wrapping_test_tracker.cc b/chromium/components/feature_engagement/public/android/wrapping_test_tracker.cc index 2ba4bf8db42..951bbb755aa 100644 --- a/chromium/components/feature_engagement/public/android/wrapping_test_tracker.cc +++ b/chromium/components/feature_engagement/public/android/wrapping_test_tracker.cc @@ -34,6 +34,11 @@ bool WrappingTestTracker::ShouldTriggerHelpUI(const base::Feature& feature) { base::android::AttachCurrentThread(), java_tracker_, jfeature); } +Tracker::TriggerDetails WrappingTestTracker::ShouldTriggerHelpUIWithSnooze( + const base::Feature& feature) { + return Tracker::TriggerDetails(false, false); +} + bool WrappingTestTracker::WouldTriggerHelpUI( const base::Feature& feature) const { JNIEnv* env = base::android::AttachCurrentThread(); @@ -71,6 +76,17 @@ void WrappingTestTracker::Dismissed(const base::Feature& feature) { java_tracker_, jfeature); } +void WrappingTestTracker::DismissedWithSnooze( + const base::Feature& feature, + absl::optional<SnoozeAction> snooze_action) { + JNIEnv* env = base::android::AttachCurrentThread(); + base::android::ScopedJavaLocalRef<jstring> jfeature( + base::android::ConvertUTF8ToJavaString(env, feature.name)); + Java_CppWrappedTestTracker_dismissedWithSnooze( + base::android::AttachCurrentThread(), java_tracker_, jfeature, + static_cast<int>(snooze_action.value())); +} + std::unique_ptr<DisplayLockHandle> WrappingTestTracker::AcquireDisplayLock() { return nullptr; } diff --git a/chromium/components/feature_engagement/public/android/wrapping_test_tracker.h b/chromium/components/feature_engagement/public/android/wrapping_test_tracker.h index 1fb51465920..cecbf4fafe6 100644 --- a/chromium/components/feature_engagement/public/android/wrapping_test_tracker.h +++ b/chromium/components/feature_engagement/public/android/wrapping_test_tracker.h @@ -18,25 +18,31 @@ namespace feature_engagement { class WrappingTestTracker : public Tracker { public: explicit WrappingTestTracker(const base::android::JavaRef<jobject>& jtracker); + + WrappingTestTracker(const WrappingTestTracker&) = delete; + WrappingTestTracker& operator=(const WrappingTestTracker&) = delete; + ~WrappingTestTracker() override; // TrackerImpl: void NotifyEvent(const std::string& event) override; bool ShouldTriggerHelpUI(const base::Feature& feature) override; + TriggerDetails ShouldTriggerHelpUIWithSnooze( + const base::Feature& feature) override; bool WouldTriggerHelpUI(const base::Feature& feature) const override; bool HasEverTriggered(const base::Feature& feature, bool from_window) const override; TriggerState GetTriggerState(const base::Feature& feature) const override; void Dismissed(const base::Feature& feature) override; + void DismissedWithSnooze(const base::Feature& feature, + absl::optional<SnoozeAction> snooze_action) override; std::unique_ptr<DisplayLockHandle> AcquireDisplayLock() override; bool IsInitialized() const override; void AddOnInitializedCallback(OnInitializedCallback callback) override; private: base::android::ScopedJavaGlobalRef<jobject> java_tracker_; - - DISALLOW_COPY_AND_ASSIGN(WrappingTestTracker); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/public/configuration.cc b/chromium/components/feature_engagement/public/configuration.cc index bde709171a7..17e4478b8d9 100644 --- a/chromium/components/feature_engagement/public/configuration.cc +++ b/chromium/components/feature_engagement/public/configuration.cc @@ -103,6 +103,12 @@ SessionRateImpact::SessionRateImpact(const SessionRateImpact& other) = default; SessionRateImpact::~SessionRateImpact() = default; +SnoozeParams::SnoozeParams() = default; + +SnoozeParams::SnoozeParams(const SnoozeParams& other) = default; + +SnoozeParams::~SnoozeParams() = default; + std::ostream& operator<<(std::ostream& os, const SessionRateImpact& impact) { os << "{ type: " << impact.type << ", affected_features: "; if (!impact.affected_features.has_value()) @@ -126,7 +132,7 @@ bool operator==(const SessionRateImpact& lhs, const SessionRateImpact& rhs) { std::tie(rhs.type, rhs.affected_features); } -FeatureConfig::FeatureConfig() : valid(false), tracking_only(false) {} +FeatureConfig::FeatureConfig() : valid(false) {} FeatureConfig::FeatureConfig(const FeatureConfig& other) = default; diff --git a/chromium/components/feature_engagement/public/configuration.h b/chromium/components/feature_engagement/public/configuration.h index 244ee94f8d1..cf0b99cd869 100644 --- a/chromium/components/feature_engagement/public/configuration.h +++ b/chromium/components/feature_engagement/public/configuration.h @@ -105,6 +105,20 @@ struct SessionRateImpact { absl::optional<std::vector<std::string>> affected_features; }; +// A SnoozeParams describes the parameters for snoozable options of in-product +// help. +struct SnoozeParams { + public: + // The maximum number of times an in-product-help can be snoozed. + uint32_t max_limit{0}; + // The minimum time interval between snoozes. + uint32_t snooze_interval{0}; + + SnoozeParams(); + SnoozeParams(const SnoozeParams& other); + ~SnoozeParams(); +}; + bool operator==(const SessionRateImpact& lhs, const SessionRateImpact& rhs); std::ostream& operator<<(std::ostream& os, const SessionRateImpact& impact); @@ -146,7 +160,10 @@ struct FeatureConfig { // Tracker::ShouldTriggerHelpUI(...) always returns false, but if all // other conditions are met, it will still be recorded as having been // shown in the internal database and through UMA. - bool tracking_only; + bool tracking_only{false}; + + // Snoozing parameter to decide if in-product help should be shown. + SnoozeParams snooze_params; }; bool operator==(const FeatureConfig& lhs, const FeatureConfig& rhs); @@ -160,6 +177,9 @@ class Configuration { // Convenience alias for typical implementations of Configuration. using ConfigMap = std::map<std::string, FeatureConfig>; + Configuration(const Configuration&) = delete; + Configuration& operator=(const Configuration&) = delete; + virtual ~Configuration() = default; // Returns the FeatureConfig for the given |feature|. The |feature| must @@ -180,9 +200,6 @@ class Configuration { protected: Configuration() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(Configuration); }; } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/public/event_constants.cc b/chromium/components/feature_engagement/public/event_constants.cc index 3214bb42dc6..da1008fbf4f 100644 --- a/chromium/components/feature_engagement/public/event_constants.cc +++ b/chromium/components/feature_engagement/public/event_constants.cc @@ -29,6 +29,8 @@ const char kGlobalMediaControlsOpened[] = "global_media_controls_opened"; const char kFocusModeOpened[] = "focus_mode_opened"; const char kFocusModeConditionsMet[] = "focus_mode_conditions_met"; +const char kSideSearchOpened[] = "side_search_opened"; + const char kTabSearchOpened[] = "tab_search_opened"; const char kWebUITabStripClosed[] = "webui_tab_strip_closed"; diff --git a/chromium/components/feature_engagement/public/event_constants.h b/chromium/components/feature_engagement/public/event_constants.h index cb5ad5ded49..538afd53749 100644 --- a/chromium/components/feature_engagement/public/event_constants.h +++ b/chromium/components/feature_engagement/public/event_constants.h @@ -51,6 +51,9 @@ extern const char kFocusModeOpened[]; // All conditions for show Focus Mode IPH were met. extern const char kFocusModeConditionsMet[]; +// The Side search panel was opened by the user. +extern const char kSideSearchOpened[]; + // Tab Search tab strip was opened by the user. extern const char kTabSearchOpened[]; diff --git a/chromium/components/feature_engagement/public/feature_configurations.cc b/chromium/components/feature_engagement/public/feature_configurations.cc index 487a454dc65..4d86e0a061d 100644 --- a/chromium/components/feature_engagement/public/feature_configurations.cc +++ b/chromium/components/feature_engagement/public/feature_configurations.cc @@ -41,6 +41,31 @@ absl::optional<FeatureConfig> GetClientSideFeatureConfig( EventConfig("profile_menu_shown", Comparator(EQUAL, 0), 360, 360); return config; } + + if (kIPHReadingListInSidePanelFeature.name == feature->name) { + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + // Show the promo once a year if the side panel was not opened. + config->trigger = + EventConfig("side_panel_trigger", Comparator(EQUAL, 0), 360, 360); + config->used = + EventConfig("side_panel_shown", Comparator(EQUAL, 0), 360, 360); + return config; + } + + if (kIPHGMCCastStartStopFeature.name == feature->name) { + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(EQUAL, 0); + config->trigger = EventConfig("gmc_start_stop_iph_trigger", + Comparator(EQUAL, 0), 180, 180); + config->used = EventConfig("media_route_stopped_from_gmc", + Comparator(EQUAL, 0), 180, 180); + return config; + } #endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) || // defined(OS_CHROMEOS) @@ -97,6 +122,8 @@ absl::optional<FeatureConfig> GetClientSideFeatureConfig( EventConfig("download_home_opened", Comparator(EQUAL, 0), 90, 360); config->event_configs.insert(EventConfig( "download_completed", Comparator(GREATER_THAN_OR_EQUAL, 1), 90, 360)); + config->snooze_params.snooze_interval = 7; + config->snooze_params.max_limit = 3; return config; } if (kIPHDownloadIndicatorFeature.name == feature->name) { @@ -244,6 +271,137 @@ absl::optional<FeatureConfig> GetClientSideFeatureConfig( return config; } + if (kIPHVideoTutorialNTPChromeIntroFeature.name == feature->name) { + // A config that allows the chrome intro video tutorial card to show up + // until explicitly interacted upon. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(GREATER_THAN_OR_EQUAL, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = EventConfig("tutorial_chrome_intro_iph_trigger", + Comparator(ANY, 0), 90, 90); + config->used = EventConfig("chrome_intro", Comparator(ANY, 0), 90, 90); + config->event_configs.insert( + EventConfig("video_tutorial_iph_clicked_chrome_intro", + Comparator(EQUAL, 0), 90, 90)); + config->event_configs.insert( + EventConfig("video_tutorial_iph_dismissed_chrome_intro", + Comparator(EQUAL, 0), 90, 90)); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::NONE; + config->session_rate_impact = session_rate_impact; + + return config; + } + + if (kIPHVideoTutorialNTPDownloadFeature.name == feature->name) { + // A config that allows the download video tutorial card to show up + // until explicitly interacted upon. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(GREATER_THAN_OR_EQUAL, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = EventConfig("tutorial_download_iph_trigger", + Comparator(ANY, 0), 90, 90); + config->used = EventConfig("download", Comparator(ANY, 0), 90, 90); + config->event_configs.insert(EventConfig( + "video_tutorial_iph_clicked_download", Comparator(EQUAL, 0), 90, 90)); + config->event_configs.insert(EventConfig( + "video_tutorial_iph_dismissed_download", Comparator(EQUAL, 0), 90, 90)); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::NONE; + config->session_rate_impact = session_rate_impact; + + return config; + } + + if (kIPHVideoTutorialNTPSearchFeature.name == feature->name) { + // A config that allows the search video tutorial card to show up + // until explicitly interacted upon. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(GREATER_THAN_OR_EQUAL, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = + EventConfig("tutorial_search_iph_trigger", Comparator(ANY, 0), 90, 90); + config->used = EventConfig("search", Comparator(ANY, 0), 90, 90); + config->event_configs.insert(EventConfig( + "video_tutorial_iph_clicked_search", Comparator(EQUAL, 0), 90, 90)); + config->event_configs.insert(EventConfig( + "video_tutorial_iph_dismissed_search", Comparator(EQUAL, 0), 90, 90)); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::NONE; + config->session_rate_impact = session_rate_impact; + + return config; + } + + if (kIPHVideoTutorialNTPVoiceSearchFeature.name == feature->name) { + // A config that allows the voice search video tutorial card to show up + // until explicitly interacted upon. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(GREATER_THAN_OR_EQUAL, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = EventConfig("tutorial_voice_search_iph_trigger", + Comparator(ANY, 0), 90, 90); + config->used = EventConfig("voice_search", Comparator(ANY, 0), 90, 90); + config->event_configs.insert( + EventConfig("video_tutorial_iph_clicked_voice_search", + Comparator(EQUAL, 0), 90, 90)); + config->event_configs.insert( + EventConfig("video_tutorial_iph_dismissed_voice_search", + Comparator(EQUAL, 0), 90, 90)); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::NONE; + config->session_rate_impact = session_rate_impact; + + return config; + } + + if (kIPHVideoTutorialNTPSummaryFeature.name == feature->name) { + // A config that allows the summary video tutorial card to show up + // until explicitly interacted upon. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(GREATER_THAN_OR_EQUAL, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = + EventConfig("tutorial_summary_iph_trigger", Comparator(ANY, 0), 90, 90); + config->used = EventConfig("summary", Comparator(ANY, 0), 90, 90); + config->event_configs.insert(EventConfig( + "video_tutorial_iph_dismissed_summary", Comparator(EQUAL, 0), 90, 90)); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::NONE; + config->session_rate_impact = session_rate_impact; + + return config; + } + + if (kIPHVideoTutorialTryNowFeature.name == feature->name) { + // A config that allows the video tutorials Try Now button click to result + // in an IPH bubble. This IPH is shown always regardless of session rate or + // any other conditions. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(GREATER_THAN_OR_EQUAL, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = + EventConfig("tutorial_try_now_iph_trigger", Comparator(ANY, 0), 90, 90); + config->used = EventConfig("try_now", Comparator(ANY, 0), 90, 90); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::NONE; + config->session_rate_impact = session_rate_impact; + + return config; + } + if (kIPHStartSurfaceTabSwitcherHomeButton.name == feature->name) { // A config that allows the StartSurfaceTabSwitcherHomeButton IPH to be // shown: @@ -265,6 +423,124 @@ absl::optional<FeatureConfig> GetClientSideFeatureConfig( Comparator(EQUAL, 0), 1, 360)); return config; } + + if (kIPHSharedHighlightingReceiverFeature.name == feature->name) { + // A config that allows the shared highlighting message IPH to be shown + // when a user receives a highlight: + // * Once per 7 days + // * Up to 5 times but only if unused in the last 7 days. + // * Used fewer than 2 times + + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = EventConfig("iph_shared_highlighting_receiver_trigger", + Comparator(LESS_THAN, 5), 360, 360); + config->used = EventConfig("iph_shared_highlighting_button_clicked", + Comparator(LESS_THAN, 2), 360, 360); + config->event_configs.insert( + EventConfig("iph_shared_highlighting_receiver_trigger", + Comparator(EQUAL, 0), 7, 360)); + return config; + } + + if (kIPHSharingHubWebnotesStylizeFeature.name == feature->name) { + // A config that allows the Webnotes Stylize IPH to be shown up to 6 times, + // but only if the feature home hasn't been used in the last 360 days. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = EventConfig("sharing_hub_webnotes_stylize_iph_trigger", + Comparator(LESS_THAN, 6), 360, 360); + config->used = EventConfig("sharing_hub_webnotes_stylize_used", + Comparator(EQUAL, 0), 360, 360); + return config; + } + + if (kIPHAutoDarkOptOutFeature.name == feature->name) { + // A config that allows the auto dark dialog to be shown when a user + // disables the feature for a site in the app menu when: + // * They have not yet opened auto dark settings. + // * The dialog has been shown 0 times before. + // * They have done so at least 3 times. + // TODO(crbug.com/1251737): Update this config from test values; Will + // likely depend on giving feedback instead of opening settings, since the + // primary purpose of the dialog has changed. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + config->used = + EventConfig("auto_dark_settings_opened", Comparator(EQUAL, 0), 90, 90); + config->trigger = EventConfig("auto_dark_opt_out_iph_trigger", + Comparator(EQUAL, 0), 90, 90); + config->event_configs.insert( + EventConfig("auto_dark_disabled_in_app_menu", + Comparator(GREATER_THAN_OR_EQUAL, 3), 90, 90)); + return config; + } + + if (kIPHAutoDarkUserEducationMessageFeature.name == feature->name) { + // A config that allows the auto dark message to be shown: + // * Until the user opens auto dark settings + // * 2 times per week + // * Up to 6 times (3 weeks) + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + config->used = EventConfig("auto_dark_settings_opened", + Comparator(EQUAL, 0), 360, 360); + config->trigger = EventConfig("auto_dark_user_education_message_trigger", + Comparator(LESS_THAN, 2), 7, 360); + config->event_configs.insert( + EventConfig("auto_dark_user_education_message_trigger", + Comparator(LESS_THAN, 6), 360, 360)); + return config; + } + + if (kIPHInstanceSwitcherFeature.name == feature->name) { + // A config that allows the 'Manage windows' text bubble IPH to be shown + // only once when the user starts using the multi-instance feature by + // opening more than one window. + + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = + EventConfig("instance_switcher_iph_trigger", Comparator(LESS_THAN, 1), + k10YearsInDays, k10YearsInDays); + config->used = EventConfig("instance_switcher_used", Comparator(EQUAL, 0), + k10YearsInDays, k10YearsInDays); + return config; + } + + if (kIPHKeyboardAccessoryPaymentVirtualCardFeature.name == feature->name) { + // A config that allows the virtual card IPH to be shown when a user + // interacts with the payment form and triggers the credit card suggestion + // list. + absl::optional<FeatureConfig> config = FeatureConfig(); + config->valid = true; + config->availability = Comparator(ANY, 0); + config->session_rate = Comparator(ANY, 0); + config->trigger = + EventConfig("keyboard_accessory_payment_virtual_card_iph_trigger", + Comparator(LESS_THAN, 3), 90, 360); + config->used = EventConfig("keyboard_accessory_payment_suggestion_accepted", + Comparator(LESS_THAN, 2), 90, 360); + + SessionRateImpact session_rate_impact; + session_rate_impact.type = SessionRateImpact::Type::EXPLICIT; + std::vector<std::string> affected_features; + affected_features.push_back("IPH_KeyboardAccessoryBarSwiping"); + session_rate_impact.affected_features = affected_features; + config->session_rate_impact = session_rate_impact; + return config; + } + #endif // defined(OS_ANDROID) if (kIPHDummyFeature.name == feature->name) { diff --git a/chromium/components/feature_engagement/public/feature_constants.cc b/chromium/components/feature_engagement/public/feature_constants.cc index da68e937039..b6c48a5654a 100644 --- a/chromium/components/feature_engagement/public/feature_constants.cc +++ b/chromium/components/feature_engagement/public/feature_constants.cc @@ -20,6 +20,8 @@ const base::Feature kIPHFocusModeFeature{"IPH_FocusMode", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHGlobalMediaControlsFeature{ "IPH_GlobalMediaControls", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHGMCCastStartStopFeature{ + "IPH_GMCCastStartStop", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHLiveCaptionFeature{"IPH_LiveCaption", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHPasswordsAccountStorageFeature{ @@ -28,8 +30,12 @@ const base::Feature kIPHReadingListDiscoveryFeature{ "IPH_ReadingListDiscovery", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHReadingListEntryPointFeature{ "IPH_ReadingListEntryPoint", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHReadingListInSidePanelFeature{ + "IPH_ReadingListInSidePanel", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHReopenTabFeature{"IPH_ReopenTab", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHSideSearchFeature{"IPH_SideSearch", + base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHTabSearchFeature{"IPH_TabSearch", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHWebUITabStripFeature{"IPH_WebUITabStrip", @@ -37,7 +43,7 @@ const base::Feature kIPHWebUITabStripFeature{"IPH_WebUITabStrip", const base::Feature kIPHDesktopSnoozeFeature{"IPH_DesktopSnoozeFeature", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHDesktopPwaInstallFeature{ - "IPH_DesktopPwaInstall", base::FEATURE_DISABLED_BY_DEFAULT}; + "IPH_DesktopPwaInstall", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHProfileSwitchFeature{"IPH_ProfileSwitch", base::FEATURE_ENABLED_BY_DEFAULT}; #endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) || @@ -58,6 +64,10 @@ const base::Feature kIPHAddToHomescreenMessageFeature{ "IPH_AddToHomescreenMessage", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHAddToHomescreenTextBubbleFeature{ "IPH_AddToHomescreenTextBubble", base::FEATURE_ENABLED_BY_DEFAULT}; +const base::Feature kIPHAutoDarkOptOutFeature{"IPH_AutoDarkOptOut", + base::FEATURE_ENABLED_BY_DEFAULT}; +const base::Feature kIPHAutoDarkUserEducationMessageFeature{ + "IPH_AutoDarkUserEducationMessage", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHDataSaverDetailFeature{ "IPH_DataSaverDetail", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHDataSaverMilestonePromoFeature{ @@ -115,6 +125,8 @@ const base::Feature kIPHHomepagePromoCardFeature{ "IPH_HomepagePromoCard", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHIdentityDiscFeature{"IPH_IdentityDisc", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHInstanceSwitcherFeature{ + "IPH_InstanceSwitcher", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHKeyboardAccessoryAddressFillingFeature{ "IPH_KeyboardAccessoryAddressFilling", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHKeyboardAccessoryBarSwipingFeature{ @@ -127,13 +139,15 @@ const base::Feature kIPHKeyboardAccessoryPaymentOfferFeature{ "IPH_KeyboardAccessoryPaymentOffer", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHKeyboardAccessoryPaymentVirtualCardFeature{ "IPH_KeyboardAccessoryPaymentVirtualCard", - base::FEATURE_DISABLED_BY_DEFAULT}; + base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHNewTabPageHomeButtonFeature{ "IPH_NewTabPageHomeButton", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHMicToolbarFeature{"IPH_MicToolbar", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHPageInfoFeature{"IPH_PageInfo", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHPageInfoStoreInfoFeature{ + "IPH_PageInfoStoreInfo", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHPreviewsOmniboxUIFeature{ "IPH_PreviewsOmniboxUI", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHTabGroupsQuicklyComparePagesFeature{ @@ -149,15 +163,17 @@ const base::Feature kIPHTabSwitcherButtonFeature{ const base::Feature kIPHTranslateMenuButtonFeature{ "IPH_TranslateMenuButton", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHVideoTutorialNTPChromeIntroFeature{ - "IPH_VideoTutorial_NTP_ChromeIntro", base::FEATURE_DISABLED_BY_DEFAULT}; + "IPH_VideoTutorial_NTP_ChromeIntro", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHVideoTutorialNTPDownloadFeature{ - "IPH_VideoTutorial_NTP_Download", base::FEATURE_DISABLED_BY_DEFAULT}; + "IPH_VideoTutorial_NTP_Download", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHVideoTutorialNTPSearchFeature{ - "IPH_VideoTutorial_NTP_Search", base::FEATURE_DISABLED_BY_DEFAULT}; + "IPH_VideoTutorial_NTP_Search", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHVideoTutorialNTPVoiceSearchFeature{ - "IPH_VideoTutorial_NTP_VoiceSearch", base::FEATURE_DISABLED_BY_DEFAULT}; + "IPH_VideoTutorial_NTP_VoiceSearch", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHVideoTutorialNTPSummaryFeature{ - "IPH_VideoTutorial_NTP_Summary", base::FEATURE_DISABLED_BY_DEFAULT}; + "IPH_VideoTutorial_NTP_Summary", base::FEATURE_ENABLED_BY_DEFAULT}; +const base::Feature kIPHVideoTutorialTryNowFeature{ + "IPH_VideoTutorial_TryNow", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHExploreSitesTileFeature{ "IPH_ExploreSitesTile", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHFeedHeaderMenuFeature{"IPH_FeedHeaderMenu", @@ -174,14 +190,20 @@ const base::Feature kIPHPwaInstallAvailableFeature{ "IPH_PwaInstallAvailableFeature", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHShareScreenshotFeature{ "IPH_ShareScreenshot", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHSharingHubLinkToggleFeature{ + "IPH_SharingHubLinkToggle", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHWebFeedFollowFeature{"IPH_WebFeedFollow", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHWebFeedPostFollowDialogFeature{ "IPH_WebFeedPostFollowDialog", base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kIPHSharedHighlightingBuilder{ "IPH_SharedHighlightingBuilder", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kIPHSharedHighlightingReceiverFeature{ + "IPH_SharedHighlightingReceiver", base::FEATURE_DISABLED_BY_DEFAULT}; const base::Feature kIPHStartSurfaceTabSwitcherHomeButton{ "IPH_StartSurfaceTabSwitcherHomeButton", base::FEATURE_ENABLED_BY_DEFAULT}; +const base::Feature kIPHSharingHubWebnotesStylizeFeature{ + "IPH_SharingHubWebnotesStylize", base::FEATURE_ENABLED_BY_DEFAULT}; #endif // defined(OS_ANDROID) #if defined(OS_IOS) diff --git a/chromium/components/feature_engagement/public/feature_constants.h b/chromium/components/feature_engagement/public/feature_constants.h index 7fa4018395b..c31aacc3f4f 100644 --- a/chromium/components/feature_engagement/public/feature_constants.h +++ b/chromium/components/feature_engagement/public/feature_constants.h @@ -24,11 +24,14 @@ extern const base::Feature kIPHDummyFeature; extern const base::Feature kIPHDesktopTabGroupsNewGroupFeature; extern const base::Feature kIPHFocusModeFeature; extern const base::Feature kIPHGlobalMediaControlsFeature; +extern const base::Feature kIPHGMCCastStartStopFeature; extern const base::Feature kIPHLiveCaptionFeature; extern const base::Feature kIPHPasswordsAccountStorageFeature; extern const base::Feature kIPHReadingListDiscoveryFeature; extern const base::Feature kIPHReadingListEntryPointFeature; +extern const base::Feature kIPHReadingListInSidePanelFeature; extern const base::Feature kIPHReopenTabFeature; +extern const base::Feature kIPHSideSearchFeature; extern const base::Feature kIPHTabSearchFeature; extern const base::Feature kIPHWebUITabStripFeature; extern const base::Feature kIPHDesktopSnoozeFeature; @@ -50,6 +53,8 @@ extern const base::Feature kIPHAdaptiveButtonInTopToolbarCustomizationVoiceSearchFeature; extern const base::Feature kIPHAddToHomescreenMessageFeature; extern const base::Feature kIPHAddToHomescreenTextBubbleFeature; +extern const base::Feature kIPHAutoDarkOptOutFeature; +extern const base::Feature kIPHAutoDarkUserEducationMessageFeature; extern const base::Feature kIPHDataSaverDetailFeature; extern const base::Feature kIPHDataSaverMilestonePromoFeature; extern const base::Feature kIPHDataSaverPreviewFeature; @@ -75,6 +80,7 @@ extern const base::Feature kIPHHomePageButtonFeature; extern const base::Feature kIPHHomepageTileFeature; extern const base::Feature kIPHHomepagePromoCardFeature; extern const base::Feature kIPHIdentityDiscFeature; +extern const base::Feature kIPHInstanceSwitcherFeature; extern const base::Feature kIPHKeyboardAccessoryAddressFillingFeature; extern const base::Feature kIPHKeyboardAccessoryBarSwipingFeature; extern const base::Feature kIPHKeyboardAccessoryPasswordFillingFeature; @@ -84,6 +90,7 @@ extern const base::Feature kIPHKeyboardAccessoryPaymentVirtualCardFeature; extern const base::Feature kIPHMicToolbarFeature; extern const base::Feature kIPHNewTabPageHomeButtonFeature; extern const base::Feature kIPHPageInfoFeature; +extern const base::Feature kIPHPageInfoStoreInfoFeature; extern const base::Feature kIPHPreviewsOmniboxUIFeature; extern const base::Feature kIPHQuietNotificationPromptsFeature; extern const base::Feature kIPHReadLaterContextMenuFeature; @@ -101,6 +108,7 @@ extern const base::Feature kIPHVideoTutorialNTPDownloadFeature; extern const base::Feature kIPHVideoTutorialNTPSearchFeature; extern const base::Feature kIPHVideoTutorialNTPVoiceSearchFeature; extern const base::Feature kIPHVideoTutorialNTPSummaryFeature; +extern const base::Feature kIPHVideoTutorialTryNowFeature; extern const base::Feature kIPHExploreSitesTileFeature; extern const base::Feature kIPHFeedHeaderMenuFeature; extern const base::Feature kIPHFeedSwipeRefresh; @@ -109,10 +117,13 @@ extern const base::Feature kIPHChromeReengagementNotification2Feature; extern const base::Feature kIPHChromeReengagementNotification3Feature; extern const base::Feature kIPHPwaInstallAvailableFeature; extern const base::Feature kIPHShareScreenshotFeature; +extern const base::Feature kIPHSharingHubLinkToggleFeature; extern const base::Feature kIPHWebFeedFollowFeature; extern const base::Feature kIPHWebFeedPostFollowDialogFeature; extern const base::Feature kIPHSharedHighlightingBuilder; +extern const base::Feature kIPHSharedHighlightingReceiverFeature; extern const base::Feature kIPHStartSurfaceTabSwitcherHomeButton; +extern const base::Feature kIPHSharingHubWebnotesStylizeFeature; #endif // defined(OS_ANDROID) #if defined(OS_IOS) diff --git a/chromium/components/feature_engagement/public/feature_list.cc b/chromium/components/feature_engagement/public/feature_list.cc index 5df2ed863ef..677ddc38f2f 100644 --- a/chromium/components/feature_engagement/public/feature_list.cc +++ b/chromium/components/feature_engagement/public/feature_list.cc @@ -21,6 +21,8 @@ const base::Feature* const kAllFeatures[] = { &kIPHAdaptiveButtonInTopToolbarCustomizationVoiceSearchFeature, &kIPHAddToHomescreenMessageFeature, &kIPHAddToHomescreenTextBubbleFeature, + &kIPHAutoDarkOptOutFeature, + &kIPHAutoDarkUserEducationMessageFeature, &kIPHDataSaverDetailFeature, &kIPHDataSaverMilestonePromoFeature, &kIPHDataSaverPreviewFeature, @@ -47,6 +49,7 @@ const base::Feature* const kAllFeatures[] = { &kIPHFeedCardMenuFeature, &kIPHHomepagePromoCardFeature, &kIPHIdentityDiscFeature, + &kIPHInstanceSwitcherFeature, &kIPHKeyboardAccessoryAddressFillingFeature, &kIPHKeyboardAccessoryBarSwipingFeature, &kIPHKeyboardAccessoryPasswordFillingFeature, @@ -56,6 +59,7 @@ const base::Feature* const kAllFeatures[] = { &kIPHMicToolbarFeature, &kIPHNewTabPageHomeButtonFeature, &kIPHPageInfoFeature, + &kIPHPageInfoStoreInfoFeature, &kIPHPreviewsOmniboxUIFeature, &kIPHPwaInstallAvailableFeature, &kIPHQuietNotificationPromptsFeature, @@ -74,15 +78,19 @@ const base::Feature* const kAllFeatures[] = { &kIPHVideoTutorialNTPSearchFeature, &kIPHVideoTutorialNTPVoiceSearchFeature, &kIPHVideoTutorialNTPSummaryFeature, + &kIPHVideoTutorialTryNowFeature, &kIPHExploreSitesTileFeature, &kIPHFeedHeaderMenuFeature, &kIPHFeedSwipeRefresh, &kIPHShareScreenshotFeature, + &kIPHSharingHubLinkToggleFeature, &kIPHWebFeedFollowFeature, &kIPHWebFeedPostFollowDialogFeature, &kIPHSharedHighlightingBuilder, + &kIPHSharedHighlightingReceiverFeature, &kIPHStartSurfaceTabSwitcherHomeButton, &kIPHUpdatedConnectionSecurityIndicatorsFeature, + &kIPHSharingHubWebnotesStylizeFeature, #endif // defined(OS_ANDROID) #if defined(OS_IOS) &kIPHBottomToolbarTipFeature, @@ -95,22 +103,25 @@ const base::Feature* const kAllFeatures[] = { &kIPHDiscoverFeedHeaderFeature, #endif // defined(OS_IOS) #if defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) || \ - defined(OS_CHROMEOS) + defined(OS_CHROMEOS) || defined(OS_FUCHSIA) &kIPHDesktopTabGroupsNewGroupFeature, &kIPHFocusModeFeature, &kIPHGlobalMediaControlsFeature, + &kIPHGMCCastStartStopFeature, &kIPHLiveCaptionFeature, &kIPHPasswordsAccountStorageFeature, &kIPHReadingListDiscoveryFeature, &kIPHReadingListEntryPointFeature, + &kIPHReadingListInSidePanelFeature, &kIPHReopenTabFeature, + &kIPHSideSearchFeature, &kIPHTabSearchFeature, &kIPHWebUITabStripFeature, &kIPHDesktopPwaInstallFeature, &kIPHProfileSwitchFeature, &kIPHUpdatedConnectionSecurityIndicatorsFeature, #endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) || - // defined(OS_CHROMEOS) + // defined(OS_CHROMEOS) || defined(OS_FUCHSIA) }; } // namespace diff --git a/chromium/components/feature_engagement/public/feature_list.h b/chromium/components/feature_engagement/public/feature_list.h index 0663795f55f..ab04100904c 100644 --- a/chromium/components/feature_engagement/public/feature_list.h +++ b/chromium/components/feature_engagement/public/feature_list.h @@ -57,6 +57,9 @@ DEFINE_VARIATION_PARAM(kIPHAddToHomescreenMessageFeature, "IPH_AddToHomescreenMessage"); DEFINE_VARIATION_PARAM(kIPHAddToHomescreenTextBubbleFeature, "IPH_AddToHomescreenTextBubble"); +DEFINE_VARIATION_PARAM(kIPHAutoDarkOptOutFeature, "IPH_AutoDarkOptOut"); +DEFINE_VARIATION_PARAM(kIPHAutoDarkUserEducationMessageFeature, + "IPH_AutoDarkUserEducationMessage"); DEFINE_VARIATION_PARAM(kIPHDataSaverDetailFeature, "IPH_DataSaverDetail"); DEFINE_VARIATION_PARAM(kIPHDataSaverMilestonePromoFeature, "IPH_DataSaverMilestonePromo"); @@ -98,6 +101,7 @@ DEFINE_VARIATION_PARAM(kIPHEphemeralTabFeature, "IPH_EphemeralTab"); DEFINE_VARIATION_PARAM(kIPHFeedCardMenuFeature, "IPH_FeedCardMenu"); DEFINE_VARIATION_PARAM(kIPHHomepagePromoCardFeature, "IPH_HomepagePromoCard"); DEFINE_VARIATION_PARAM(kIPHIdentityDiscFeature, "IPH_IdentityDisc"); +DEFINE_VARIATION_PARAM(kIPHInstanceSwitcherFeature, "IPH_InstanceSwitcher"); DEFINE_VARIATION_PARAM(kIPHKeyboardAccessoryAddressFillingFeature, "IPH_KeyboardAccessoryAddressFilling"); DEFINE_VARIATION_PARAM(kIPHKeyboardAccessoryBarSwipingFeature, @@ -113,6 +117,7 @@ DEFINE_VARIATION_PARAM(kIPHKeyboardAccessoryPaymentVirtualCardFeature, DEFINE_VARIATION_PARAM(kIPHMicToolbarFeature, "IPH_MicToolbar"); DEFINE_VARIATION_PARAM(kIPHNewTabPageButtonFeature, "IPH_NewTabPageHomeButton"); DEFINE_VARIATION_PARAM(kIPHPageInfoFeature, "IPH_PageInfo"); +DEFINE_VARIATION_PARAM(kIPHPageInfoStoreInfoFeature, "IPH_PageInfoStoreInfo"); DEFINE_VARIATION_PARAM(kIPHPreviewsOmniboxUIFeature, "IPH_PreviewsOmniboxUI"); DEFINE_VARIATION_PARAM(kIPHPwaInstallAvailableFeature, "IPH_PwaInstallAvailableFeature"); @@ -147,19 +152,27 @@ DEFINE_VARIATION_PARAM(kIPHVideoTutorialNTPVoiceSearchFeature, "IPH_VideoTutorial_NTP_VoiceSearch"); DEFINE_VARIATION_PARAM(kIPHVideoTutorialNTPSummaryFeature, "IPH_VideoTutorial_NTP_Summary"); +DEFINE_VARIATION_PARAM(kIPHVideoTutorialTryNowFeature, + "IPH_VideoTutorial_TryNow"); DEFINE_VARIATION_PARAM(kIPHExploreSitesTileFeature, "IPH_ExploreSitesTile"); DEFINE_VARIATION_PARAM(kIPHFeedHeaderMenuFeature, "IPH_FeedHeaderMenu"); DEFINE_VARIATION_PARAM(kIPHFeedSwipeRefresh, "IPH_FeedSwipeRefresh"); DEFINE_VARIATION_PARAM(kIPHShareScreenshotFeature, "IPH_ShareScreenshot"); +DEFINE_VARIATION_PARAM(kIPHSharingHubLinkToggleFeature, + "IPH_SharingHubLinkToggle"); DEFINE_VARIATION_PARAM(kIPHWebFeedFollowFeature, "IPH_WebFeedFollow"); DEFINE_VARIATION_PARAM(kIPHWebFeedPostFollowDialogFeature, "IPH_WebFeedPostFollowDialog"); DEFINE_VARIATION_PARAM(kIPHSharedHighlightingBuilder, "IPH_SharedHighlightingBuilder"); +DEFINE_VARIATION_PARAM(kIPHSharedHighlightingReceiverFeature, + "IPH_SharedHighlightingReceiver"); DEFINE_VARIATION_PARAM(kIPHStartSurfaceTabSwitcherHomeButton, "IPH_StartSurfaceTabSwitcherHomeButton"); DEFINE_VARIATION_PARAM(kIPHUpdatedConnectionSecurityIndicatorsFeature, "IPH_UpdatedConnectionSecurityIndicators"); +DEFINE_VARIATION_PARAM(kIPHSharingHubWebnotesStylizeFeature, + "IPH_SharingHubWebnotesStylize"); #endif // defined(OS_ANDROID) #if defined(OS_IOS) DEFINE_VARIATION_PARAM(kIPHBottomToolbarTipFeature, "IPH_BottomToolbarTip"); @@ -182,6 +195,7 @@ DEFINE_VARIATION_PARAM(kIPHDesktopTabGroupsNewGroupFeature, "IPH_DesktopTabGroupsNewGroup"); DEFINE_VARIATION_PARAM(kIPHFocusModeFeature, "IPH_FocusMode"); DEFINE_VARIATION_PARAM(kIPHGlobalMediaControls, "IPH_GlobalMediaControls"); +DEFINE_VARIATION_PARAM(kIPHGMCCastStartStopFeature, "IPH_GMCCastStartStop"); DEFINE_VARIATION_PARAM(kIPHLiveCaption, "IPH_LiveCaption"); DEFINE_VARIATION_PARAM(kIPHPasswordsAccountStorageFeature, "IPH_PasswordsAccountStorage"); @@ -189,7 +203,10 @@ DEFINE_VARIATION_PARAM(kIPHReadingListDiscoveryFeature, "IPH_ReadingListDiscovery"); DEFINE_VARIATION_PARAM(kIPHReadingListEntryPointFeature, "IPH_ReadingListEntryPoint"); +DEFINE_VARIATION_PARAM(kIPHReadingListInSidePanelFeature, + "IPH_ReadingListInSidePanel"); DEFINE_VARIATION_PARAM(kIPHReopenTabFeature, "IPH_ReopenTab"); +DEFINE_VARIATION_PARAM(kIPHSideSearchFeature, "IPH_SideSearch"); DEFINE_VARIATION_PARAM(kIPHTabSearchFeature, "IPH_TabSearch"); DEFINE_VARIATION_PARAM(kIPHWebUITabStripFeature, "IPH_WebUITabStrip"); DEFINE_VARIATION_PARAM(kIPHDesktopPwaInstallFeature, "IPH_DesktopPwaInstall"); @@ -215,6 +232,8 @@ constexpr flags_ui::FeatureEntry::FeatureVariation kIPHAdaptiveButtonInTopToolbarCustomizationVoiceSearchFeature), VARIATION_ENTRY(kIPHAddToHomescreenMessageFeature), VARIATION_ENTRY(kIPHAddToHomescreenTextBubbleFeature), + VARIATION_ENTRY(kIPHAutoDarkOptOutFeature), + VARIATION_ENTRY(kIPHAutoDarkUserEducationMessageFeature), VARIATION_ENTRY(kIPHDataSaverDetailFeature), VARIATION_ENTRY(kIPHDataSaverMilestonePromoFeature), VARIATION_ENTRY(kIPHDataSaverPreviewFeature), @@ -241,6 +260,7 @@ constexpr flags_ui::FeatureEntry::FeatureVariation VARIATION_ENTRY(kIPHFeedCardMenuFeature), VARIATION_ENTRY(kIPHHomepagePromoCardFeature), VARIATION_ENTRY(kIPHIdentityDiscFeature), + VARIATION_ENTRY(kIPHInstanceSwitcherFeature), VARIATION_ENTRY(kIPHKeyboardAccessoryAddressFillingFeature), VARIATION_ENTRY(kIPHKeyboardAccessoryBarSwipingFeature), VARIATION_ENTRY(kIPHKeyboardAccessoryPasswordFillingFeature), @@ -250,6 +270,7 @@ constexpr flags_ui::FeatureEntry::FeatureVariation VARIATION_ENTRY(kIPHMicToolbarFeature), VARIATION_ENTRY(kIPHNewTabPageButtonFeature), VARIATION_ENTRY(kIPHPageInfoFeature), + VARIATION_ENTRY(kIPHPageInfoStoreInfoFeature), VARIATION_ENTRY(kIPHPreviewsOmniboxUIFeature), VARIATION_ENTRY(kIPHPwaInstallAvailableFeature), VARIATION_ENTRY(kIPHQuietNotificationPromptsFeature), @@ -268,13 +289,17 @@ constexpr flags_ui::FeatureEntry::FeatureVariation VARIATION_ENTRY(kIPHVideoTutorialNTPSearchFeature), VARIATION_ENTRY(kIPHVideoTutorialNTPVoiceSearchFeature), VARIATION_ENTRY(kIPHVideoTutorialNTPSummaryFeature), + VARIATION_ENTRY(kIPHVideoTutorialTryNowFeature), VARIATION_ENTRY(kIPHExploreSitesTileFeature), VARIATION_ENTRY(kIPHFeedHeaderMenuFeature), VARIATION_ENTRY(kIPHShareScreenshotFeature), + VARIATION_ENTRY(kIPHSharingHubLinkToggleFeature), VARIATION_ENTRY(kIPHWebFeedFollowFeature), VARIATION_ENTRY(kIPHWebFeedPostFollowDialogFeature), VARIATION_ENTRY(kIPHSharedHighlightingBuilder), + VARIATION_ENTRY(kIPHSharedHighlightingReceiverFeature), VARIATION_ENTRY(kIPHUpdatedConnectionSecurityIndicatorsFeature), + VARIATION_ENTRY(kIPHSharingHubWebnotesStylizeFeature), #elif defined(OS_IOS) VARIATION_ENTRY(kIPHBottomToolbarTipFeature), VARIATION_ENTRY(kIPHLongPressToolbarTipFeature), @@ -289,11 +314,14 @@ constexpr flags_ui::FeatureEntry::FeatureVariation VARIATION_ENTRY(kIPHDesktopTabGroupsNewGroupFeature), VARIATION_ENTRY(kIPHFocusModeFeature), VARIATION_ENTRY(kIPHGlobalMediaControls), + VARIATION_ENTRY(kIPHGMCCastStartStopFeature), VARIATION_ENTRY(kIPHLiveCaption), VARIATION_ENTRY(kIPHPasswordsAccountStorageFeature), VARIATION_ENTRY(kIPHReadingListDiscoveryFeature), VARIATION_ENTRY(kIPHReadingListEntryPointFeature), + VARIATION_ENTRY(kIPHReadingListInSidePanelFeature), VARIATION_ENTRY(kIPHReopenTabFeature), + VARIATION_ENTRY(kIPHSideSearchFeature), VARIATION_ENTRY(kIPHTabSearchFeature), VARIATION_ENTRY(kIPHWebUITabStripFeature), VARIATION_ENTRY(kIPHDesktopPwaInstallFeature), diff --git a/chromium/components/feature_engagement/public/tracker.cc b/chromium/components/feature_engagement/public/tracker.cc index 3cabdb8ccbc..aa6fb699a33 100644 --- a/chromium/components/feature_engagement/public/tracker.cc +++ b/chromium/components/feature_engagement/public/tracker.cc @@ -18,4 +18,22 @@ DisplayLockHandle::~DisplayLockHandle() { std::move(release_callback_).Run(); } +Tracker::TriggerDetails::TriggerDetails(bool should_trigger_iph, + bool should_show_snooze) + : should_trigger_iph_(should_trigger_iph), + should_show_snooze_(should_show_snooze) {} + +Tracker::TriggerDetails::TriggerDetails(const TriggerDetails& trigger_details) = + default; + +Tracker::TriggerDetails::~TriggerDetails() = default; + +bool Tracker::TriggerDetails::ShouldShowIph() const { + return should_trigger_iph_; +} + +bool Tracker::TriggerDetails::ShouldShowSnooze() const { + return should_show_snooze_; +} + } // namespace feature_engagement diff --git a/chromium/components/feature_engagement/public/tracker.h b/chromium/components/feature_engagement/public/tracker.h index 3fa02b8d0eb..f081213f68b 100644 --- a/chromium/components/feature_engagement/public/tracker.h +++ b/chromium/components/feature_engagement/public/tracker.h @@ -35,11 +35,14 @@ class DisplayLockHandle { public: typedef base::OnceClosure ReleaseCallback; explicit DisplayLockHandle(ReleaseCallback callback); + + DisplayLockHandle(const DisplayLockHandle&) = delete; + DisplayLockHandle& operator=(const DisplayLockHandle&) = delete; + ~DisplayLockHandle(); private: ReleaseCallback release_callback_; - DISALLOW_COPY_AND_ASSIGN(DisplayLockHandle); }; // The Tracker provides a backend for displaying feature @@ -61,6 +64,36 @@ class Tracker : public KeyedService, public base::SupportsUserData { NOT_READY = 2 }; + // Represents the action taken by the user on the snooze UI. + // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.components.feature_engagement + enum class SnoozeAction : int { + // User chose to snooze the IPH. + SNOOZED = 1, + // User chose to dismiss the IPH. + DISMISSED = 2, + // Constant used by the histogram macros. + kMaxValue = DISMISSED + }; + + // Result of the backend query for whether or not to trigger any help UI. + // A similar class will also be added to the java layer. + struct TriggerDetails { + public: + TriggerDetails(bool should_trigger_iph, bool should_show_snooze); + TriggerDetails(const TriggerDetails& trigger_details); + ~TriggerDetails(); + + // Whether or not to show the help UI. + bool ShouldShowIph() const; + + // Whether to show a snooze option in the help UI. + bool ShouldShowSnooze() const; + + private: + bool should_trigger_iph_; + bool should_show_snooze_; + }; + #if defined(OS_ANDROID) // Returns a Java object of the type Tracker for the given Tracker. static base::android::ScopedJavaLocalRef<jobject> GetJavaObject( @@ -79,6 +112,9 @@ class Tracker : public KeyedService, public base::SupportsUserData { const scoped_refptr<base::SequencedTaskRunner>& background_task_runner, leveldb_proto::ProtoDatabaseProvider* db_provider); + Tracker(const Tracker&) = delete; + Tracker& operator=(const Tracker&) = delete; + // Must be called whenever an event happens. virtual void NotifyEvent(const std::string& event) = 0; @@ -90,6 +126,11 @@ class Tracker : public KeyedService, public base::SupportsUserData { virtual bool ShouldTriggerHelpUI(const base::Feature& feature) WARN_UNUSED_RESULT = 0; + // For callers interested in showing a snooze button. For other callers, use + // the ShouldTriggerHelpUI(..) method. + virtual TriggerDetails ShouldTriggerHelpUIWithSnooze( + const base::Feature& feature) = 0; + // Invoking this is basically the same as being allowed to invoke // ShouldTriggerHelpUI(...) without requiring to show the in-product help. // This function may be called to inspect if the current state would allow the @@ -132,6 +173,12 @@ class Tracker : public KeyedService, public base::SupportsUserData { // particular |feature|. virtual void Dismissed(const base::Feature& feature) = 0; + // For callers interested in showing a snooze button. For other callers, use + // the Dismissed(..) method. + virtual void DismissedWithSnooze( + const base::Feature& feature, + absl::optional<SnoozeAction> snooze_action) = 0; + // Acquiring a display lock means that no in-product help can be displayed // while it is held. To release the lock, delete the handle. // If in-product help is already displayed while the display lock is @@ -160,9 +207,6 @@ class Tracker : public KeyedService, public base::SupportsUserData { protected: Tracker() = default; - - private: - DISALLOW_COPY_AND_ASSIGN(Tracker); }; } // namespace feature_engagement |