[User Education] Move storage service up to c/b/user_education
Storage service should always have been per-profile not per-browser.
Direct access to storage service is needed by IPH session limiting logic
(which is also per-profile) in order to preserve session data across
browser restart in prefs.
This should have no real effect on behavior as the pref data is still
per-profile.
Bug: 1479773
Change-Id: I150c277750e43e644e2a453e3b881ad31a656a65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4977690
Reviewed-by: Dominic Battre <[email protected]>
Reviewed-by: Mickey Burks <[email protected]>
Code-Coverage: [email protected] <[email protected]>
Commit-Queue: Dana Fried <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1215783}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 4cd34e5..e063790 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -1959,6 +1959,8 @@
"privacy_sandbox/tracking_protection_notice_factory.h",
"privacy_sandbox/tracking_protection_notice_service.cc",
"privacy_sandbox/tracking_protection_notice_service.h",
+ "user_education/browser_feature_promo_storage_service.cc",
+ "user_education/browser_feature_promo_storage_service.h",
"user_education/browser_tutorial_service.cc",
"user_education/browser_tutorial_service.h",
"user_education/user_education_configuration_provider.cc",
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index f4cab49..b423ca4 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -85,13 +85,13 @@
#include "chrome/browser/ui/toolbar/chrome_labs_prefs.h"
#include "chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h"
#include "chrome/browser/ui/toolbar/toolbar_pref_names.h"
-#include "chrome/browser/ui/user_education/browser_feature_promo_storage_service.h"
#include "chrome/browser/ui/webui/bookmarks/bookmark_prefs.h"
#include "chrome/browser/ui/webui/flags/flags_ui.h"
#include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
#include "chrome/browser/ui/webui/policy/policy_ui.h"
#include "chrome/browser/ui/webui/print_preview/policy_settings.h"
#include "chrome/browser/updates/announcement_notification/announcement_notification_service.h"
+#include "chrome/browser/user_education/browser_feature_promo_storage_service.h"
#include "chrome/browser/webauthn/chrome_authenticator_request_delegate.h"
#include "chrome/browser/webauthn/webauthn_pref_names.h"
#include "chrome/common/buildflags.h"
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 9e4c43e..70647fc 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -1587,8 +1587,6 @@
"uma_browsing_activity_observer.h",
"unload_controller.cc",
"unload_controller.h",
- "user_education/browser_feature_promo_storage_service.cc",
- "user_education/browser_feature_promo_storage_service.h",
"user_education/scoped_new_badge_tracker.cc",
"user_education/scoped_new_badge_tracker.h",
"user_education/show_promo_in_page.cc",
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 363bd5fd..4078b1d 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -91,7 +91,6 @@
#include "chrome/browser/ui/tabs/tab_utils.h"
#include "chrome/browser/ui/toolbar/app_menu_model.h"
#include "chrome/browser/ui/ui_features.h"
-#include "chrome/browser/ui/user_education/browser_feature_promo_storage_service.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/accelerator_table.h"
#include "chrome/browser/ui/views/accessibility/accessibility_focus_highlight.h"
@@ -902,8 +901,6 @@
MaybeRegisterChromeFeaturePromos(
user_education_service->feature_promo_registry());
MaybeRegisterChromeTutorials(user_education_service->tutorial_registry());
- feature_promo_storage_service_ =
- std::make_unique<BrowserFeaturePromoStorageService>(GetProfile());
feature_promo_controller_ =
std::make_unique<BrowserFeaturePromoController>(
this,
@@ -911,7 +908,7 @@
GetProfile()),
&user_education_service->feature_promo_registry(),
&user_education_service->help_bubble_factory_registry(),
- feature_promo_storage_service_.get(),
+ &user_education_service->feature_promo_storage_service(),
&user_education_service->tutorial_service());
}
}
diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h
index 5f646e8..d6d8db2 100644
--- a/chrome/browser/ui/views/frame/browser_view.h
+++ b/chrome/browser/ui/views/frame/browser_view.h
@@ -28,7 +28,6 @@
#include "chrome/browser/ui/tabs/tab_renderer_data.h"
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
#include "chrome/browser/ui/translate/partial_translate_bubble_model.h"
-#include "chrome/browser/ui/user_education/browser_feature_promo_storage_service.h"
#include "chrome/browser/ui/views/exclusive_access_bubble_views_context.h"
#include "chrome/browser/ui/views/extensions/extension_keybinding_registry_views.h"
#include "chrome/browser/ui/views/frame/browser_frame.h"
@@ -1276,8 +1275,6 @@
std::unique_ptr<AccessibilityFocusHighlight> accessibility_focus_highlight_;
- std::unique_ptr<BrowserFeaturePromoStorageService>
- feature_promo_storage_service_ = nullptr;
std::unique_ptr<BrowserFeaturePromoController> feature_promo_controller_ =
nullptr;
diff --git a/chrome/browser/ui/user_education/browser_feature_promo_storage_service.cc b/chrome/browser/user_education/browser_feature_promo_storage_service.cc
similarity index 98%
rename from chrome/browser/ui/user_education/browser_feature_promo_storage_service.cc
rename to chrome/browser/user_education/browser_feature_promo_storage_service.cc
index f7ae065..f83114d 100644
--- a/chrome/browser/ui/user_education/browser_feature_promo_storage_service.cc
+++ b/chrome/browser/user_education/browser_feature_promo_storage_service.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/browser/ui/user_education/browser_feature_promo_storage_service.h"
+#include "chrome/browser/user_education/browser_feature_promo_storage_service.h"
#include <ostream>
diff --git a/chrome/browser/ui/user_education/browser_feature_promo_storage_service.h b/chrome/browser/user_education/browser_feature_promo_storage_service.h
similarity index 83%
rename from chrome/browser/ui/user_education/browser_feature_promo_storage_service.h
rename to chrome/browser/user_education/browser_feature_promo_storage_service.h
index ee9bae95..a75787a 100644
--- a/chrome/browser/ui/user_education/browser_feature_promo_storage_service.h
+++ b/chrome/browser/user_education/browser_feature_promo_storage_service.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef CHROME_BROWSER_UI_USER_EDUCATION_BROWSER_FEATURE_PROMO_STORAGE_SERVICE_H_
-#define CHROME_BROWSER_UI_USER_EDUCATION_BROWSER_FEATURE_PROMO_STORAGE_SERVICE_H_
+#ifndef CHROME_BROWSER_USER_EDUCATION_BROWSER_FEATURE_PROMO_STORAGE_SERVICE_H_
+#define CHROME_BROWSER_USER_EDUCATION_BROWSER_FEATURE_PROMO_STORAGE_SERVICE_H_
#include "base/memory/raw_ptr.h"
#include "components/user_education/common/feature_promo_storage_service.h"
@@ -35,4 +35,4 @@
const raw_ptr<Profile> profile_;
};
-#endif // CHROME_BROWSER_UI_USER_EDUCATION_BROWSER_FEATURE_PROMO_STORAGE_SERVICE_H_
+#endif // CHROME_BROWSER_USER_EDUCATION_BROWSER_FEATURE_PROMO_STORAGE_SERVICE_H_
diff --git a/chrome/browser/ui/user_education/browser_feature_promo_storage_service_unittest.cc b/chrome/browser/user_education/browser_feature_promo_storage_service_unittest.cc
similarity index 97%
rename from chrome/browser/ui/user_education/browser_feature_promo_storage_service_unittest.cc
rename to chrome/browser/user_education/browser_feature_promo_storage_service_unittest.cc
index 6c72b7d..67fcda84 100644
--- a/chrome/browser/ui/user_education/browser_feature_promo_storage_service_unittest.cc
+++ b/chrome/browser/user_education/browser_feature_promo_storage_service_unittest.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/browser/ui/user_education/browser_feature_promo_storage_service.h"
+#include "chrome/browser/user_education/browser_feature_promo_storage_service.h"
#include <memory>
diff --git a/chrome/browser/user_education/user_education_service.cc b/chrome/browser/user_education/user_education_service.cc
index 3f1116a..0804d5f 100644
--- a/chrome/browser/user_education/user_education_service.cc
+++ b/chrome/browser/user_education/user_education_service.cc
@@ -5,13 +5,16 @@
#include "chrome/browser/user_education/user_education_service.h"
#include <memory>
+#include "components/user_education/common/feature_promo_storage_service.h"
const char kSidePanelCustomizeChromeTutorialId[] =
"Side Panel Customize Chrome Tutorial";
const char kTabGroupTutorialId[] = "Tab Group Tutorial";
const char kPasswordManagerTutorialId[] = "Password Manager Tutorial";
-UserEducationService::UserEducationService()
- : tutorial_service_(&tutorial_registry_, &help_bubble_factory_registry_) {}
+UserEducationService::UserEducationService(
+ std::unique_ptr<user_education::FeaturePromoStorageService> storage_service)
+ : tutorial_service_(&tutorial_registry_, &help_bubble_factory_registry_),
+ feature_promo_storage_service_(std::move(storage_service)) {}
UserEducationService::~UserEducationService() = default;
diff --git a/chrome/browser/user_education/user_education_service.h b/chrome/browser/user_education/user_education_service.h
index bcae242..a23a4ff 100644
--- a/chrome/browser/user_education/user_education_service.h
+++ b/chrome/browser/user_education/user_education_service.h
@@ -5,9 +5,12 @@
#ifndef CHROME_BROWSER_USER_EDUCATION_USER_EDUCATION_SERVICE_H_
#define CHROME_BROWSER_USER_EDUCATION_USER_EDUCATION_SERVICE_H_
+#include <memory>
+
#include "chrome/browser/user_education/browser_tutorial_service.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/user_education/common/feature_promo_registry.h"
+#include "components/user_education/common/feature_promo_storage_service.h"
#include "components/user_education/common/help_bubble_factory_registry.h"
#include "components/user_education/common/product_messaging_controller.h"
#include "components/user_education/common/tutorial.h"
@@ -19,7 +22,9 @@
class UserEducationService : public KeyedService {
public:
- UserEducationService();
+ explicit UserEducationService(
+ std::unique_ptr<user_education::FeaturePromoStorageService>
+ storage_service);
~UserEducationService() override;
user_education::TutorialRegistry& tutorial_registry() {
@@ -37,6 +42,9 @@
user_education::ProductMessagingController& product_messaging_controller() {
return product_messaging_controller_;
}
+ user_education::FeaturePromoStorageService& feature_promo_storage_service() {
+ return *feature_promo_storage_service_;
+ }
private:
user_education::TutorialRegistry tutorial_registry_;
@@ -44,6 +52,8 @@
user_education::FeaturePromoRegistry feature_promo_registry_;
BrowserTutorialService tutorial_service_;
user_education::ProductMessagingController product_messaging_controller_;
+ std::unique_ptr<user_education::FeaturePromoStorageService>
+ feature_promo_storage_service_;
};
#endif // CHROME_BROWSER_UI_USER_EDUCATION_USER_EDUCATION_SERVICE_H_
diff --git a/chrome/browser/user_education/user_education_service_factory.cc b/chrome/browser/user_education/user_education_service_factory.cc
index 3f4a494..4f057359 100644
--- a/chrome/browser/user_education/user_education_service_factory.cc
+++ b/chrome/browser/user_education/user_education_service_factory.cc
@@ -4,6 +4,8 @@
#include "chrome/browser/user_education/user_education_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/user_education/browser_feature_promo_storage_service.h"
#include "chrome/browser/user_education/user_education_service.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -34,5 +36,7 @@
std::unique_ptr<KeyedService>
UserEducationServiceFactory::BuildServiceInstanceForBrowserContext(
content::BrowserContext* context) const {
- return std::make_unique<UserEducationService>();
+ return std::make_unique<UserEducationService>(
+ std::make_unique<BrowserFeaturePromoStorageService>(
+ Profile::FromBrowserContext(context)));
}
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 34887b9..2918bec 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -6368,6 +6368,7 @@
"../browser/ui/safety_hub/menu_notification_unittest.cc",
"../browser/ui/safety_hub/notification_permission_review_service_unittest.cc",
"../browser/ui/safety_hub/safety_hub_service_unittest.cc",
+ "../browser/user_education/browser_feature_promo_storage_service_unittest.cc",
"../browser/user_education/user_education_configuration_provider_unittest.cc",
]
}
@@ -9609,7 +9610,6 @@
"../browser/signin/signin_ui_util_unittest.cc",
"../browser/signin/signin_util_unittest.cc",
"../browser/ui/profiles/profile_customization_synced_theme_waiter_unittest.cc",
- "../browser/ui/user_education/browser_feature_promo_storage_service_unittest.cc",
"../browser/ui/views/download/bubble/download_bubble_contents_view_unittest.cc",
"../browser/ui/views/download/bubble/download_bubble_row_list_view_unittest.cc",
"../browser/ui/views/download/bubble/download_bubble_row_view_unittest.cc",
diff --git a/components/user_education/common/feature_promo_session_policy.cc b/components/user_education/common/feature_promo_session_policy.cc
new file mode 100644
index 0000000..a68b911
--- /dev/null
+++ b/components/user_education/common/feature_promo_session_policy.cc
@@ -0,0 +1,41 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/user_education/common/feature_promo_session_policy.h"
+
+#include "components/user_education/common/feature_promo_result.h"
+#include "components/user_education/common/feature_promo_specification.h"
+
+namespace user_education {
+
+FeaturePromoSessionPolicy::FeaturePromoSessionPolicy() = default;
+FeaturePromoSessionPolicy::~FeaturePromoSessionPolicy() = default;
+
+FeaturePromoSessionPolicyV1::FeaturePromoSessionPolicyV1() = default;
+FeaturePromoSessionPolicyV1::~FeaturePromoSessionPolicyV1() = default;
+
+FeaturePromoResult FeaturePromoSessionPolicyV1::CanShowPromo(
+ const FeaturePromoSpecification& promo_specification) const {
+ return FeaturePromoResult::Success();
+}
+
+void FeaturePromoSessionPolicyV1::OnPromoShown(
+ const FeaturePromoSpecification& promo_specification) {
+ // No-op.
+}
+
+FeaturePromoSessionPolicyV2::FeaturePromoSessionPolicyV2() = default;
+FeaturePromoSessionPolicyV2::~FeaturePromoSessionPolicyV2() = default;
+
+FeaturePromoResult FeaturePromoSessionPolicyV2::CanShowPromo(
+ const FeaturePromoSpecification& promo_specification) const {
+ return FeaturePromoResult::Success();
+}
+
+void FeaturePromoSessionPolicyV2::OnPromoShown(
+ const FeaturePromoSpecification& promo_specification) {
+ // No-op.
+}
+
+} // namespace user_education
diff --git a/components/user_education/common/feature_promo_session_policy.h b/components/user_education/common/feature_promo_session_policy.h
new file mode 100644
index 0000000..d3a63e3c
--- /dev/null
+++ b/components/user_education/common/feature_promo_session_policy.h
@@ -0,0 +1,85 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_USER_EDUCATION_COMMON_FEATURE_PROMO_SESSION_POLICY_H_
+#define COMPONENTS_USER_EDUCATION_COMMON_FEATURE_PROMO_SESSION_POLICY_H_
+
+#include "base/memory/raw_ptr.h"
+#include "components/user_education/common/feature_promo_result.h"
+#include "components/user_education/common/feature_promo_session_manager.h"
+#include "components/user_education/common/feature_promo_specification.h"
+#include "components/user_education/common/feature_promo_storage_service.h"
+
+namespace user_education {
+
+// Describes how IPH interact with each other inside the same session.
+class FeaturePromoSessionPolicy {
+ public:
+ FeaturePromoSessionPolicy();
+ virtual ~FeaturePromoSessionPolicy();
+ FeaturePromoSessionPolicy(const FeaturePromoSessionPolicy&) = delete;
+ void operator=(const FeaturePromoSessionPolicy&) = delete;
+
+ // Determines whether a promo can be displayed based on the current session
+ // state. Returns FeaturePromoResult::Success() if the promo is not blocked
+ // by session policy, or a specific failure type if it is blocked.
+ virtual FeaturePromoResult CanShowPromo(
+ const FeaturePromoSpecification& promo_specification) const = 0;
+
+ // Notifies the policy that a promo with `promo_specification` has shown.
+ // Implementations should update their internal state if this would affect
+ // subsequent IPH.
+ virtual void OnPromoShown(const FeaturePromoSpecification& promo_specification) = 0;
+
+ void set_session_manager(const FeaturePromoSessionManager* session_manager) {
+ session_manager_ = session_manager;
+ }
+ const FeaturePromoSessionManager* session_manager() const { return session_manager_; }
+
+ private:
+ raw_ptr<const FeaturePromoSessionManager> session_manager_ = nullptr;
+};
+
+// Implements the legacy session policy, which is to never block a promo due to
+// session state.
+class FeaturePromoSessionPolicyV1 : public FeaturePromoSessionPolicy {
+ public:
+ FeaturePromoSessionPolicyV1();
+ ~FeaturePromoSessionPolicyV1() override;
+
+ // FeaturePromoSessionPolicy:
+ FeaturePromoResult CanShowPromo(
+ const FeaturePromoSpecification& promo_specification) const override;
+ void OnPromoShown(const FeaturePromoSpecification& promo_specification) override;
+};
+
+// Implements the User Education Experience v2.0 session policy, which is to
+// block heavyweight, low-priority promos during the session start grace period
+// as well as during a several-day cooldown after another heavyweight promo.
+class FeaturePromoSessionPolicyV2 : public FeaturePromoSessionPolicy {
+ public:
+ FeaturePromoSessionPolicyV2();
+ ~FeaturePromoSessionPolicyV2() override;
+
+ // FeaturePromoSessionPolicy:
+ FeaturePromoResult CanShowPromo(
+ const FeaturePromoSpecification& promo_specification) const override;
+ void OnPromoShown(const FeaturePromoSpecification& promo_specification) override;
+
+ void set_storage_service(FeaturePromoStorageService* storage_service) {
+ storage_service_ = storage_service;
+ }
+ FeaturePromoStorageService* storage_service() { return storage_service_; }
+
+ protected:
+ FeaturePromoSessionPolicyV2(base::)
+
+ private:
+ raw_ptr<FeaturePromoStorageService> storage_service_ = nullptr;
+};
+
+
+} // namespace user_education
+
+#endif // COMPONENTS_USER_EDUCATION_COMMON_FEATURE_PROMO_SESSION_POLICY_H_