Commit bc92e9d2 authored by Collin Baker's avatar Collin Baker Committed by Commit Bot

Add global registry for Views-based in-product help

Bug: 1106523
Change-Id: I1876b84f2d24a4a664a9aaf5f19eb126d3543e3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360503
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: default avatarDana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800802}
parent 6283cb25
...@@ -3525,6 +3525,8 @@ static_library("ui") { ...@@ -3525,6 +3525,8 @@ static_library("ui") {
"views/in_product_help/feature_promo_controller.h", "views/in_product_help/feature_promo_controller.h",
"views/in_product_help/feature_promo_controller_views.cc", "views/in_product_help/feature_promo_controller_views.cc",
"views/in_product_help/feature_promo_controller_views.h", "views/in_product_help/feature_promo_controller_views.h",
"views/in_product_help/feature_promo_registry.cc",
"views/in_product_help/feature_promo_registry.h",
"views/in_product_help/global_media_controls_promo_controller.cc", "views/in_product_help/global_media_controls_promo_controller.cc",
"views/in_product_help/global_media_controls_promo_controller.h", "views/in_product_help/global_media_controls_promo_controller.h",
"views/in_product_help/reopen_tab_promo_controller.cc", "views/in_product_help/reopen_tab_promo_controller.cc",
......
...@@ -557,7 +557,7 @@ BrowserView::BrowserView(std::unique_ptr<Browser> browser) ...@@ -557,7 +557,7 @@ BrowserView::BrowserView(std::unique_ptr<Browser> browser)
std::make_unique<TabStripRegionView>(std::move(tabstrip))); std::make_unique<TabStripRegionView>(std::move(tabstrip)));
feature_promo_controller_ = feature_promo_controller_ =
std::make_unique<FeaturePromoControllerViews>(browser_->profile()); std::make_unique<FeaturePromoControllerViews>(this);
// Must be destroyed before the tab strip and |feature_promo_controller_|. // Must be destroyed before the tab strip and |feature_promo_controller_|.
tab_groups_iph_controller_ = std::make_unique<TabGroupsIPHController>( tab_groups_iph_controller_ = std::make_unique<TabGroupsIPHController>(
......
...@@ -494,8 +494,8 @@ class WebUITabStripContainerView::IPHController : public TabStripModelObserver { ...@@ -494,8 +494,8 @@ class WebUITabStripContainerView::IPHController : public TabStripModelObserver {
bubble_params.body_string_specifier = IDS_WEBUI_TAB_STRIP_PROMO; bubble_params.body_string_specifier = IDS_WEBUI_TAB_STRIP_PROMO;
bubble_params.anchor_view = anchor_view; bubble_params.anchor_view = anchor_view;
bubble_params.arrow = views::BubbleBorder::TOP_RIGHT; bubble_params.arrow = views::BubbleBorder::TOP_RIGHT;
promo_controller_->MaybeShowPromo( promo_controller_->MaybeShowPromoWithParams(
feature_engagement::kIPHWebUITabStripFeature, std::move(bubble_params)); feature_engagement::kIPHWebUITabStripFeature, bubble_params);
} }
private: private:
......
...@@ -7,8 +7,6 @@ ...@@ -7,8 +7,6 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
struct FeaturePromoBubbleParams;
namespace base { namespace base {
struct Feature; struct Feature;
} }
...@@ -22,10 +20,10 @@ class FeaturePromoController { ...@@ -22,10 +20,10 @@ class FeaturePromoController {
// Starts the promo if possible. Returns whether it started. // Starts the promo if possible. Returns whether it started.
// |iph_feature| must be an IPH feature defined in // |iph_feature| must be an IPH feature defined in
// components/feature_engagement/public/feature_list.cc. Note that // components/feature_engagement/public/feature_list.cc and registered
// this is different than the feature that the IPH is for. // with |FeaturePromoRegistry|. Note that this is different than the
virtual bool MaybeShowPromo(const base::Feature& iph_feature, // feature that the IPH is showing for.
FeaturePromoBubbleParams params) = 0; virtual bool MaybeShowPromo(const base::Feature& iph_feature) = 0;
// Returns whether a bubble is showing for the given IPH. Note that if // Returns whether a bubble is showing for the given IPH. Note that if
// this is false, a promo might still be in progress; for example, a // this is false, a promo might still be in progress; for example, a
......
...@@ -8,16 +8,21 @@ ...@@ -8,16 +8,21 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.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/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/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 "components/feature_engagement/public/tracker.h" #include "components/feature_engagement/public/tracker.h"
FeaturePromoControllerViews::FeaturePromoControllerViews(Profile* profile) FeaturePromoControllerViews::FeaturePromoControllerViews(
: tracker_( BrowserView* browser_view)
feature_engagement::TrackerFactory::GetForBrowserContext(profile)) { : browser_view_(browser_view),
tracker_(feature_engagement::TrackerFactory::GetForBrowserContext(
browser_view->browser()->profile())) {
DCHECK(tracker_); DCHECK(tracker_);
} }
...@@ -32,9 +37,9 @@ FeaturePromoControllerViews::~FeaturePromoControllerViews() { ...@@ -32,9 +37,9 @@ FeaturePromoControllerViews::~FeaturePromoControllerViews() {
promo_bubble_->GetWidget()->Close(); promo_bubble_->GetWidget()->Close();
} }
bool FeaturePromoControllerViews::MaybeShowPromo( bool FeaturePromoControllerViews::MaybeShowPromoWithParams(
const base::Feature& iph_feature, const base::Feature& iph_feature,
FeaturePromoBubbleParams params) { const FeaturePromoBubbleParams& params) {
if (!tracker_->ShouldTriggerHelpUI(iph_feature)) if (!tracker_->ShouldTriggerHelpUI(iph_feature))
return false; return false;
...@@ -52,6 +57,16 @@ bool FeaturePromoControllerViews::MaybeShowPromo( ...@@ -52,6 +57,16 @@ bool FeaturePromoControllerViews::MaybeShowPromo(
return true; return true;
} }
bool FeaturePromoControllerViews::MaybeShowPromo(
const base::Feature& iph_feature) {
base::Optional<FeaturePromoBubbleParams> params =
FeaturePromoRegistry::GetInstance()->GetParamsForFeature(iph_feature,
browser_view_);
if (!params)
return false;
return MaybeShowPromoWithParams(iph_feature, *params);
}
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;
......
...@@ -12,9 +12,9 @@ ...@@ -12,9 +12,9 @@
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_observer.h" #include "ui/views/widget/widget_observer.h"
class BrowserView;
class FeaturePromoBubbleView; class FeaturePromoBubbleView;
struct FeaturePromoBubbleParams; struct FeaturePromoBubbleParams;
class Profile;
namespace base { namespace base {
struct Feature; struct Feature;
...@@ -24,11 +24,13 @@ namespace feature_engagement { ...@@ -24,11 +24,13 @@ namespace feature_engagement {
class Tracker; class Tracker;
} }
// Views implementation of FeaturePromoController. // Views implementation of FeaturePromoController. There is one instance
// per window.
class FeaturePromoControllerViews : public FeaturePromoController, class FeaturePromoControllerViews : public FeaturePromoController,
public views::WidgetObserver { public views::WidgetObserver {
public: public:
explicit FeaturePromoControllerViews(Profile* profile); // Create the instance for the given |browser_view|.
explicit FeaturePromoControllerViews(BrowserView* browser_view);
~FeaturePromoControllerViews() override; ~FeaturePromoControllerViews() override;
// Repositions the bubble (if showing) relative to the anchor view. // Repositions the bubble (if showing) relative to the anchor view.
...@@ -36,9 +38,13 @@ class FeaturePromoControllerViews : public FeaturePromoController, ...@@ -36,9 +38,13 @@ class FeaturePromoControllerViews : public FeaturePromoController,
// moved. It is safe to call this if a bubble is not showing. // moved. It is safe to call this if a bubble is not showing.
void UpdateBubbleForAnchorBoundsChange(); void UpdateBubbleForAnchorBoundsChange();
// For IPH not registered with |FeaturePromoRegistry|. Only use this
// if it is infeasible to pre-register your IPH.
bool MaybeShowPromoWithParams(const base::Feature& iph_feature,
const FeaturePromoBubbleParams& params);
// FeaturePromoController: // FeaturePromoController:
bool MaybeShowPromo(const base::Feature& iph_feature, bool MaybeShowPromo(const base::Feature& iph_feature) override;
FeaturePromoBubbleParams params) override;
bool BubbleIsShowing(const base::Feature& iph_feature) const override; bool BubbleIsShowing(const base::Feature& iph_feature) const override;
void CloseBubble(const base::Feature& iph_feature) override; void CloseBubble(const base::Feature& iph_feature) override;
PromoHandle CloseBubbleAndContinuePromo( PromoHandle CloseBubbleAndContinuePromo(
...@@ -59,6 +65,9 @@ class FeaturePromoControllerViews : public FeaturePromoController, ...@@ -59,6 +65,9 @@ class FeaturePromoControllerViews : public FeaturePromoController,
void HandleBubbleClosed(); void HandleBubbleClosed();
// The browser window this instance is responsible for.
BrowserView* const browser_view_;
// 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_;
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/optional.h" #include "base/optional.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/views/chrome_view_class_properties.h" #include "chrome/browser/ui/views/chrome_view_class_properties.h"
...@@ -14,6 +15,7 @@ ...@@ -14,6 +15,7 @@
#include "chrome/browser/ui/views/frame/test_with_browser_view.h" #include "chrome/browser/ui/views/frame/test_with_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/toolbar/browser_app_menu_button.h" #include "chrome/browser/ui/views/toolbar/browser_app_menu_button.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -44,6 +46,13 @@ class FeaturePromoControllerViewsTest : public TestWithBrowserView { ...@@ -44,6 +46,13 @@ class FeaturePromoControllerViewsTest : public TestWithBrowserView {
static_cast<NiceMock<feature_engagement::test::MockTracker>*>( static_cast<NiceMock<feature_engagement::test::MockTracker>*>(
feature_engagement::TrackerFactory::GetForBrowserContext( feature_engagement::TrackerFactory::GetForBrowserContext(
profile())); profile()));
FeaturePromoRegistry::GetInstance()->ClearFeaturesForTesting();
}
void TearDown() override {
FeaturePromoRegistry::GetInstance()->ReinitializeForTesting();
TestWithBrowserView::TearDown();
} }
TestingProfile::TestingFactories GetTestingFactories() override { TestingProfile::TestingFactories GetTestingFactories() override {
...@@ -91,8 +100,8 @@ TEST_F(FeaturePromoControllerViewsTest, AsksBackendToShowPromo) { ...@@ -91,8 +100,8 @@ TEST_F(FeaturePromoControllerViewsTest, AsksBackendToShowPromo) {
EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature))) EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature)))
.Times(1) .Times(1)
.WillOnce(Return(false)); .WillOnce(Return(false));
EXPECT_FALSE( EXPECT_FALSE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
controller_->MaybeShowPromo(kTestIPHFeature, DefaultBubbleParams())); DefaultBubbleParams()));
EXPECT_FALSE(controller_->BubbleIsShowing(kTestIPHFeature)); EXPECT_FALSE(controller_->BubbleIsShowing(kTestIPHFeature));
EXPECT_FALSE(controller_->promo_bubble_for_testing()); EXPECT_FALSE(controller_->promo_bubble_for_testing());
} }
...@@ -101,8 +110,8 @@ TEST_F(FeaturePromoControllerViewsTest, ShowsBubble) { ...@@ -101,8 +110,8 @@ TEST_F(FeaturePromoControllerViewsTest, ShowsBubble) {
EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature))) EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature)))
.Times(1) .Times(1)
.WillOnce(Return(true)); .WillOnce(Return(true));
EXPECT_TRUE( EXPECT_TRUE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
controller_->MaybeShowPromo(kTestIPHFeature, DefaultBubbleParams())); DefaultBubbleParams()));
EXPECT_TRUE(controller_->BubbleIsShowing(kTestIPHFeature)); EXPECT_TRUE(controller_->BubbleIsShowing(kTestIPHFeature));
EXPECT_TRUE(controller_->promo_bubble_for_testing()); EXPECT_TRUE(controller_->promo_bubble_for_testing());
} }
...@@ -112,8 +121,8 @@ TEST_F(FeaturePromoControllerViewsTest, PromoEndsWhenRequested) { ...@@ -112,8 +121,8 @@ TEST_F(FeaturePromoControllerViewsTest, PromoEndsWhenRequested) {
.Times(1) .Times(1)
.WillOnce(Return(true)); .WillOnce(Return(true));
EXPECT_CALL(*mock_tracker_, Dismissed(Ref(kTestIPHFeature))).Times(0); EXPECT_CALL(*mock_tracker_, Dismissed(Ref(kTestIPHFeature))).Times(0);
ASSERT_TRUE( ASSERT_TRUE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
controller_->MaybeShowPromo(kTestIPHFeature, DefaultBubbleParams())); DefaultBubbleParams()));
// Only valid before the widget is closed. // Only valid before the widget is closed.
FeaturePromoBubbleView* const bubble = FeaturePromoBubbleView* const bubble =
...@@ -137,8 +146,8 @@ TEST_F(FeaturePromoControllerViewsTest, PromoEndsOnBubbleClosure) { ...@@ -137,8 +146,8 @@ TEST_F(FeaturePromoControllerViewsTest, PromoEndsOnBubbleClosure) {
.Times(1) .Times(1)
.WillOnce(Return(true)); .WillOnce(Return(true));
EXPECT_CALL(*mock_tracker_, Dismissed(Ref(kTestIPHFeature))).Times(0); EXPECT_CALL(*mock_tracker_, Dismissed(Ref(kTestIPHFeature))).Times(0);
ASSERT_TRUE( ASSERT_TRUE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
controller_->MaybeShowPromo(kTestIPHFeature, DefaultBubbleParams())); DefaultBubbleParams()));
// Only valid before the widget is closed. // Only valid before the widget is closed.
FeaturePromoBubbleView* const bubble = FeaturePromoBubbleView* const bubble =
...@@ -161,8 +170,8 @@ TEST_F(FeaturePromoControllerViewsTest, ContinuedPromoDefersBackendDismissed) { ...@@ -161,8 +170,8 @@ TEST_F(FeaturePromoControllerViewsTest, ContinuedPromoDefersBackendDismissed) {
.Times(1) .Times(1)
.WillOnce(Return(true)); .WillOnce(Return(true));
EXPECT_CALL(*mock_tracker_, Dismissed(Ref(kTestIPHFeature))).Times(0); EXPECT_CALL(*mock_tracker_, Dismissed(Ref(kTestIPHFeature))).Times(0);
ASSERT_TRUE( ASSERT_TRUE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
controller_->MaybeShowPromo(kTestIPHFeature, DefaultBubbleParams())); DefaultBubbleParams()));
// Only valid before the widget is closed. // Only valid before the widget is closed.
FeaturePromoBubbleView* const bubble = FeaturePromoBubbleView* const bubble =
...@@ -197,10 +206,29 @@ TEST_F(FeaturePromoControllerViewsTest, ...@@ -197,10 +206,29 @@ TEST_F(FeaturePromoControllerViewsTest,
EXPECT_FALSE(GetAnchorView()->GetProperty(kHasInProductHelpPromoKey)); EXPECT_FALSE(GetAnchorView()->GetProperty(kHasInProductHelpPromoKey));
ASSERT_TRUE( ASSERT_TRUE(controller_->MaybeShowPromoWithParams(kTestIPHFeature,
controller_->MaybeShowPromo(kTestIPHFeature, DefaultBubbleParams())); DefaultBubbleParams()));
EXPECT_TRUE(GetAnchorView()->GetProperty(kHasInProductHelpPromoKey)); EXPECT_TRUE(GetAnchorView()->GetProperty(kHasInProductHelpPromoKey));
controller_->CloseBubble(kTestIPHFeature); controller_->CloseBubble(kTestIPHFeature);
EXPECT_FALSE(GetAnchorView()->GetProperty(kHasInProductHelpPromoKey)); EXPECT_FALSE(GetAnchorView()->GetProperty(kHasInProductHelpPromoKey));
} }
TEST_F(FeaturePromoControllerViewsTest, GetsParamsFromRegistry) {
FeaturePromoBubbleParams params = DefaultBubbleParams();
params.anchor_view = nullptr;
FeaturePromoRegistry::GetInstance()->RegisterFeature(
kTestIPHFeature, DefaultBubbleParams(),
base::BindRepeating([](BrowserView* browser_view) {
return static_cast<views::View*>(
browser_view->toolbar()->app_menu_button());
}));
EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(Ref(kTestIPHFeature)))
.Times(1)
.WillOnce(Return(true));
ASSERT_TRUE(controller_->MaybeShowPromo(kTestIPHFeature));
EXPECT_EQ(browser_view()->toolbar()->app_menu_button(),
controller_->promo_bubble_for_testing()->GetAnchorView());
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/ui/views/in_product_help/feature_promo_registry.h"
#include "base/no_destructor.h"
#include "base/optional.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/tabs/tab_strip.h"
#include "chrome/grit/generated_resources.h"
#include "components/feature_engagement/public/feature_constants.h"
namespace {
// Functions to get an anchor view for an IPH should go here.
// kIPHDesktopTabGroupsNewGroupFeature:
views::View* GetTabGroupsAnchorView(BrowserView* browser_view) {
constexpr int kPreferredAnchorTab = 2;
return browser_view->tabstrip()->GetTabViewForPromoAnchor(
kPreferredAnchorTab);
}
} // namespace
FeaturePromoRegistry::FeaturePromoRegistry() {
RegisterKnownFeatures();
}
FeaturePromoRegistry::~FeaturePromoRegistry() = default;
// static
FeaturePromoRegistry* FeaturePromoRegistry::GetInstance() {
static base::NoDestructor<FeaturePromoRegistry> instance;
return instance.get();
}
base::Optional<FeaturePromoBubbleParams>
FeaturePromoRegistry::GetParamsForFeature(const base::Feature& iph_feature,
BrowserView* browser_view) {
auto data_it = feature_promo_data_.find(&iph_feature);
DCHECK(data_it != feature_promo_data_.end());
views::View* const anchor_view =
data_it->second.get_anchor_view_callback.Run(browser_view);
if (!anchor_view)
return base::nullopt;
FeaturePromoBubbleParams params = data_it->second.params;
params.anchor_view = anchor_view;
return params;
}
void FeaturePromoRegistry::RegisterFeature(
const base::Feature& iph_feature,
const FeaturePromoBubbleParams& params,
GetAnchorViewCallback get_anchor_view_callback) {
FeaturePromoData data;
data.params = params;
data.get_anchor_view_callback = std::move(get_anchor_view_callback);
feature_promo_data_.emplace(&iph_feature, std::move(data));
}
void FeaturePromoRegistry::ClearFeaturesForTesting() {
feature_promo_data_.clear();
}
void FeaturePromoRegistry::ReinitializeForTesting() {
ClearFeaturesForTesting();
RegisterKnownFeatures();
}
void FeaturePromoRegistry::RegisterKnownFeatures() {
{
// kIPHDesktopTabGroupsNewGroupFeature:
FeaturePromoBubbleParams params;
params.body_string_specifier = IDS_TAB_GROUPS_NEW_GROUP_PROMO;
params.arrow = views::BubbleBorder::TOP_LEFT;
RegisterFeature(feature_engagement::kIPHDesktopTabGroupsNewGroupFeature,
params, base::BindRepeating(GetTabGroupsAnchorView));
}
}
FeaturePromoRegistry::FeaturePromoData::FeaturePromoData() = default;
FeaturePromoRegistry::FeaturePromoData::FeaturePromoData(FeaturePromoData&&) =
default;
FeaturePromoRegistry::FeaturePromoData::~FeaturePromoData() = default;
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_REGISTRY_H_
#define CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_REGISTRY_H_
#include <map>
#include "base/callback.h"
#include "base/optional.h"
#include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_params.h"
class BrowserView;
namespace base {
struct Feature;
}
namespace views {
class View;
}
// Stores parameters for in-product help promos. For each registered
// IPH, has the bubble parameters and a method for getting an anchor
// view for a given BrowserView. Promos should be registered here when
// feasible.
class FeaturePromoRegistry {
public:
using GetAnchorViewCallback =
base::RepeatingCallback<views::View*(BrowserView*)>;
FeaturePromoRegistry();
~FeaturePromoRegistry();
static FeaturePromoRegistry* GetInstance();
// Returns a complete FeaturePromoBubbleParams object to start IPH for
// the given feature. |iph_feature| is the feature to show for.
// |browser_view| is the window it should show in.
//
// The params must be used immediately since it contains a View
// pointer that may become stale. This may return nothing in which
// case the promo shouldn't show.
base::Optional<FeaturePromoBubbleParams> GetParamsForFeature(
const base::Feature& iph_feature,
BrowserView* browser_view);
// Registers a feature promo. |iph_feature| is the feature. |params|
// are normal bubble params except the anchor_view member should be
// null. |get_anchor_view_callback| specifies how to get the bubble's
// anchor view for an arbitrary browser window.
//
// Prefer putting these calls in the body of RegisterKnownFeatures()
// when possible.
void RegisterFeature(const base::Feature& iph_feature,
const FeaturePromoBubbleParams& params,
GetAnchorViewCallback get_anchor_view_callback);
void ClearFeaturesForTesting();
void ReinitializeForTesting();
private:
// To avoid sprinkling RegisterFeature() calls throughout the Top
// Chrome codebase, you can put your call in here.
void RegisterKnownFeatures();
struct FeaturePromoData {
FeaturePromoData();
FeaturePromoData(FeaturePromoData&&);
~FeaturePromoData();
// The params for a promo, minus the anchor view.
FeaturePromoBubbleParams params;
GetAnchorViewCallback get_anchor_view_callback;
};
std::map<const base::Feature*, FeaturePromoData> feature_promo_data_;
};
#endif // CHROME_BROWSER_UI_VIEWS_IN_PRODUCT_HELP_FEATURE_PROMO_REGISTRY_H_
...@@ -16,10 +16,7 @@ class MockFeaturePromoController : public FeaturePromoController { ...@@ -16,10 +16,7 @@ class MockFeaturePromoController : public FeaturePromoController {
~MockFeaturePromoController() override; ~MockFeaturePromoController() override;
// FeaturePromoController: // FeaturePromoController:
MOCK_METHOD(bool, MOCK_METHOD(bool, MaybeShowPromo, (const base::Feature&), (override));
MaybeShowPromo,
(const base::Feature&, FeaturePromoBubbleParams),
(override));
MOCK_METHOD(bool, BubbleIsShowing, (const base::Feature&), (const, override)); MOCK_METHOD(bool, BubbleIsShowing, (const base::Feature&), (const, override));
MOCK_METHOD(void, CloseBubble, (const base::Feature&), (override)); MOCK_METHOD(void, CloseBubble, (const base::Feature&), (override));
MOCK_METHOD(PromoHandle, MOCK_METHOD(PromoHandle,
......
...@@ -22,13 +22,6 @@ ...@@ -22,13 +22,6 @@
#include "ui/views/bubble/bubble_border.h" #include "ui/views/bubble/bubble_border.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
namespace {
// The index of the tab we'd like to anchor our bubble to.
constexpr int kPreferredAnchorTab = 2;
} // namespace
TabGroupsIPHController::TabGroupsIPHController( TabGroupsIPHController::TabGroupsIPHController(
Browser* browser, Browser* browser,
FeaturePromoController* promo_controller, FeaturePromoController* promo_controller,
...@@ -81,13 +74,8 @@ void TabGroupsIPHController::OnTabStripModelChanged( ...@@ -81,13 +74,8 @@ void TabGroupsIPHController::OnTabStripModelChanged(
tracker_->NotifyEvent(feature_engagement::events::kSixthTabOpened); tracker_->NotifyEvent(feature_engagement::events::kSixthTabOpened);
FeaturePromoBubbleParams bubble_params;
bubble_params.body_string_specifier = IDS_TAB_GROUPS_NEW_GROUP_PROMO;
bubble_params.anchor_view = get_tab_view_.Run(kPreferredAnchorTab);
bubble_params.arrow = views::BubbleBorder::TOP_LEFT;
promo_controller_->MaybeShowPromo( promo_controller_->MaybeShowPromo(
feature_engagement::kIPHDesktopTabGroupsNewGroupFeature, feature_engagement::kIPHDesktopTabGroupsNewGroupFeature);
std::move(bubble_params));
} }
void TabGroupsIPHController::OnTabGroupChanged(const TabGroupChange& change) { void TabGroupsIPHController::OnTabGroupChanged(const TabGroupChange& change) {
......
...@@ -10,8 +10,9 @@ ...@@ -10,8 +10,9 @@
#include "base/bind.h" #include "base/bind.h"
#include "chrome/browser/feature_engagement/tracker_factory.h" #include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_commands.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/in_product_help/feature_promo_controller_views.h" #include "chrome/browser/ui/views/in_product_help/feature_promo_controller_views.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "chrome/test/views/chrome_test_widget.h" #include "chrome/test/views/chrome_test_widget.h"
#include "components/feature_engagement/public/event_constants.h" #include "components/feature_engagement/public/event_constants.h"
...@@ -25,65 +26,57 @@ ...@@ -25,65 +26,57 @@
using ::testing::_; using ::testing::_;
using ::testing::AnyNumber; using ::testing::AnyNumber;
using ::testing::NiceMock;
using ::testing::Ref; using ::testing::Ref;
using ::testing::Return; using ::testing::Return;
class TabGroupsIPHControllerTest : public BrowserWithTestWindowTest { class TabGroupsIPHControllerTest : public TestWithBrowserView {
public: public:
void SetUp() override { void SetUp() override {
BrowserWithTestWindowTest::SetUp(); TestWithBrowserView::SetUp();
views::Widget::InitParams widget_params;
widget_params.context = GetContext();
anchor_widget_ =
views::UniqueWidgetPtr(std::make_unique<ChromeTestWidget>());
anchor_widget_->Init(std::move(widget_params));
mock_tracker_ = mock_tracker_ =
feature_engagement::TrackerFactory::GetInstance() static_cast<NiceMock<feature_engagement::test::MockTracker>*>(
->SetTestingSubclassFactoryAndUse( feature_engagement::TrackerFactory::GetForBrowserContext(
GetProfile(), base::BindRepeating([](content::BrowserContext*) { profile()));
return std::make_unique<
feature_engagement::test::MockTracker>();
}));
// Other features call into the IPH backend. We don't want to fail
// on their calls, so allow them. Our test cases will set
// expectations for the calls they are interested in.
EXPECT_CALL(*mock_tracker_, NotifyEvent(_)).Times(AnyNumber());
EXPECT_CALL(*mock_tracker_, ShouldTriggerHelpUI(_))
.Times(AnyNumber())
.WillRepeatedly(Return(false));
promo_controller_ =
std::make_unique<FeaturePromoControllerViews>(browser()->profile());
iph_controller_ = std::make_unique<TabGroupsIPHController>( promo_controller_ = browser_view()->feature_promo_controller();
browser(), promo_controller_.get(), iph_controller_ = browser_view()->tab_groups_iph_controller();
base::BindRepeating(&TabGroupsIPHControllerTest::GetAnchorView,
base::Unretained(this)));
} }
void TearDown() override { void TearDown() override {
iph_controller_.reset(); iph_controller_ = nullptr;
anchor_widget_.reset(); promo_controller_ = nullptr;
BrowserWithTestWindowTest::TearDown(); TestWithBrowserView::TearDown();
} }
private: TestingProfile::TestingFactories GetTestingFactories() override {
views::View* GetAnchorView(int tab_index) { TestingProfile::TestingFactories factories =
return anchor_widget_->GetContentsView(); TestWithBrowserView::GetTestingFactories();
factories.emplace_back(feature_engagement::TrackerFactory::GetInstance(),
base::BindRepeating(MakeTestTracker));
return factories;
} }
// The Widget our IPH bubble is anchored to. It is specifically private:
// anchored to its contents view. static std::unique_ptr<KeyedService> MakeTestTracker(
views::UniqueWidgetPtr anchor_widget_; content::BrowserContext* context) {
auto tracker =
std::make_unique<NiceMock<feature_engagement::test::MockTracker>>();
// Allow other code to call into the tracker.
EXPECT_CALL(*tracker, NotifyEvent(_)).Times(AnyNumber());
EXPECT_CALL(*tracker, ShouldTriggerHelpUI(_))
.Times(AnyNumber())
.WillRepeatedly(Return(false));
return tracker;
}
protected: protected:
feature_engagement::test::MockTracker* mock_tracker_; NiceMock<feature_engagement::test::MockTracker>* mock_tracker_;
std::unique_ptr<FeaturePromoController> promo_controller_; FeaturePromoController* promo_controller_;
std::unique_ptr<TabGroupsIPHController> iph_controller_; TabGroupsIPHController* iph_controller_;
}; };
TEST_F(TabGroupsIPHControllerTest, NotifyEventAndTriggerOnSixthTabOpened) { TEST_F(TabGroupsIPHControllerTest, NotifyEventAndTriggerOnSixthTabOpened) {
......
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