Commit 19350367 authored by Keren Zhu's avatar Keren Zhu Committed by Commit Bot

Add snooze to IPH controller

This CL adds snooze to Tab Group IPH. It will not be in effect
until Finch is set up on the server side.

Future CLs:
* Add UMA.
* Add multi-arm Finch experiments to test on/off snooze
  and snooze duration.

Bug: 1121399
Change-Id: I5d7396ad783c6da14fd33ed8d9d78e4b79179c21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398940
Auto-Submit: Keren Zhu <kerenzhu@chromium.org>
Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarCollin Baker <collinbaker@chromium.org>
Reviewed-by: default avatarWei Li <weili@chromium.org>
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807991}
parent 55b85c54
...@@ -11,16 +11,20 @@ ...@@ -11,16 +11,20 @@
#include "base/optional.h" #include "base/optional.h"
#include "chrome/browser/feature_engagement/tracker_factory.h" #include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/in_product_help/feature_promo_snooze_service.h"
#include "chrome/browser/ui/views/chrome_view_class_properties.h" #include "chrome/browser/ui/views/chrome_view_class_properties.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_params.h" #include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_params.h"
#include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_view.h" #include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_view.h"
#include "chrome/browser/ui/views/in_product_help/feature_promo_registry.h" #include "chrome/browser/ui/views/in_product_help/feature_promo_registry.h"
#include "components/feature_engagement/public/feature_constants.h"
#include "components/feature_engagement/public/tracker.h" #include "components/feature_engagement/public/tracker.h"
FeaturePromoControllerViews::FeaturePromoControllerViews( FeaturePromoControllerViews::FeaturePromoControllerViews(
BrowserView* browser_view) BrowserView* browser_view)
: browser_view_(browser_view), : browser_view_(browser_view),
snooze_service_(std::make_unique<FeaturePromoSnoozeService>(
browser_view->browser()->profile())),
tracker_(feature_engagement::TrackerFactory::GetForBrowserContext( tracker_(feature_engagement::TrackerFactory::GetForBrowserContext(
browser_view->browser()->profile())) { browser_view->browser()->profile())) {
DCHECK(tracker_); DCHECK(tracker_);
...@@ -40,6 +44,9 @@ FeaturePromoControllerViews::~FeaturePromoControllerViews() { ...@@ -40,6 +44,9 @@ FeaturePromoControllerViews::~FeaturePromoControllerViews() {
bool FeaturePromoControllerViews::MaybeShowPromoWithParams( bool FeaturePromoControllerViews::MaybeShowPromoWithParams(
const base::Feature& iph_feature, const base::Feature& iph_feature,
const FeaturePromoBubbleParams& params) { const FeaturePromoBubbleParams& params) {
if (snooze_service_->IsBlocked(iph_feature))
return false;
if (!tracker_->ShouldTriggerHelpUI(iph_feature)) if (!tracker_->ShouldTriggerHelpUI(iph_feature))
return false; return false;
...@@ -51,7 +58,12 @@ bool FeaturePromoControllerViews::MaybeShowPromoWithParams( ...@@ -51,7 +58,12 @@ bool FeaturePromoControllerViews::MaybeShowPromoWithParams(
anchor_view_tracker_.SetView(params.anchor_view); anchor_view_tracker_.SetView(params.anchor_view);
current_iph_feature_ = &iph_feature; current_iph_feature_ = &iph_feature;
promo_bubble_ = FeaturePromoBubbleView::Create(std::move(params)); promo_bubble_ = FeaturePromoBubbleView::Create(
std::move(params),
base::BindRepeating(&FeaturePromoControllerViews::OnUserSnooze,
base::Unretained(this), iph_feature),
base::BindRepeating(&FeaturePromoControllerViews::OnUserDismiss,
base::Unretained(this), iph_feature));
widget_observer_.Add(promo_bubble_->GetWidget()); widget_observer_.Add(promo_bubble_->GetWidget());
return true; return true;
...@@ -67,6 +79,16 @@ bool FeaturePromoControllerViews::MaybeShowPromo( ...@@ -67,6 +79,16 @@ bool FeaturePromoControllerViews::MaybeShowPromo(
return MaybeShowPromoWithParams(iph_feature, *params); return MaybeShowPromoWithParams(iph_feature, *params);
} }
void FeaturePromoControllerViews::OnUserSnooze(
const base::Feature& iph_feature) {
snooze_service_->OnUserSnooze(iph_feature);
}
void FeaturePromoControllerViews::OnUserDismiss(
const base::Feature& iph_feature) {
snooze_service_->OnUserDismiss(iph_feature);
}
bool FeaturePromoControllerViews::BubbleIsShowing( bool FeaturePromoControllerViews::BubbleIsShowing(
const base::Feature& iph_feature) const { const base::Feature& iph_feature) const {
return promo_bubble_ && current_iph_feature_ == &iph_feature; return promo_bubble_ && current_iph_feature_ == &iph_feature;
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_CONTROLLER_VIEWS_H_ #ifndef CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_CONTROLLER_VIEWS_H_
#define CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_CONTROLLER_VIEWS_H_ #define CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_CONTROLLER_VIEWS_H_
#include <memory>
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "chrome/browser/ui/views/in_product_help/feature_promo_controller.h" #include "chrome/browser/ui/views/in_product_help/feature_promo_controller.h"
...@@ -15,6 +17,7 @@ ...@@ -15,6 +17,7 @@
class BrowserView; class BrowserView;
class FeaturePromoBubbleView; class FeaturePromoBubbleView;
struct FeaturePromoBubbleParams; struct FeaturePromoBubbleParams;
class FeaturePromoSnoozeService;
namespace base { namespace base {
struct Feature; struct Feature;
...@@ -59,15 +62,27 @@ class FeaturePromoControllerViews : public FeaturePromoController, ...@@ -59,15 +62,27 @@ class FeaturePromoControllerViews : public FeaturePromoController,
return promo_bubble_; return promo_bubble_;
} }
FeaturePromoSnoozeService* snooze_service_for_testing() {
return snooze_service_.get();
}
private: private:
// Called when PromoHandle is destroyed to finish the promo. // Called when PromoHandle is destroyed to finish the promo.
void FinishContinuedPromo() override; void FinishContinuedPromo() override;
void HandleBubbleClosed(); void HandleBubbleClosed();
// Call these methods when the user actively snooze or dismiss the IPH.
void OnUserSnooze(const base::Feature& iph_feature);
void OnUserDismiss(const base::Feature& iph_feature);
// The browser window this instance is responsible for. // The browser window this instance is responsible for.
BrowserView* const browser_view_; BrowserView* const browser_view_;
// Snooze service that is notified when a user snoozes or dismisses the promo.
// Ask this service for display permission before |tracker_|.
std::unique_ptr<FeaturePromoSnoozeService> snooze_service_;
// IPH backend that is notified of user events and decides whether to // IPH backend that is notified of user events and decides whether to
// trigger IPH. // trigger IPH.
feature_engagement::Tracker* const tracker_; feature_engagement::Tracker* const tracker_;
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "chrome/browser/feature_engagement/tracker_factory.h" #include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/in_product_help/feature_promo_snooze_service.h"
#include "chrome/browser/ui/views/chrome_view_class_properties.h" #include "chrome/browser/ui/views/chrome_view_class_properties.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/test_with_browser_view.h" #include "chrome/browser/ui/views/frame/test_with_browser_view.h"
...@@ -116,6 +117,17 @@ TEST_F(FeaturePromoControllerViewsTest, ShowsBubble) { ...@@ -116,6 +117,17 @@ TEST_F(FeaturePromoControllerViewsTest, ShowsBubble) {
EXPECT_TRUE(controller_->promo_bubble_for_testing()); EXPECT_TRUE(controller_->promo_bubble_for_testing());
} }
TEST_F(FeaturePromoControllerViewsTest, SnoozeServiceBlocksPromo) {
EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature)))
.Times(0);
controller_->snooze_service_for_testing()->OnUserDismiss(kTestIPHFeature);
EXPECT_FALSE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
DefaultBubbleParams()));
EXPECT_FALSE(controller_->BubbleIsShowing(kTestIPHFeature));
EXPECT_FALSE(controller_->promo_bubble_for_testing());
controller_->snooze_service_for_testing()->Reset(kTestIPHFeature);
}
TEST_F(FeaturePromoControllerViewsTest, PromoEndsWhenRequested) { TEST_F(FeaturePromoControllerViewsTest, PromoEndsWhenRequested) {
EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature))) EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature)))
.Times(1) .Times(1)
......
...@@ -78,6 +78,11 @@ void FeaturePromoRegistry::RegisterKnownFeatures() { ...@@ -78,6 +78,11 @@ void FeaturePromoRegistry::RegisterKnownFeatures() {
FeaturePromoBubbleParams params; FeaturePromoBubbleParams params;
params.body_string_specifier = IDS_TAB_GROUPS_NEW_GROUP_PROMO; params.body_string_specifier = IDS_TAB_GROUPS_NEW_GROUP_PROMO;
params.arrow = views::BubbleBorder::TOP_LEFT; params.arrow = views::BubbleBorder::TOP_LEFT;
// Turn on IPH Snooze only for Tab Group.
params.allow_snooze = base::FeatureList::IsEnabled(
feature_engagement::kIPHDesktopSnoozeFeature);
RegisterFeature(feature_engagement::kIPHDesktopTabGroupsNewGroupFeature, RegisterFeature(feature_engagement::kIPHDesktopTabGroupsNewGroupFeature,
params, base::BindRepeating(GetTabGroupsAnchorView)); params, base::BindRepeating(GetTabGroupsAnchorView));
} }
......
...@@ -26,6 +26,8 @@ const base::Feature kIPHReopenTabFeature{"IPH_ReopenTab", ...@@ -26,6 +26,8 @@ const base::Feature kIPHReopenTabFeature{"IPH_ReopenTab",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kIPHWebUITabStripFeature{"IPH_WebUITabStrip", const base::Feature kIPHWebUITabStripFeature{"IPH_WebUITabStrip",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kIPHDesktopSnoozeFeature{"IPH_DesktopSnoozeFeature",
base::FEATURE_DISABLED_BY_DEFAULT};
#endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) || #endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) ||
// defined(OS_CHROMEOS) // defined(OS_CHROMEOS)
......
...@@ -24,6 +24,7 @@ extern const base::Feature kIPHGlobalMediaControlsFeature; ...@@ -24,6 +24,7 @@ extern const base::Feature kIPHGlobalMediaControlsFeature;
extern const base::Feature kIPHPasswordsAccountStorageFeature; extern const base::Feature kIPHPasswordsAccountStorageFeature;
extern const base::Feature kIPHReopenTabFeature; extern const base::Feature kIPHReopenTabFeature;
extern const base::Feature kIPHWebUITabStripFeature; extern const base::Feature kIPHWebUITabStripFeature;
extern const base::Feature kIPHDesktopSnoozeFeature;
#endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) || #endif // defined(OS_WIN) || defined(OS_APPLE) || defined(OS_LINUX) ||
// defined(OS_CHROMEOS) // defined(OS_CHROMEOS)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment